Make bounced requests default to non-persistent. #10

Closed
wants to merge 1 commit into
from

Projects

None yet

2 participants

@sethml
Contributor
sethml commented Nov 11, 2011

This is really more of a request for assistance / bug request than a pull request. This isn't ready to be merged yet.

Whenever a request is bounced, a new connection is made to the target. These connections default to keep-alive. This means that if the server making the original request makes several requests on the same connection, all of the bounced requests get piped back into the original request. The most obvious result is that you get this message, due to all the pending pipes:

(node) warning: possible EventEmitter memory leak detected. 11 listeners added. Use emitter.setMaxListeners() to increase limit.
Trace: 
    at Socket.<anonymous> (events.js:126:17)
    at Socket.pipe (stream.js:46:8)
    at /Users/seth/development/service/node_modules/bouncy/index.js:90:20
    at /Users/seth/development/service/scalar.js:52:5
    at IncomingMessage.onHeaders (/Users/seth/development/service/node_modules/bouncy/index.js:46:13)
    at IncomingMessage.emit (events.js:64:17)
    at Parser.<anonymous> (/Users/seth/development/service/node_modules/bouncy/node_modules/parsley/lib/modes.js:135:21)
    at Parser.execute (/Users/seth/development/service/node_modules/bouncy/node_modules/parsley/index.js:20:35)
    at Socket.<anonymous> (/Users/seth/development/service/node_modules/bouncy/node_modules/parsley/index.js:11:14)
    at Socket.emit (events.js:64:17)

Presumably also it can tie up a fair amount of resources, since there can be a large number of pending keep-alive connections on the target.

My solution works in my tests, but it causes some of the bouncy testcases to fail, and I think it breaks some reasonable uses. Any suggestions for a better approach?

For best performance, it would really be best if the server could re-use the keepalive bounces, but I'm not sure what an API to do that woudl look like.

@substack
Owner

Now in 3.0 bouncy will stop caring what happens past the first message so keep-alive will just work. There is a test verifying this behavior too.

@substack substack closed this Mar 22, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment