Ws: read ECONNRESET error on disconnect (ws 3.3.3)

[G-Rath]

• [x] I've searched for any related issues and avoided creating a duplicate issue.

Description

Upon updating to ws 3.3.3, when a client disconnects the following exception is thrown:

events.js:183
      throw er; // Unhandled 'error' event
      ^

Error: read ECONNRESET
    at _errnoException (util.js:1024:11)
    at TCP.onread (net.js:615:25)

Code works fine in previous version.

Reproducible in:

version: 3.3.3
Node.js version(s): v8.9.1
OS version(s): Windows 10 Pro, Fall update

Steps to reproduce:

1. Create a websocket server
2. Connect to the websocket server via Chrome (`new WebSocket('ws://localhost:3000'))
3. Disconnect from the websocket server (via refreshing the page in Chrome)

Expected result:

The disconnect is handled gracefully, with the 'close' event being called.

Actual result:

An exception is thrown, which crashes the app:

events.js:183
      throw er; // Unhandled 'error' event
      ^

Error: read ECONNRESET
    at _errnoException (util.js:1024:11)
    at TCP.onread (net.js:615:25)

[G-Rath]

Issue is caused by lack of error listener on the socket, and can be fixed like so:

ws.on('error', () => console.log('errored'));

This isn't documented in the README.md, and seems to be a pretty big breaking change.

[lpinca]

You need to always add a listener for the 'error' event as there are other errors that can be emitted.
net.Socket errors are no longer swallowed in version 3.3.3.

[wangjian1119]

his is the case for the latest Google browsers

[polkovnikov-ph]

@lpinca So can we add it to README? If it's a required thing to do, why is it absent there?

_Edit._ This is totally a good change, but it should be at least documented properly.

[lpinca]

It is documented, see https://github.com/websockets/ws/blob/180b388a2ed83fddd667d88970a943d0f47d32ec/doc/ws.md#event-error-1

[G-Rath]

@lpinca You document that that event is where any errors from underlying net.Socket are forwarded to, but not the fact that a simple client disconnection will cause such an exception.

To me, because you have a 'close' event, I'd assume that exceptions caused as a result of a client disconnecting would be handled by this library, since it's something that the library seems to be able to detect and act on.

I think is a reasonable assumption to make, given that clients disconnecting is a perfectly normal event for a socket server.

[quigleyj97]

We've been running into this issue as well, but it's puzzling.

Chrome 62 and below do not cause an error using these steps, and neither do any other browsers that I've tested (Firefox 57, Firefox Mobile (57), Chrome Mobile (62)).

Further, with Wireshark I wasn't able to observe any significant difference in the close frames. Both Firefox and Chrome send the same 1001 closing code ("Going away").

With the error handler, it appears when closing a Chrome 63 tab that the error handler and the close handler are called at roughly the same time, but I haven't yet confirmed this.

[lpinca]

@G-Rath

but not the fact that a simple client disconnection will cause such an exception.

Yes because it shouldn't, the issue seems to happen only on Chrome 63.

To me, because you have a 'close' event, I'd assume that exceptions caused as a result of a client disconnecting would be handled by this library, since it's something that the library seems to be able to detect and act on.

I think is a reasonable assumption to make, given that clients disconnecting is a perfectly normal event for a socket server.

Swallowing errors is never a good idea. Developers can ignore errors but they may want to know if the connection was closed cleanly or not.

@quigleyj97

I didn't check but maybe latest Chrome sends the close frame and abruptly closes the connection immediately after that.

[G-Rath]

@lpinca That all makes far more sense; that's a mistake on my part - I didn't test it with another browser, and so assumed that that was something that commonly happens with client disconnections.

[lpinca]

I can no longer reproduce the issue on Chrome 63.0.3239.108 can anyone confirm?

[lpinca]

Nevermind, here is a test case which doesn't use ws:

const assert = require('assert');
const crypto = require('crypto');
const http = require('http');

const GUID = '258EAFA5-E914-47DA-95CA-C5AB0DC85B11';

const data = `<!DOCTYPE html>
<html>
  <head>
    <meta charset="utf-8">
  </head>
  <body>
    <script>
      (function () {
        var ws = new WebSocket('ws://localhost:8080');

        ws.onopen = function () {
          console.log('open');
        };
      })();
    </script>
  </body>
</html>`;

const server = http.createServer();

server.on('request', (req, res) => {
  res.setHeader('Content-Type', 'text/html');
  res.end(data);
});

server.on('upgrade', (req, socket) => {
  const key = crypto.createHash('sha1')
    .update(req.headers['sec-websocket-key'] + GUID)
    .digest('base64');

  socket.on('error', console.error);
  socket.on('data', (data) => {
    assert.ok(data.slice(0, 2).equals(Buffer.from([0x88, 0x82])));
    socket.write(Buffer.from([0x88, 0x00]));
  });

  socket.write([
    'HTTP/1.1 101 Switching Protocols',
    'Upgrade: websocket',
    'Connection: Upgrade',
    `Sec-WebSocket-Accept: ${key}`,
    '\r\n'
  ].join('\r\n'));
});

server.listen(8080, () => console.log('Listening on *:8080'));

[ProgrammingLife]

I'm in the same boat.

ws.on('error', () => console.log('errored'));

this doesn't solve the problem.

[lpinca]

@ProgrammingLife it should.
In https://github.com/websockets/ws/issues/1266#issue-285162876 you added the listener on the server so make sure you are adding it to the WebSocket instance instead.

[ICarryTheDustOfAJourney]

I'm in the same boat.

ws.on('error', () => console.log('errored'));

this doesn't solve the problem.

I can confirm this statement, when the handler is applied to the server. No more crashes when applied to each connection individually.

These errors occur quite often on Chrome V 63.0.3239.84 (=latest for my distribution).

FF 57.0.3 doesn't produce this error at all. Same with Edge on Windows clients.

Node v8.9.3 ws 3.3.3 ubuntu1~16.04.5 Linux 4.4.0-21-generic (x86_64)

[lpinca]

According to https://bugs.chromium.org/p/chromium/issues/detail?id=798194#c6 this works as intended.

What we can do here is adding a default 'error' listener which is a noop, so people are not forced to add one. Users can still get errors by adding their own 'error' listener(s).

Thoughts?

[lpinca]

Another possible workaround for Chrome is calling close() from a listener of the 'beforeunload' event:

window.addEventListener('beforeunload', function () {
  ws.close();
});

[endel]

I like this proposal https://github.com/websockets/ws/issues/1256#issuecomment-355536426

The error looks like a bug in the library since it started to happen recently.

[lpinca]

@endel the error is not a bug in the library and https://github.com/websockets/ws/issues/1256#issuecomment-354305677 is the proof. The fact that it only happens on Chrome is another proof.

You get the error only on ws@>=3.3.3 because it is the first version that does not swallow the net.Socket errors.

[jkasun]

Hi I had the same issue,
According to websocket specification, it is necessary to send a close frame before disconnecting from the server. That's why after properly closing the socket with sending the close frame this error didn't occur. as @lpinca mentioned.

window.addEventListener('beforeunload', function () { ws.close(); });

So I guess its fair to emit an error that point. Make sure you close the websocket from the client side before disconnecting, this could happen due to closing the browser or refreshing

[lpinca]

@jkasun well all browsers sends a close frame when you refresh the page if you didn't already close the WebSocket. The problem is that Chrome closes the TCP socket while the server response is on the wire.

To be honest I don't know why it doesn't do the same when the WebSocket is closed from a listener of the 'beforeunload' event. Maybe it does but the small timing difference is sufficient (at least for the above example) to prevent the issue from happening.

[pbarbiero]

So we started having a production server crashing today after a recent deployment moved the latest version of ws to 3.3.3 as well. We do not use it directly, but instead use it indirectly through socket.io v2 and puppeteer. Looking at those repos (socket.io uses it inside of engine.io and engine.io-client) I see them attaching on('error', cb) and now I am confused.

My gut instinct is maybe puppeteer is doing something wrong and not socket.io since they do it a little differently than the examples in this issue show. Specifically, right here: https://github.com/GoogleChrome/puppeteer/blob/master/lib/Connection.js#L31

I would open a ticket against their repo, but I don't even know if they are doing anything wrong yet since they don't use it through a server, and I don't have the knowledge to rule out socket.io either (they attach an error handler here: https://github.com/socketio/engine.io/blob/master/lib/server.js#L380)

@lpinca I hate to bug you, but can you double check my analysis before I consider opening tickets elsewhere?

[lpinca]

Socket.IO seems to correctly add the 'error' listener on both the server and client.
Same is for puppeteer. I've only checked the latest versions though.

The read ECONNRESET error can be emitted also on the client not only the server.

[pbarbiero]

Thank you, so now I am stumped. If it throws on the client, how would it crash the node instance? Also, it wouldn't make it to the server logs either, right?

[lpinca]

I don't know how you are using it but if the client and server are two separate processes then ofc the server does not crash because of the client. From I what I can see puppeteer uses ws only as client.

[dieunguyen95]

please change version of ionic/app-scripts to 3.1.6. you can see:
https://github.com/ionic-team/ionic-app-scripts/blob/master/CHANGELOG.md

[ztiandan]

I got this issue too, it made node crash, how could i catch this error, make node does not crash

[Sat Feb 10 2018 17:34:06 GMT+0800 (CST)] [graceful:worker:4131:uncaughtException] throw error 1 times
{ Error: read ECONNRESET
    at _errnoException (util.js:1022:11)
    at TLSWrap.onread (net.js:615:25)
  code: 'ECONNRESET',
  errno: 'ECONNRESET',
  syscall: 'read',
  name: 'ECONNRESETError' }
Error: read ECONNRESET
    at _errnoException (util.js:1022:11)
    at TLSWrap.onread (net.js:615:25)

added onerror and onclose

this.onerror = function (err) {
   console.log('[svc_sdk] h5 websocket onerror:', err.message);
}
this.wsSock.onclose = this.onclose;
this.wsSock.onerror = this.onerror;

[idkRage]

just add onerror and onclose (inside of wss.on.('connection',...)) to your server :

ws.on('close', function(){
console.log("client closed connection!");
});

ws.on('error', function(e){
console.log("error!: " + e);
})

[idkRage]

:)

[dzek69]

This still happens on 4.0.0, adding both error and close event handlers on ws instance doesn't help.

[idkRage]

idk then. works fine for me.

my code(server):

const WebSocket = require('ws');

const wss = new WebSocket.Server({ port: 8080 });

wss.on('connection', function(ws) {
ws.on('message', function(message) {
console.log("msg: " + message);
});

ws.on('close', function(){
console.log("client closed connection!");
});

ws.on('error', function(e){
console.log("error!: " + e);
})

ws.send('something');
});

[idkRage]

hope it helps

[rufat]

Same for me. It was happening in every refresh. But after adding the event listener, it happens rarely and when you refresh too much in a second. But now, the client side says: ' 'WebSocket': Still in CONNECTING state.'.

Problem isn't solved totally.

Chrome Canary v66.0.3345.0
NodeJS v8.9.3 / ws library v4.0.0

the listener:
window.addEventListener('beforeunload', function () { ws.close(); });

[lpinca]

Everyone please add a listener for the 'error' event on the WebSocket instance.

// Server example.
wss.on('connection', (ws) => {
  ws.on('error', (err) => {
    // Ignore network errors like `ECONNRESET`, `EPIPE`, etc.
    if (err.errno) return;
    throw err;
  });
});

// Client example.
const ws = new WebSocket(url);

ws.on('error', handleError);

[itaysabato]

@lpinca If the error is never re-thrown (not just when there is an errno), can it have a negative effect?

[lpinca]

No, there are no side effects, the WebSocket is always closed when an error occurs, so you can use a noop as listener if you don't care about errors.

ws.on('error', () => {});

[kahluagenie]

Spent some time figuring this one out in my unit tests. I had code like this to finish the tests:

wsClient.close();
server.close(done);

The problem is that the server tries to close before the event from the client reaches it, hence forcing the close connection and causing the error.

So I'm doing this now:

        if (server.clients.size) {
            server.clients.forEach(ws => {
                ws.on('close', () => {
                    if (!server.clients.size) { // last client
                        server.close(done);
                    }
                });

                wsClient.close();
            });
        } else {
            server.close(done);
        }

[lpinca]

I didn't expect to create so much disruption by re-emitting net.Socket errors as I assumed that everyone already had 'error' listeners set up but I was wrong and it seems that these errors are ignored anyway, so I'm thinking to not re-emit them again, restoring behavior of ws@<=3.3.2.

What do you think? It would be great if you could test upcoming changes installing from GitHub.

npm i websockets/ws#subclass/stream-writable

Thanks.

[dzek69]

I'll test it again, but for me the problem is that even when there is an error listener - ws still crashes.

I prefer it to re-emit the errors, just make sure I'll be able to catch them

[cluxter]

What helped me to solve this issue was @lpinca comment here: https://github.com/websockets/ws/issues/1256#issuecomment-364988689

I was adding the ws.on("error", function (err) { console.error("Error: " + err) } ) at the root of the server code when it should actually be inside the wss.on("connection", ... ) part!

The indentation in the comment I provided helped me to see that my code was not in the right scope. Some other comments didn't provide indentation in their examples and this is why I misread them.

Problem solved, thanks a lot!

[dzek69]

Looks like you're right.

According to your message it seems like error is happening on connected client instance, not whole server.
That ws and wss wasn't distinguishable enough in the example. That's why I am always using client as name inside connection.

So to resolve the issue we should use:

ws.on("connection", (client) => {
  client.on("error", () => {
    // do something or ignore
  });
});

This is perfectly fine, I'll always vote for explicit code - this just needs to be explained in README.

[SinvalMAO]

Most people with problems here are putting the error event in wss that references the instance of WebSocketServer, but you should put in the instance created when the client opens a connection, this happens because you should have tried to listen after the connection was already closed thus taking a DENY of the server, if it does not treat the generated exemption, it will be sent to the parent that is the own generating a shutdown. (Less technical)

'use strict';

const WebSocket = require('ws').Server;

const SERVER_PORT = 8081;
const wss = new WebSocket({port: SERVER_PORT});
const connections = [];

wss.on('connection', handleConnection);

wss.on('error', () => {
    console.log('NOT WORK!!!');
});

function handleConnection(ws) {
    console.log('New Connection');
    connections.push(ws);
    console.log(connections.length);

    ws.on('error', () => {
        console.log('WORK! =)');
    });

    ws.on('close', function() {
        console.log('Connection closed');
        let position = connections.indexOf(ws);
        connections.splice(position, 1);
    });
}

function broadcast(data) {
    for (let connection in connections) {
        try {
            connections[connection].send(data);
        }catch(e) {
            console.log(e);
        }
    }
}

setInterval(() => broadcast(new Buffer('A')), 2000);

[Hypnosphi]

ws.on('error', (err) => {
 // Ignore network errors like `ECONNRESET`, `EPIPE`, etc.
 if (err.errno) return;
 throw err;
});

@lpinca in 4.0, it should be rewritten to this, right?

ws.on('error', ({error}) => {
 // Ignore network errors like `ECONNRESET`, `EPIPE`, etc.
 if (error.errno) return;
 throw error;
});

[lpinca]

@Hypnosphi only if you are using onerror as per https://github.com/websockets/ws/commit/63e275e75f8ab8f4385e7ef104a74c582cd0c793.

[lpinca]

Closing as we are no longer re-emitting net.Socket errors on version 5.

[RebelSyntax]

First, thanks for the great library. I am testing it out on a really cool IoT prototype. I love that it does one thing good. I ran into this error the last couple days. I am guessing because my headless client timed out it's http connection for whatever reason (don't have ping pong going yet, just a reconnect on the heartbeat.)

Anyways, I had already set up the error handler in connection event for the connected socket. So I was a little surprised that the server halted on this error. I realize that the error is not going to be re-emitted, but I guess I am confused by that. Why wouldn't that error get handled the way it was intended?

[EvanCarroll]

@kahluagenie here because of Devskiller interview assessment?

---

I wonder how many people are coming here because of bad spec tests in timed Devskiller assessments? Here is my theory: I think the ws team is getting played. =P Devskiller's package.json version-fixed to 3.3.3 which has the issue, and all we have three hours to figure out why we're too stupid to scaffold a Web Socket interface (beginner level stuff). The real reason is because they mirror in their .spec.ts a very subtle bug that's only valid on archaic versions of ws

wsClient.close();
server.close();

or,

ws.close()
wsServer.close()

You have three hours to scour and find the GitHub issue before you get the senior level remote-coffee-shop Node.JS guru position. If you found this issue. Congrats. You won.

The funny thing I read this and I wonder if everyone is making this honest mistake, or if we're all here for the same malformed spec/wsServer-api.spec.js. =P

https://devskiller.com/coding-tests/middle-javascript-developer-node-js-server-side-step-tracker/

[kahluagenie]

Haha, negative. I was using ws for a project of mine, so was figuring stuff out when I ran across the issue. All good now though!

[haroldiedema]

Apologies for commenting on an old issue, but this is still a problem.

Error: write EPIPE
    at afterWriteDispatched (internal/stream_base_commons.js:156:25)
    at writevGeneric (internal/stream_base_commons.js:139:3)
    at Socket._writeGeneric (net.js:783:11)
    at Socket._writev (net.js:792:8)
    at doWrite (internal/streams/writable.js:375:12)
    at clearBuffer (internal/streams/writable.js:521:5)
    at Socket.Writable.uncork (internal/streams/writable.js:317:7)
    at Sender.sendFrame (/home/harold/projects/mercury/server/mercury.js:34612:22)
    at Sender.send (/home/harold/projects/mercury/server/mercury.js:34507:12)
    at WebSocket.send (/home/harold/projects/mercury/server/mercury.js:35382:18)

Whenever a client disconnects _while_ a large amount of data is being sent, I consistently get full server crashes.
Everywhere in my code, I've added listeners to the error event where possible; on the http server, request and response object, the socket server and each individual client. I've even gone as far as ws._socket.on('error', () => ...), but to no avail.

Using ws 7.4.2, running on Node v14.15.4 on Ubuntu 20.04.

I'm actually unsure if this is a node internal error or has something to do with this library, since I also get ECONNRESET errors that I can't seem to catch from time to time.

edit:

Right as I finished up writing this post, I figured the listener was attached too late to the error event. My server runs inside a worker thread and the issue only seems to occur when my CPU load is reasonably high. The crash happened when the HTTP-server connection event is fired, then the client disconnects, and _then_ the request _should be_ fired. It's not a problem with this library, but the fact the error event is emitted from the socket _before_ an event listener is attached to it.

For anyone running into similar issues: The fix for me was to attach a listener to the error event _immediately_ as the client connects, and not a 'tick' later, because the client disconnects _after_ the connection event, but _before_ the upgrade event.

So, instead of:

httpServer.on('upgrade', (request: IncomingMessage, socket: Socket, head: Buffer) => {
    socket.on('error', () => console.log('poof!'));
});

All you have to do is bind the listener during the connection event.

httpServer.on('connection', (socket: Socket) => {
    socket.on('error', () => console.log('poof!'));
});

[cluxter]

@haroldiedema Could you please provide us with a full proof of concept that makes your server crash?

[haroldiedema]

@cluxter Sorry, I don't think I can. It was triggered by a combination of circumstances (high CPU load, multiple active connections, etc.) The problem was on my end.

All I had to do was _immediately_ attach a listener to the error event on the socket when the connection event got fired on the http server (from nodejs itself). Most documentation / similar reports I've seen regarding this issue all say the same: Add an error listener to the client, but nobody explicitly mentions to do it during the connection event.

The problem specifically is when the client connects (page is loading), and _before_ the HTTP server request event got fired, the client disconnects, due to a page reload or closing of the browser tab. This consistently crashes the server because the request event wasn't fired yet, so no listener was attached to the error event :stuck_out_tongue:

Anyway, like I said, it's not a problem with the 'ws' library. People running into this issue should simply attach the error listener immediately when the client connects, and not a bit later to prevent these kinds of 'race conditions'.

[lpinca]

@haroldiedema that listener is already added by Node.js as per https://github.com/nodejs/node/blob/v15.6.0/lib/_http_server.js#L498.