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

Avoid error Uncaught Error: Can't set headers after they are sent. #867

Open
abarre opened this issue Aug 21, 2015 · 8 comments
Open

Avoid error Uncaught Error: Can't set headers after they are sent. #867

abarre opened this issue Aug 21, 2015 · 8 comments

Comments

@abarre
Copy link

abarre commented Aug 21, 2015

In this portion of the code : https://github.com/nodejitsu/node-http-proxy/blob/master/lib/http-proxy/passes/web-incoming.js#L148...L157, there is no verification if the headers of the response have already been sent to the client.
It happens in my case when an error is fired and after I return an error 500 to the client.

Fix :
This portion of code can be wrapped by if(!res.finished){} to prevent the exception Uncaught Error: Can't set headers after they are sent..

@jcrugzz
Copy link
Contributor

jcrugzz commented Aug 23, 2015

@abarre could you show me the full code where this happens? This shouldn't happen to you as the consumer but I'm guessing you are writing your own headers in a non error case?

@abarre
Copy link
Author

abarre commented Aug 23, 2015

My error handler is something similar to the example below :

proxy.on('error', function (err, req, res, target) {
  if (err.message.indexOf("ETIMEDOUT") !== -1) {
    res.writeHead(globals.TIMEOUT_ERROR, "socket hang up);
    res.end();
  }
 else.... (for other error case)
};

A response is returned to the client in the error handler.

@jcrugzz
Copy link
Contributor

jcrugzz commented Aug 24, 2015

@abarre so this happens in a case where both response and error events get emit? Does it throw specifically on res.writeHead(globals.TIMEOUT_ERROR, "socket hang up);?

@jcrugzz
Copy link
Contributor

jcrugzz commented Aug 24, 2015

If so your fix seems like it would be correct. I just want to establish why this is happening exactly :).

@abarre
Copy link
Author

abarre commented Aug 24, 2015

Hum, I don't remember well, I think that an error event is also emitted when we call the proxyReq.abort method.

@giuliopaci
Copy link

I am also experiencing very similar issue, but on lines 112-115.
In my case I am trying to remove cookie header from the request, before submitting the request to the proxyed server. The code is working with GET requests, but not with POST. According to "http" manual, this is expected as with GET it is not usual that headers are sent before end() function is called, while with POST requests, headers are sent as soon as the first data are sent.
Moreover my understanding is that the listener code and the rest of the code are executed asynchronously, so there is no success guarantee with GET method as well, but it is just much more probable to succeed. Am I right?

In my specific case headers are sent immediately during http.request creation. In this case it would be more useful to add an event (hopefully a synchronous one) at the end of common.setupOutgoing(), so that it would be possible to modify options instead of the request.

@bubenshchykov
Copy link

having smth similar
the difference is that my host app is doing nothing, no additional handling
and error comes from proxied app

var proxy = httpProxy.createProxyServer().on('error', () => {});
var apiProxy = route => http.use(route, (req, res) => {
    req.headers.host = url.parse(config.debitoorApi).host;
    proxy.web(req, res, {target: config.debitoorApi + route});
});

apiProxy('/api');
apiProxy('/partnersite');
apiProxy('/auth/autologin');
apiProxy('/login/oauth2');
apiProxy('/admin');
apiProxy('/apidoc');
stack:[] 11 items
0:Error: Can't set headers after they are sent.
1: at ServerResponse.OutgoingMessage.setHeader (_http_outgoing.js:346:11)
2: at /app/node_modules/http-proxy/lib/http-proxy/passes/web-outgoing.js:86:13
3: at Array.forEach (native)
4: at Array.writeHeaders (/app/node_modules/http-proxy/lib/http-proxy/passes/web-outgoing.js:84:35)
5: at ClientRequest.<anonymous> (/app/node_modules/http-proxy/lib/http-proxy/passes/web-incoming.js:157:20)
6: at emitOne (events.js:90:13)
7: at ClientRequest.emit (events.js:182:7)
8: at HTTPParser.parserOnIncomingClient [as onIncoming] (_http_client.js:458:21)
9: at HTTPParser.parserOnHeadersComplete (_http_common.js:103:23)
10: at TLSSocket.socketOnData (_http_client.js:348:20)
message:Can't set headers after they are sent.

It started to happen only http-proxy introduced. Would be nice to have web-outgoing.js functions wrapped in try catches with some error events emitted

@rick-kilgore
Copy link

rick-kilgore commented Nov 17, 2016

I forked this project into https://github.com/HBOCodeLabs/node-http-proxy and created a synchronous callback hook like @giuliopaci described. Here is the PR: #1091

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

No branches or pull requests

5 participants