Conversation
|
This keeps failing on node v4.0. |
|
OK, I will look into it. |
|
My investigation shows that the test fails because wreck never receives the reply. The behavior changes in the v4.1.1 to v4.1.2 update, and I believe my test exposes the bug / security issue fixed in nodejs/node@0504066. Do we really need to fully support 4.0.0 considering outmoded/hapi-contrib#54? |
|
Look at that, my rebased version passes 😉 Let me know if you have any other concerns. |
| } | ||
|
|
||
| const isInjection = Shot.isInjection(request.raw.req); | ||
| if (!isInjection && !request.connection._started) { |
There was a problem hiding this comment.
I don't like accessing non-public node apis with _started.
There was a problem hiding this comment.
This is a Hapi connection property lookup to know when server.stop() has been called.
|
This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions. |
2 participants
When a server is handling persistent connections (using http keep-alive), the
timeoutoption forserver.stop()is a bit mislabeled. If just one of the persistent connections don't make any requests within the timeout duration, the server waits for the timeout even though no requests are processing. As such, it is effectively a stop delay option.This patch tracks the request processing state of the connections, and immediately ends idle connections on
server.stop(). Additionally, it signals that the server is closing on active requests (by addingconnection: closeto the response), and forcibly ends the connection once the response has been submitted. This means that thetimeoutoption works as expected, and the server is much more likely to stop before the timeout is reached).Note that actively closing these connections means that the closed server check on new requests from #2363, is no longer needed.
The first 3 patches adds additional tests that validate the current behavior, and the final patch actually implements the new behavior along with new and modified tests.