oven-sh / bun

Incredibly fast JavaScript runtime, bundler, test runner, and package manager – all in one
https://bun.sh
Other
74.18k stars 2.77k forks source link

Bun.serve() silently drops connection after 10 seconds #13712

Closed saminiemi closed 2 months ago

saminiemi commented 2 months ago

What version of Bun is running?

1.1.26+0a37423ba

What platform is your computer?

Darwin 23.5.0 arm64 arm

What steps can reproduce the bug?

import { sleep, serve } from "bun";
const s = serve({
  async fetch(req) {
    console.log(req.method + " " + req.url)
    const start = performance.now();
    console.log("start")
    await sleep(1000*15);
    console.log("stop")
    const end = performance.now();
    return new Response(`Slept for ${end - start}ms`);
  },
});
console.log("http://"+s.hostname+":"+s.port)

Run code and connect with curl:

# time curl http://localhost:3000
curl: (52) Empty reply from server
curl http://localhost:3000  0,01s user 0,01s system 0% cpu 10,066 total

What is the expected behavior?

Bun.serve should keep up running request more than 10 sec

What do you see instead?

Bun.serve() silently drops request after 10 sec.

Additional information

This is working well with Bun 1.1.25 and earlier. We have tried also in Linux environment and on Google Cloud Run with official Bun docker container, and result is same: 1.1.26 drops connection taking more than 10 seconds.

songinnight commented 2 months ago

Why not set the idleTimeout (in seconds) in the serve parameter? (The maximum limit seems to be 255).

serve({
  idleTimeout: 255,
  async fetch(req) {
    // ...
  }
})
saminiemi commented 2 months ago

Thanks @songinnight .

While that will fix the example, it is more tricky to set idleTimeout on frameworks using Bun.serve under the hood, like Astro and Elysia. Also, some error message should be printed on log when hit the timeout.

Default of serve timeout should be at least 60 seconds (I prefer no timeout by default like Node) when using it building APIs etc.

cirospaciari commented 2 months ago

By default uWS timeouts in 10s in this specific case, if you send a response and stream the body using a ReadableStream each time ou send a chunk will reset the timeout again, so the total timeout is not 10s, but 10s idle. This was already a thing before 1.1.25 you see the Issue here: https://github.com/oven-sh/bun/issues/13392 I plan to add more improvements on timeouts soon, so the user will have more control over it. Node.js by default dont have a idleTimeout (you can add one using setTimeout), but have a requestTimeout of 300s or 300000ms https://nodejs.org/api/http.html#serverrequesttimeout

saminiemi commented 2 months ago

Thanks @cirospaciari for your quick reply.

My apologize, but I have to disagree.

First, example above is working with Bun 1.1.25 flawless, no silent timeout. Issue #13392 was with HTTPS, which no doubt is different implementation internally and has been with 10 sec timeout on earlier Bun releases also as you said.

Let's try with basic Node/Express example:

const express = require('express')
const app = express()
const port = 3000

app.get('/', async (req, res) => {
    await new Promise((resolve) => setTimeout(resolve, 15000));
    res.send('Hello World!')
})

app.listen(port, () => {
  console.log(`Example app listening on port ${port}`)
})

This code will run nice with Node and with Bun 1.1.25, but with Bun 1.1.26 it will drop connection after 10 sec.

Considering that express is one of the most used npm package with over 5 million downloads per week, there must be hundreds of thousands Apis, backends etc built on it. Now, since Bun is drop-in replacement for Node, all backends which replaces Node with Bun as runtime (like we happily do), will not work as expected after upgrading to Bun 1.1.26. Even worse, there is no any kind of error message about timeout on server logs, only clients will notice dropped connection. We spent hours to find root reason.

Now when we all know this, I am sure we can agree that default timeout has to be much higher that 10 sec (Node's 300 seconds sound good). Otherwise, we cannot anymore consider Bun as drop-in replacement for Node.

BridgeSenseDev commented 2 months ago

Thanks @cirospaciari for your quick reply.

My apologize, but I have to disagree.

First, example above is working with Bun 1.1.25 flawless, no silent timeout. Issue #13392 was with HTTPS, which no doubt is different implementation internally and has been with 10 sec timeout on earlier Bun releases also as you said.

Let's try with basic Node/Express example:

const express = require('express')
const app = express()
const port = 3000

app.get('/', async (req, res) => {
    await new Promise((resolve) => setTimeout(resolve, 15000));
    res.send('Hello World!')
})

app.listen(port, () => {
  console.log(`Example app listening on port ${port}`)
})

This code will run nice with Node and with Bun 1.1.25, but with Bun 1.1.26 it will drop connection after 10 sec.

Considering that express is one of the most used npm package with over 5 million downloads per week, there must be hundreds of thousands Apis, backends etc built on it. Now, since Bun is drop-in replacement for Node, all backends which replaces Node with Bun as runtime (like we happily do), will not work as expected after upgrading to Bun 1.1.26. Even worse, there is no any kind of error message about timeout on server logs, only clients will notice dropped connection. We spent hours to find root reason.

Now when we all know this, I am sure we can agree that default timeout has to be much higher that 10 sec (Node's 300 seconds sound good). Otherwise, we cannot anymore consider Bun as drop-in replacement for Node.

Same, I'm using hono and spent a long time wondering why connections just get dropped after 10 seconds