Description
- Operating System: macOs 10.14.1
- Node Version: 10.15.0
- NPM Version: yarn 1.13.0
- webpack Version: 4.23.1
- webpack-dev-server Version: 3.1.14
- This is a bug
- This is a modification request
Code
The bug I'm reporting here showed up after switching to Node js v10.x. I believe there are differences in how the net core package raises exceptions/policy toward unhandled error events.
To repro, you need to proxy a connection that uses WebSockets, allow it to connect using chrome, then restart the target server of the proxy, for example with Nodemon. Want to avoid creating a minimal repo of this if possible since it's a bit comple and there seems to be a nice trail of ticket/changelogs.
This will cause the WDS process to die due to unhandled error event from ECONNRESET with the following stack trace:
HPM] Error occurred while trying to proxy request /foo from localhost:9090 to https://localhost:3333 (ECONNREFUSED) (https://nodejs.org/api/errors.html#errors_common_system_errors)
events.js:167
throw er; // Unhandled 'error' event
^
Error: read ECONNRESET
at TLSWrap.onStreamRead (internal/stream_base_commons.js:111:27)
Emitted 'error' event at:
at emitErrorNT (internal/streams/destroy.js:82:8)
at emitErrorAndCloseNT (internal/streams/destroy.js:50:3)
at Object.apply (/Users/kieran/work/Atlas/ui/node_modules/harmony-reflect/reflect.js:2064:37)
at process._tickCallback (internal/process/next_tick.js:63:19)
Before upgrading to node js 10, I believe something similar was happening but the exception wasn't fatal.
After some debugging/ticket searching it seems that what changed is that it is now an error for the https module to encounter an exception without a registered handler, which impacts the behavior of your dependency node-http-proxy, like this ticket describes.
Debugging the error, I discovered that WDS proxy config is really http-proxy-middleware config, so it is possible to suppress the error and keep the server up by modifying config by adding an onError handler to proxy config for future readers, however this was fairly difficult to debug and ideally this package would register some sort of handler for this exception so that it isn't fatal.
Willing to submit a PR, the change is very straight-forward. Involve modifying this code:
// Proxy websockets without the initial http request
// https://github.com/chimurai/http-proxy-middleware#external-websocket-upgrade
websocketProxies.forEach(function (wsProxy) {
this.listeningApp.on('upgrade', wsProxy.upgrade);
}, this);To be like:
// Proxy websockets without the initial http request
// https://github.com/chimurai/http-proxy-middleware#external-websocket-upgrade
websocketProxies.forEach(function (wsProxy) {
this.listeningApp.on('upgrade', (req, socket, ...args) => {
socket.on('error', (err) => console.error(err));
return wsProxy.upgrade(req, socket, ...args);
};
}, this);Expected Behavior
The server should stay alive when a client (on either side of the proxy) disconnects.
Actual Behavior
Server dies due to no handler for ECONNRESET
For Bugs; How can we reproduce the behavior?
Use Node 10.