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

Close the connection when target sends "Connection: close" header #1001

Closed
wants to merge 3 commits into from

Conversation

nicolayr
Copy link

@nicolayr nicolayr commented May 4, 2016

This prevents ECONNRESET errors from being thrown for connections
that are supposed to be closed (fixes #1000).

Nicolay Ramm added 2 commits May 4, 2016 17:35
This prevents ECONNRESET errors from being thrown for connections
that are supposed to be closed (fixes http-party#1000).
Don't immediately close the response socket when a `Connection: close`
header is received from the target. Instead simply skip the error
callback for connection errors on sockets that are supposed to be
closed.
@nicolayr
Copy link
Author

nicolayr commented May 6, 2016

The initial change caused some connections to be closed before all data was transmitted to the client.

I changed it so that instead of closing connections, connection errors for connections that are supposed to be closed don't trigger the error callback.

@@ -137,6 +137,12 @@ web_o = Object.keys(web_o).map(function(pass) {
proxyReq.on('error', proxyError);

function proxyError (err){
if (err.code === 'ECONNRESET' && proxyReq._headers && proxyReq._headers.connection === 'close') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

im about to merge the case with the econnreset throwing errors that arent really errors. you should rebase all that and try and get the tests to pass and ill consider the use case here.

@nicolayr
Copy link
Author

Seems to have been fixed by #966

@nicolayr nicolayr closed this Jun 15, 2016
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.

"Connection: close" header from target server is not honored
2 participants