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

Release #43 throws an error after a couple of refreshes #106

Closed
klaasman opened this issue Dec 19, 2017 · 6 comments
Closed

Release #43 throws an error after a couple of refreshes #106

klaasman opened this issue Dec 19, 2017 · 6 comments
Assignees
Labels

Comments

@klaasman
Copy link
Contributor

Waiting for file changes...

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

Error: read ECONNRESET
    at _errnoException (util.js:1024:11)
    at TCP.onread (net.js:615:25)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Trace with longjohn:

Waiting for file changes...

Error: read ECONNRESET
    at _errnoException (util.js:1024:11)
    at TCP.onread (net.js:615:25)
---------------------------------------------
    at Socket.Readable.on (_stream_readable.js:772:35)
    at Ultron.on (/Users/klandsman/Sites/kaliber/velux/velux-always-on/node_modules/ultron/index.js:42:11)
    at WebSocket.setSocket (/Users/klandsman/Sites/kaliber/velux/velux-always-on/node_modules/ws/lib/WebSocket.js:131:18)
    at WebSocket.initAsServerClient (/Users/klandsman/Sites/kaliber/velux/velux-always-on/node_modules/ws/lib/WebSocket.js:465:8)
    at new WebSocket (/Users/klandsman/Sites/kaliber/velux/velux-always-on/node_modules/ws/lib/WebSocket.js:70:26)
    at WebSocketServer.completeUpgrade (/Users/klandsman/Sites/kaliber/velux/velux-always-on/node_modules/ws/lib/WebSocketServer.js:275:20)
    at WebSocketServer.handleUpgrade (/Users/klandsman/Sites/kaliber/velux/velux-always-on/node_modules/ws/lib/WebSocketServer.js:227:10)
    at Server.WebSocketServer._ultron.on (/Users/klandsman/Sites/kaliber/velux/velux-always-on/node_modules/ws/lib/WebSocketServer.js:86:14)
---------------------------------------------
    at Ultron.on (/Users/klandsman/Sites/kaliber/velux/velux-always-on/node_modules/ultron/index.js:42:11)
    at new WebSocketServer (/Users/klandsman/Sites/kaliber/velux/velux-always-on/node_modules/ws/lib/WebSocketServer.js:85:20)
    at startWebSocketServer (/Users/klandsman/Sites/kaliber/velux/velux-always-on/node_modules/@kaliber/build/webpack-plugins/websocket-communication-plugin.js:84:15)
    at <anonymous>
    at process._tickCallback (internal/process/next_tick.js:188:7)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
error Command failed with signal "SIGTERM".
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
ERROR: "watch" exited with 1.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
@EECOLOR
Copy link
Member

EECOLOR commented Dec 19, 2017

request/request#2161

@klaasman
Copy link
Contributor Author

klaasman commented Jan 8, 2018

Temporary fix:

config/dev.js:

// Temporary fix, remove when https://github.com/kaliberjs/build/issues/106 is resolved
process.on('uncaughtException', (err) => { if (err.code !== 'ECONNRESET') throw err })

module.exports = { ... }

@dennispassway
Copy link
Contributor

Will this be fixed?

@EECOLOR
Copy link
Member

EECOLOR commented Feb 24, 2018

nodejs/node#14102 suggest a missing socket.on('error', handleError); might be the cause. However, nodejs/node#14102 shows that there is a more fundamental problem with nodejs itself.

websockets/ws#1256 suggests ws.on('error', () => console.log('errored')); It's unclear however if that is attached to the ws.Server instance.

We could however add a line in https://github.com/kaliberjs/build/blob/master/library/webpack-plugins/websocket-communication-plugin.js

const wss = new ws.Server({ port })
wss.on('error', () => ...)

Please try that on a machine where you can easily reproduce the problem and see if the error moves through that method.

A suggestion for a workaround in case that doesn't work, add the following to the client:

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

Note that this only has effect on Chrome, but according to the reports Chrome > 63 is the browser affected most.

A related quote:

@endel the error is not a bug in the library and #1256 (comment) 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.

More possible fixes (I think these are the most promising):

// 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);

Lastly (from websockets/ws#1256):

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.

@EECOLOR
Copy link
Member

EECOLOR commented Feb 24, 2018

So yeah, I think we now have enough information to fix it. Let's test it next week.

@lpinca
Copy link

lpinca commented Feb 24, 2018

You definitely need an 'error' listener on the the WebSocket instance. Even if we decide to not re-emit the socket errors again, there are others errors that can be emitted.

Errors can also be emitted on the WebSocketServer instance (WebSocket.Server) but those are re-emitted from the underlying HTTP(S) server and unrelated to this bug report.

Don't use process.on('uncaughtException') as it causes more issues than it solves.

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

No branches or pull requests

4 participants