search

yahoo / monitr

A Node.js process monitoring tool.
BSD 3-Clause "New" or "Revised" License
317 stars 39 forks source link

Can't set headers after sent error #1

Open rragan opened 10 years ago

rragan commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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.