SMC-728 - pull latest upstream master; add error handler in WebSocketRequest.js - #8
Conversation
csampsonza
left a comment
There was a problem hiding this comment.
Would be nice to drop the fork. But we need protection against EPIPE, .on('error' ... , and we have some pong logic - fix for connections being dropped when inbound queue causes us to miss pongs before keepalive timeout
| *Released 2015-07-22* | ||
|
|
||
| * Incremented and re-published to work around an aborted npm publish of v1.0.20. | ||
| * Incrememnted and re-published to work around an aborted npm publish of v1.0.20. |
| // Watch for the underlying TCP socket closing before we call accept | ||
| this._socketIsClosing = false; | ||
| this._socketCloseHandler = this._handleSocketCloseBeforeAccept.bind(this); | ||
| this._sockeErrorHandler = this._handleSocketErrorBeforeAccept.bind(this); |
There was a problem hiding this comment.
add missing 't' into _sockeErrorHandler
| this._removeSocketCloseListeners(); | ||
| }; | ||
|
|
||
| WebSocketRequest.prototype._handleSocketErrorBeforeAccept = function(err) { |
There was a problem hiding this comment.
why does this only apply to 'BeforeAccept'? are the error handlers overwritten later?
There was a problem hiding this comment.
Honestly I don't really understand it very well. It looks like this file only accepts or rejects the requests, then it gets handled on another level. So we add the error handler within this scope and remove it once it is accepted or rejected. I assume if an error can happen somewhere outside of this file that might cause an uncaught exception we would have to handle it there
JensEggers
left a comment
There was a problem hiding this comment.
see comment on typos, but if these came in from upstream merge it's prob best to leave them alone
No description provided.