connection are not taken into account related to async handlers

[axelbodo]

These types of codes doesn't work with graceful shutdown mw: app.use('/longop',function(req, res, next) { var i = 0; var callback = function() { if(i == 30) { res.end(); return; } res.write(i + '\n'); i++; setTimeout(callback, 1000); }; callback(); });

This kind of handlers are very important, as they are common when nodejs is the frontend, and after some little logic they propagate the requests for one or more backends.

[serby]

Hi @axelbodo

Thanks for reporting the issue. Do you have any more information on the error you are getting? Also how do you expect this module to handle your situation?

[axelbodo]

Hi Serby, I think in your solution when next is reached in you mw, it decreases the inflight request count (am I reght?). however in case of async processing, like in the code I've mentioned, which is inserted after your mw, it returns sooner, then the request is handled.

For better handling I'd use http.ServerResponse 'finish' event (https://nodejs.org/api/http.html#http_event_finish) to catch the end of an inflight request. And I'use 'close' event too, in order not to miss a premature close of the connection.

[serby]

Sorry, I'm really not following you.

This is all that happens in this module, it is very simple as behaves as I would expect.

https://github.com/serby/express-graceful-shutdown/blob/master/index.js

[axelbodo]

try that code I've written, and curl it. kill the express app, while the curl is in the middle of counting to 0 to 30. the result: admin@ip-172-20-67-173:~$ curl 100.96.10.12:3001/longop 0 1 2 3 4 5 6 7 8 9 curl: (18) transfer closed with outstanding read data remaining.

I've ment close event not close function. As when your sigterm handler runs, it means this is the only script which runs on the event loop, but not means there is no outstanding response due to timecallback or io callback on backend sockets (e.g. a redis/cassandra query or a backend service rest api call). The time callback just demonstrates this situation.

[serby]

That is the expected behaviour. https://github.com/serby/express-graceful-shutdown/blob/master/index.js#L25

[axelbodo]

it is shutdwon at once, not after 30 sec, at the very time i call kill .

[serby]

I understand. That is NOT correct behaviour. ctrl-c and kill should send a SIGTERM

I'll investigate further.

[axelbodo]

I've found one part of the problem. NODE_ENV was not mentioned in readme.md, this caused the premature end. after setting it the curl runs correctly. When I tried it with links (as I remember) the server waited longer, as the connection is in keep-alive.

I think it would be appriciated, if the idle alive connections will be closed before calling server.close, and every connection with inflight request would be closed in case of beeing in shutdown state, end the response triggered a finish or close event. This would make faster the shutdown process, it won't wait unecessarily for kept alive but actually idle connections, making for example a kubernetes rolling update more smooth as and fast.

However this needs some sort of connection tracking, whether they are idle or inflight and so on. Because I've missed the NODE_ENV setting, the original issue is voided, and my advice in this comment is just a nice to have feature.