-
-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Not working with fastify #32
Comments
Hello @cjroebuck 👋 Thanks for opening the issue I think there is a bit a misunderstanding about the goal of Graceful Server project. Graceful Server does not wait for inflight requests. It just stops the incoming calls, returns a 503 via readiness, runs the close promises that are setup and then stop the server. The goal is to be sure to close all database pools (redis, postgres, etc). If you want this feature, feel free to make a PR / I can develop it but it will not be the default behavior because in the provided example, the number of inflight calls can really be huge 😅 What do you think of this? |
Ah ok, no problem! I had thought I had seen code where you were tracking
the connections etc!
Anyway not to worry, I would maybe make the fastify example a little bit
clearer as that is the one I followed and thought it was working ok before.
I know that you call server.close, but if fastify.close is already called
in the closePromises before then that will throw an unhandled
exception/rejection saying that the server is not running.
One other tiny thing - it is a little difficult to mock the process.exit in
tests due to the way it is currently imported.
Thank you Guillaume for your work on graceful server so far!!
…On Thu, 28 Nov 2024 at 09:08, Guillaume Quittet ***@***.***> wrote:
Hello @cjroebuck <https://github.com/cjroebuck> 👋
Thanks for opening the issue
I think there is a bit a misunderstanding about the goal of Graceful
Server project.
Graceful Server does not wait for inflight requests.
It just stops the incoming calls, returns a 503 via readiness, runs the
close promises that are setup and then stop the server.
The goal is to be sure to close all database pools (redis, postgres, etc).
If you want this feature, feel free to make a PR / I can develop it but it
will not be the default behavior because in the provided example, the
number of inflight calls can really be huge 😅
What do you think of this?
—
Reply to this email directly, view it on GitHub
<#32 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAC7MGJVP2WYWZXA26RQ632C3MSFAVCNFSM6AAAAABSTO2BVCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKMBVGYYDINRQHE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
It doesn't seem to work with fastify (v5) anymore - just shuts down even though there are inflight requests.
Example server :
How to recreate:
run the example server
node example.js
run a curl to http://127.0.0.1:3000
curl -v http://127.0.0.1:3000/
note that the log shows an incoming request (and it should be waiting for a response for 10 seconds).
now immediately either ctrl+c on the server, or send a SIGTERM to the process' pid:
kill -15 $PID
The server logs 'Server is shutting down', followed by 'Server is down because of SIGTERM' and it shuts down immediately.
The curl request gets cut and replies with curl: (52) Empty reply from server.
Expected - the server should wait for the inflight request to finish (after 10s) and then shutdown. The curl command should return 200.
The text was updated successfully, but these errors were encountered: