Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error: This socket is closed. #74

Closed
nicokaiser opened this issue May 21, 2012 · 8 comments
Closed

Error: This socket is closed. #74

nicokaiser opened this issue May 21, 2012 · 8 comments

Comments

@nicokaiser
Copy link
Contributor

I just got this crash:

node.js:201
        throw e; // process.nextTick error, or 'error' event on first tick
              ^
Error: This socket is closed.
    at Socket._write (net.js:474:19)
    at Socket.write (net.js:466:15)
    at WebSocketServer.<anonymous> (/home/wwwdocs/applications/services/webpush/node_modules/ws/lib/WebSocketServer.js:292:16)
    at /home/wwwdocs/applications/services/webpush/node_modules/ws/lib/WebSocketServer.js:321:25
    at WebSocketServer.handleHixieUpgrade (/home/wwwdocs/applications/services/webpush/node_modules/ws/lib/WebSocketServer.js:368:3)
    at WebSocketServer.handleUpgrade (/home/wwwdocs/applications/services/webpush/node_modules/ws/lib/WebSocketServer.js:141:61)
    at Server.handleUpgrade (/home/wwwdocs/applications/services/webpush/lib/engine/lib/server.js:70:11)
    at Server.<anonymous> (/home/wwwdocs/applications/services/webpush/lib/engine/lib/webpush-engine.js:47:12)
    at Server.emit (events.js:88:20)
    at Socket.<anonymous> (http.js:1483:14)

node.js 0.6.17, ws 0.4.15.

I think (!) this has nothing to do with the latest update of WebSocketServer.js – and I wonder why this error was not caught by the try/catch (maybe this is a deferred thing again, and you have to check if the socket is open before the writes).

@nicokaiser
Copy link
Contributor Author

Just upgraded to node.js 0.6.18, but I'm not too optimistic...

I really don't understand when socket.write throws an exception and when it emits an 'error' event. You are try/catching exceptions, so either the write must be delayed, or emitting an error event which is not caught by the default http error handler (https://github.com/joyent/node/blob/master/lib/http.js#L1311) for some reason.

Maybe a check if the socket is still writable in completeHandshake workarounds this, but I'd like to understand it first...

Let's see if node 0.6.18 fixes or at least spits out a better backtrace.

@nicokaiser
Copy link
Contributor Author

Update: it crashed again:


events.js:48
        throw arguments[1]; // Unhandled 'error' event
                       ^
Error: This socket is closed.
    at Socket._write (net.js:474:19)
    at Socket.write (net.js:466:15)
    at WebSocketServer.<anonymous> (/home/wwwdocs/applications/services/webpush/node_modules/ws/lib/WebSocketServer.js:292:16)
    at /home/wwwdocs/applications/services/webpush/node_modules/ws/lib/WebSocketServer.js:321:25
    at WebSocketServer.handleHixieUpgrade (/home/wwwdocs/applications/services/webpush/node_modules/ws/lib/WebSocketServer.js:368:3)
    at WebSocketServer.handleUpgrade (/home/wwwdocs/applications/services/webpush/node_modules/ws/lib/WebSocketServer.js:141:61)
    at Server.handleUpgrade (/home/wwwdocs/applications/services/webpush/lib/engine/lib/server.js:79:11)
    at Server.<anonymous> (/home/wwwdocs/applications/services/webpush/lib/engine/lib/webpush-engine.js:47:12)
    at Server.emit (events.js:88:20)
    at Socket.<anonymous> (http.js:1521:14)

Now trying to add error and timeout handlers to the socket before handleUpgrade.

@nicokaiser
Copy link
Contributor Author

So far no crashes, but I'll observe it over the day.

These handlers should be in ws as the server should not crash before I (the application) have the chance to react and catch those crashes...

@nicokaiser
Copy link
Contributor Author

Update: still crashes even if I catch the socket's error event. This is really bad.


events.js:48
        throw arguments[1]; // Unhandled 'error' event
        ^
Error: This socket is closed.
    at Socket._write (net.js:474:19)
    at Socket.write (net.js:466:15)
    at WebSocketServer.<anonymous> (/home/wwwdocs/applications/services/webpush/node_modules/ws/lib/WebSocketServer.js:292:16)
    at /home/wwwdocs/applications/services/webpush/node_modules/ws/lib/WebSocketServer.js:321:25
    at WebSocketServer.handleHixieUpgrade (/home/wwwdocs/applications/services/webpush/node_modules/ws/lib/WebSocketServer.js:368:3)
    at WebSocketServer.handleUpgrade (/home/wwwdocs/applications/services/webpush/node_modules/ws/lib/WebSocketServer.js:141:61)
    at Server.handleUpgrade (/home/wwwdocs/applications/services/webpush/lib/engine/lib/server.js:79:11)
    at Server.<anonymous> (/home/wwwdocs/applications/services/webpush/lib/engine/lib/webpush-engine.js:47:12)
    at Server.emit (events.js:88:20)
    at Socket.<anonymous> (http.js:1521:14)

@nicokaiser
Copy link
Contributor Author

I have no idea

  • where this happens
  • why there is not 'error' handler (as I'm defining one before handing the socket to handleUpgrade)

Now I added a check for socket.writable before writing the nonce, let's wait some hours to see if this fixes the problem. Sigh.

@einaros
Copy link
Contributor

einaros commented May 23, 2012

Does #75 work for you? I have a different fix brewing, which seems to be a more long-term way to handle this issue. That'll take me a couple of hours to implement, though.

@nicokaiser
Copy link
Contributor Author

@einaros: so far it seems so...

How would you like to handle it other that that? Seems like node's socket.write is quite a mess, as it can a) throw exceptions b) return false and c) emit an 'error' if something goes wrong...

@einaros
Copy link
Contributor

einaros commented Jun 14, 2012

Closing this - let's reiterate if the current fixes aren't sufficient :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants