maxSockets option

[super-ienien]

Hi,

I think there is a problem with the way this option is done in the code.

It create a new pool object for each request. Here is the documentation of the request module about that :

An object describing which agents to use for the request. If this option is omitted the request will use the global agent (as long as your options allow for it). Otherwise, request will search the pool for your custom agent. If no custom agent is found, a new agent will be created and added to the pool. Note: pool is used only when the agent option is not specified.

So a new agent is created for each request.

And the doc also say :

Note that if you are sending multiple requests in a loop and creating multiple new pool objects, maxSockets will not work as intended. To work around this, either use request.defaults with your pool options or create the pool object with the maxSockets property outside of the loop.

[dantman]

You might be right. To fix this we'd have to cache the pool object for each possible maxSockets value. It's also of note that the README doesn't actually document maxSockets as an option, nor the MAX_SOCKETS env.

This would also probably be a breaking change. fb.js sets maxSockets to 5 be default, but because of this issue the real value is Infinity.

I think we can ditch Thuzi's random choices. We could drop the MAX_SOCKETS env and the 5 default.

However I think it's worth going further than that and thinking about the more practical usage. maxSockets as it is, is basically the same as the global setting but limited to Facebook connections. In the real world developers may want more specific control such as a pool tied to one app id or socket limits around only one loop.

Perhaps we should instead drop the maxSockets option and offer a pool option.

[super-ienien]

Yeah that's what I did in my fork of facebook-node.

I added a pool option in order to manage the sockets pool myself.

I think that actually this maxSockets option is very badly coded for those reasons :

• it creates a new http agent for each request
• it does not work as intended
• the default value of maxSockets 5 is an arbitrary value. The real default value should be Infinity. That's the default value when a new http agent is created (see the nodejs documentation)