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

Running 'setHeader' during 'proxyReq' event fails for POST requests #908

Open
cdl opened this issue Nov 16, 2015 · 9 comments
Open

Running 'setHeader' during 'proxyReq' event fails for POST requests #908

cdl opened this issue Nov 16, 2015 · 9 comments

Comments

@cdl
Copy link

cdl commented Nov 16, 2015

Right now, it looks like trying to do a setHeader on the passed in proxyReq for POST requests fails. (see below:)

proxy.on('proxyReq', function(proxyReq, req, res, options) {
  proxyReq.setHeader('origin', 'https://example.com');
});

The following error is thrown when the above tries to execute:

Error: Can't set headers after they are sent.

This seems to work totally fine for GET requests, just not POST requests. Managed to hack around it at first by wrapping the .setHeader call in check to make sure proxyReq._header was null, but it looks like that totally fails when Firefox tries to perform a POST request (the header doesn't get set).

The same thing appears to happen when trying to do res.setHeader, as suggested by #819.

@jcrugzz
Copy link
Contributor

jcrugzz commented Nov 16, 2015

@cdl what node version does this happen in? Could you write a reproducible test case? I want to establish if this is a deterministic issue and if not what conditions FULLY create it. Thanks!

@cdl
Copy link
Author

cdl commented Nov 16, 2015

@jcrugzz Happens in node 4.2.1. Will see about stubbing out a reproducible test case for it for sure.

@jcrugzz
Copy link
Contributor

jcrugzz commented Nov 17, 2015

@cdl can you upgrade to [email protected] there was an http bug in 4.2.1 that was resolved in the patch version. See if upgrading resolves this one as well. Let me know either way! Thanks

@cdl
Copy link
Author

cdl commented Nov 17, 2015

@jcrugzz Just tested in [email protected], issue still persists. Will write up a gist demonstrating the issue shortly.

@jcrugzz
Copy link
Contributor

jcrugzz commented Nov 17, 2015

@cdl sounds good, and if you can encapsulate it into a test that you can PR to this project that would get us closer to a solution :). Thanks for the update!

@stickystyle
Copy link

I had the same issue after upgrading to 1.12.0, downgrading back to 1.11.3 resolved it for me.

@jcrugzz
Copy link
Contributor

jcrugzz commented Nov 21, 2015

@stickystyle could you post a sample of the code that you use for http-proxy? This doesn't seem like a bug in http-proxy since its part of the http semantic so this is very curious as nothing changed around this code.

@stickystyle
Copy link

@jcrugzz I've just made a scrubbed gist of the app.js and package.json that I'm using. It's a simple proxy that serves to layer JWT and caching on top of a RESTful API.
I too did a compare of the tag to master and didn't see anything that stood out, so it makes me think that one of the http-proxy dependencies that got updated is triggering it. I'm on [email protected] as well.

https://gist.github.com/stickystyle/a35fa8bc7461018aad83

@stickystyle
Copy link

For me, this issue has gone away in the most recent v1.13.2

thiagobustamante added a commit to thiagobustamante/node-http-proxy that referenced this issue May 26, 2017
This PR tries to fix "Can't set headers after they are sent" errors.
That are a lot of situations where this error can occurs. In my case, it is happening because I have others middlewares (in an expressjs application that tries to proxy requests). Some of those middlewares (like [passportjs](http://passportjs.org/), or [cors](https://www.npmjs.com/package/cors)) can run ```res.end()``` and when the proxy receive a response, it is already finished.
So, it is necessary to test if we can write on the user response when the proxy response is ready.
I think it could also fix http-party#930, http-party#1168, http-party#908
jcrugzz pushed a commit that referenced this issue Apr 19, 2018
This PR tries to fix "Can't set headers after they are sent" errors.
That are a lot of situations where this error can occurs. In my case, it is happening because I have others middlewares (in an expressjs application that tries to proxy requests). Some of those middlewares (like [passportjs](http://passportjs.org/), or [cors](https://www.npmjs.com/package/cors)) can run ```res.end()``` and when the proxy receive a response, it is already finished.
So, it is necessary to test if we can write on the user response when the proxy response is ready.
I think it could also fix #930, #1168, #908
jcrugzz pushed a commit that referenced this issue Apr 20, 2018
This PR tries to fix "Can't set headers after they are sent" errors.
That are a lot of situations where this error can occurs. In my case, it is happening because I have others middlewares (in an expressjs application that tries to proxy requests). Some of those middlewares (like [passportjs](http://passportjs.org/), or [cors](https://www.npmjs.com/package/cors)) can run ```res.end()``` and when the proxy receive a response, it is already finished.
So, it is necessary to test if we can write on the user response when the proxy response is ready.
I think it could also fix #930, #1168, #908
indexzero pushed a commit that referenced this issue Apr 20, 2018
This PR tries to fix "Can't set headers after they are sent" errors.
That are a lot of situations where this error can occurs. In my case, it is happening because I have others middlewares (in an expressjs application that tries to proxy requests). Some of those middlewares (like [passportjs](http://passportjs.org/), or [cors](https://www.npmjs.com/package/cors)) can run ```res.end()``` and when the proxy receive a response, it is already finished.
So, it is necessary to test if we can write on the user response when the proxy response is ready.
I think it could also fix #930, #1168, #908
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

3 participants