indutny / sticky-session

Sticky session balancer based on a `cluster` module
964 stars 99 forks source link

handle errors, like ECONNRESET, on the socket #12

Closed necccc closed 9 years ago

necccc commented 10 years ago

the socket, that was passed on to the worker, occasionally threw errors (like ECONNRESET), so at least handle&log the error

indutny commented 10 years ago

Does it really fix anything? At this point socket should be already sent to a worker process.

Do you have a test that will reproduce the problem?

necccc commented 10 years ago

I've been experimenting with primus/engine.io and sticky-sessions was almost perfect for clustering it on a server. During load tests occasionally an uncaptured ECONNRESET error killed the process, originating from the socket. Since most users use the simpler HTTP server in node, and not the Net server, I thought it may be not trivial to handle this error outside sticky-sessions. This fix does almost nothing, only logs the error, but the process won't terminate.

The Node HTTP module docs hints this at the 'connection' event:

Usually users will not want to access this event.

If you think - and I can understand why - that this error should be handled outside sticky-session, then maybe a hint in the docs would be nice.

ps.: sorry about the whitespace, fixed that

jondubois commented 10 years ago

Did you try to run sticky() inside an error domain? http://nodejs.org/api/domain.html

indutny commented 9 years ago

This is not going to land. Sorry!