Skip to content

Conversation

EugenDueck
Copy link

by passing in an http.Agent instance (either http.globalAgent or any custom instance) to the XMLHttpRequest constructor.

This addresses #44 and #48.

This is so that connections can be reused by the standard Http Keep-Alive mechanism, that is based on the http.Agent.maxSockets setting. Without agents, every request means a new TCP connections, behavior that differs from browser implementations of XHR.

One ramification of the former that made me implement this was that one TCP socket per HTTP request severely limits your ability to do load testing, as all your outbound ports get consumed in the TIME_WAIT state in no time. Try e.g. socket-io-client and the xhr-polling transport from node.js, which will use node-XMLHttpRequest, to verify.

Browser implementations of XHR on the other hand recycle their TCP sockets, otherwise socket-io based clients would bail as well in normal, non load-testing scenarios, when more than 16k events in 4 minutes are sent (the numbers highly depend on your OS and TCP settings).

Implementation note: It maintains backwards compatibility, in that if you don't pass in an agent, none will be used.

Ideally however, I'd like http.globalAgent to be the default, like it is with node's http requests, and like it was the case for node-XMLHttpRequest until changed by 8958631 , which @guile also laments in #44. However, my patch maintains the current status quo.

by passing in an `http.Agent` instance (either `http.globalAgent` or a new instance) to the XMLHttpRequest constructor
@TooTallNate
Copy link

+1

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For future-proof-ness, maybe we should make this be an "options" object with and agent property. That allows for more options to be added in the future without adding unnecessary arguments.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes please. I'm about to send in an optional proxy as well.

johanneswuerbach added a commit to johanneswuerbach/node-XMLHttpRequest that referenced this pull request Jul 3, 2014
johanneswuerbach added a commit to johanneswuerbach/node-XMLHttpRequest that referenced this pull request Jul 4, 2014
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

Successfully merging this pull request may close these issues.

3 participants