Skip to content
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

Handle context canceled errors + better http proxy error handling #644

Merged
merged 2 commits into from
Dec 5, 2019
Merged

Handle context canceled errors + better http proxy error handling #644

merged 2 commits into from
Dec 5, 2019

Conversation

danlsgiga
Copy link
Contributor

@danlsgiga danlsgiga commented Apr 17, 2019

This PR fixes how fabio is handling the HTTP/HTTPS proxy "context canceled" errors that are caused basically by the client side closing the connection before getting any response and wrongly reporting a 502 status code in the logs. This is specially annoying for systems like Nomad that use long polling API endpoints and whenever you navigate in the UI, you are constantly closing the long polled connection and getting the context canceled error in the logs.

Along with the fix, we are using the HTTP Status Code 499 which is a non standard status code created by Nginx to represent Client Closed Request responses.

Since Fabio also supports HTTP/2, this also solves misleading logs reporting 502 responses when HTTP/2 is used... The HTTP/2 RFC allows a client to close a request connection when the requested resources are already in cache.

I've tested these changes in our internal QA environment and stderr logging is also improved by using the HTTP Proxy ErrorHandler along with context canceled being correctly handled.

Closes #264

@CLAassistant
Copy link

CLAassistant commented Apr 17, 2019

CLA assistant check
All committers have signed the CLA.

@danlsgiga
Copy link
Contributor Author

Can I please get someone to take a look in this PR? It would be nice to get it in the next release.

@aaronhurt
Copy link
Member

@danlsgiga yes, sorry I'll try to find some time to give this a look.

@danslimmon
Copy link

My team at Hashicorp would love to see this merged. We have some legitimate 502s we'd like to investigate, but we can't see them through the forest of illegitimate context-canceled 502s.

@aaronhurt
Copy link
Member

Seems a simple change. Thank you @danlsgiga ... and sorry for the long delay.

@aaronhurt aaronhurt merged commit 9c6c64a into fabiolb:master Dec 5, 2019
@danlsgiga danlsgiga deleted the context-cancelled-logging-fix branch December 5, 2019 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

http proxy error context canceled
4 participants