Skip to content

Fix some corner case client errors#3971

Merged
hueniverse merged 3 commits intohapijs:masterfrom
kanongil:fix-client-errors
Sep 14, 2019
Merged

Fix some corner case client errors#3971
hueniverse merged 3 commits intohapijs:masterfrom
kanongil:fix-client-errors

Conversation

@kanongil
Copy link
Copy Markdown
Contributor

It turns out that the trigger in #3969 does indeed contain a bug. Namely that a request, that is aborted during the request lifecycle, can cause a timeout, and log a 503 response even though nothing was ever sent down the wire.

Further investigation found that this was caused by the stream draining code in the default handler, which never completes if the stream is aborted. This in turn means that the _lifecycle() call in request._execute() never returns, and thus skips the call to _reply() that would clear the timeout timer.

The other part of the patch relates to the clientError handling. Here I found a case where a client can receive a 400 Bad Request response, while the server logs it as a regular response.
This is because, contrary to the node docs, the clientError event can be triggered while processing an request on the socket, which hapi doesn't expect.

I have updated the handler, to send a 400 Bad Request response through any outstanding requests instead of writing it directly to the socket, which seems to fix the issue.

This ensures that any active request tied to the socked is logged correctly
@hueniverse hueniverse self-assigned this Sep 14, 2019
@hueniverse hueniverse added the bug Bug or defect label Sep 14, 2019
@hueniverse hueniverse added this to the 18.4.0 milestone Sep 14, 2019
@hueniverse hueniverse merged commit 9279db5 into hapijs:master Sep 14, 2019
hueniverse added a commit that referenced this pull request Sep 14, 2019
@lock
Copy link
Copy Markdown

lock bot commented Jan 9, 2020

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.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bug Bug or defect

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants