josephg / node-browserchannel

An implementation of a google browserchannel server in node.js
287 stars 47 forks source link

Uncaught error on hangup (new in nodejs v0.8.20) #20

Closed dejj closed 11 years ago

dejj commented 11 years ago

nodejs Node v0.8.20 (Stable) introduced the following change:

http: Raise hangup error on destroyed socket write (isaacs)

This breaks server.coffee on line 910 with the following error:

timers.js:103
            if (!process.listeners('uncaughtException').length) throw e;
                                                                      ^
Error: socket hang up
    at createHangUpError (http.js:1360:15)
    at ServerResponse.OutgoingMessage._writeRaw (http.js:507:26)
    at ServerResponse.OutgoingMessage._send (http.js:476:15)
    at ServerResponse.OutgoingMessage.write (http.js:740:18)
    at messagingMethods.writeRaw (/home/dejj/myproject/node_modules/browserchannel/lib/server.coffee:93:22)
    at Object.module.exports.session [as _onTimeout] (/home/dejj/myproject/node_modules/browserchannel/lib/server.coffee:557:13)
    at Timer.list.ontimeout (timers.js:101:19)

Setting the timeout needs to be wrapped in try-catch and possibly some error handling.

Also see: https://github.com/LearnBoost/socket.io/issues/1160

jagill commented 11 years ago

+1. This breaks our app when we tried to upgrade to 0.8.20.

rissem commented 11 years ago

+1. The error seems to be coming from https://github.com/josephg/node-browserchannel/blob/master/lib/server.coffee#L910

josephg commented 11 years ago

Yep, that sure would crash browserchannel. Closed sockets throwing errors is annoying - though I can't complain too much since browserchannel has the same behaviour :)

The simplest fix is to make the write methods ( eg these: https://github.com/josephg/node-browserchannel/blob/master/lib/server.coffee#L180-L181 ) do nothing if the socket is already closed.

rissem commented 11 years ago

This actually seems to be resolved by simply upgrading to node v0.8.21

josephg commented 11 years ago

Yeah, this issue is isolated to 0.8.20. Just don't use that version of nodejs.