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

Can't set headers after sent error #1

Open
rragan opened this issue Nov 26, 2013 · 3 comments
Open

Can't set headers after sent error #1

rragan opened this issue Nov 26, 2013 · 3 comments

Comments

@rragan
Copy link

rragan commented Nov 26, 2013

monitr works fine when I run jMeter with 1 user against our app. When I try to increase the number of concurrent users I rapidly get this error:

Error: Can't set headers after they are sent.
at ServerResponse.OutgoingMessage.setHeader (http.js:692:11)
at ServerResponse.res.setHeader (/x/web/STAGE2LP39/walletexpnodeweb/node_modules/express/node_modules/connect/lib/patch.js:63:22)
at next (/x/web/STAGE2LP39/walletexpnodeweb/node_modules/express/node_modules/connect/lib/proto.js:156:13)
at /x/web/STAGE2LP39/walletexpnodeweb/node_modules/express/lib/application.js:121:9
at next (/x/web/STAGE2LP39/walletexpnodeweb/node_modules/express/node_modules/connect/lib/proto.js:129:23)
at /x/web/STAGE2LP39/walletexpnodeweb/node_modules/async/lib/async.js:229:13
at /x/web/STAGE2LP39/walletexpnodeweb/node_modules/async/lib/async.js:116:25
at /x/web/STAGE2LP39/walletexpnodeweb/node_modules/async/lib/async.js:24:16
at /x/web/STAGE2LP39/walletexpnodeweb/node_modules/async/lib/async.js:226:17
at /x/web/STAGE2LP39/walletexpnodeweb/node_modules/async/lib/async.js:513:34

it appears to be due to the interception of the resp.end in lib/monitor.js

I don't know the timing considerations but I'm guessing there is some sort of timing issues around the patched intercept. Sorry I don't have much more to go on. Merely doing the require of monitr is enough to cause the problem. No monitor.start() needed.

@rohiniwork
Copy link
Contributor

Hi Richard,

Although my examples I cannot repro the issue, I am guessing where the problem could be.

Could you please try out my branch and let me know if that fixes it

https://github.com/rohiniwork/monitr/tree/setheaders-issue-1

If it does, we will merge it in.

Thanks,
--rohini

@rragan
Copy link
Author

rragan commented Nov 26, 2013

The change seems to have fixed the set headers problem. However when monitr is loaded into the app it starts getting a number of CSRF reported errors (FORBIDDEN) which do not happen when monitr is not around. I'm guessing something is messing up the request object in some way so the session info is changed or missing when express/connect make the CSRF check. When I display the token in the csrf.js module of connect it show "NOT FOUND".

@rohiniwork
Copy link
Contributor

Thanks Richard.
I have merged the fix.

Regarding the other issue (CSRF issue), I will try to reproduce it sometime. Meanwhile if you can find the root cause, feel free to submit a pull request.

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

2 participants