oven-sh / bun

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

Provide a way to await the http server close or time it out #9359

Closed ksmithut closed 2 weeks ago

ksmithut commented 8 months ago

What is the problem this feature would solve?

Shutting down the http server is not a clean process. You have the choice of either gracefully shutting down with the possibility of connections continuing to hang or just forcibly ending all connections, leaving clients with bad connection statuses.

What is the feature you are proposing to solve the problem?

I would like to have cleaner server shutdown logic. Ideally, SIGINT is sent to the process and we can run server.close() to stop new connections from coming in, but let existing requests finish up. It would be nice if server.close() also triggered each of the requests abort signals so that things like websockets and server-sent-events could manually be shut down. Then having server.close() or something else expose a promise that would resolve when all the requests exited cleanly, we could then clean up the database connections that those open requests were using. Exposing the promise would make it easier to time out the cleanup process in the event of runaway requests.

Something like this:

const server = Bun.serve({
  async fetch(req) {
    // stream returns a Response with a streaming body that we control with the callback
    // The function we return gets called when the `req.signal` is aborted.
    return stream(req.signal, (send) => {
      const interval = setInterval(() => {
        send(': keepalive\n\n')
      }, 3000)
      return () => clearInterval(interval)
    })
  }
})

process.on('SIGINT', () => {
  // timeout would be a function that takes in a promise and a timeout in ms
  // it will throw if the promise doesn't resolve before the provided ms have elapsed.
  timeout(server.close(), 5000)
    .catch(error => server.close(true))
    .then(() => process.exit(0))
})

What alternatives have you considered?

I've considered wrapping the fetch callback in something that initializes a new AbortController that I could keep track of for each request and then loop over upon server shutdown to abort each request. However, for that to work without a memory leak I need to figure out a way to tie into a response finishing so I can clean that up. I couldn't find a way to know when the connection finished to clean up those abort controllers. It seems like it would be easier to do that in the internal server code.

e3dio commented 6 months ago

server.close() should return a Promise resolving when all connections are closed, currently there is no way to know when server is done so you can finish app cleanup

ibash commented 3 months ago

You can busy wait right now:

process.on('SIGTERM', async () => {
  console.log('SIGTERM received, shutting down')
  server.stop()

  while (server.pendingRequests !== 0) {
    await new Promise((resolve) => setTimeout(resolve, 1000))
  }

  process.exit()
})
Jarred-Sumner commented 2 weeks ago

In Bun v1.1.30, we added support for await'ing the server.stop() function call.

test("should be able to await server.stop()", async () => {
  const ready = Promise.withResolvers();
  const received = Promise.withResolvers();
  using server = Bun.serve({
    port: 0,
    // Avoid waiting for DNS resolution in fetch()
    hostname: "127.0.0.1",
    async fetch(req) {
      received.resolve();
      await ready.promise;
      return new Response("Hello World", {
        headers: {
          // Prevent Keep-Alive from keeping the connection open
          "Connection": "close",
        },
      });
    },
  });

  // Start the request
  const responsePromise = fetch(server.url);
  // Wait for the server to receive it.
  await received.promise;
  // Stop listening for new connections
  const stopped = server.stop();
  // Continue the request
  ready.resolve();
  // Wait for the response
  await (await responsePromise).text();
  // Wait for the server to stop
  await stopped;
  // Ensure the server is completely stopped
  expect(async () => await fetch(server.url)).toThrow();
});

https://github.com/oven-sh/bun/pull/14120