Hard to catch error on proxying in Node.js 10

[kmannislands]

• 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.

[alexander-akait]

@kmannislands Can you create minimum reproducible test repo? It is hard to fix without this and maybe never will be fixed without this (so issue will be closed)

[kmannislands]

@evilebottnawi Minimal repro is more effort here than submitting a PR.

I modified my ticket to include the specific code that needs to be changed and how. Not sure what your policy on exception handling/logging is.

I'm sure you'll have others trip over this as they go to 10.x

[alexander-akait]

@kmannislands Fatal error should be don't crash webpack-dev-server so we will accept a PR, but we need test case or minimum reproducible test repo to unsure we fix this problem and avoid regressions in future

[alexander-akait]

@kmannislands we need minimum reproducible test repo for manual testing (hard to testing), it is increase speed for solving problem

[mjrussell]

Can confirm we are also experiencing this error proxying websockets when the server doesn't gracefully respond to the connection or it closes abruptly. Only happens on Node 10.x

[alexander-akait]

@mjrussell can you create minimum reproducible test repo?

[alexander-akait]

Friendly ping guys

[cha0s]

Yes it's still an issue.

No I can't produce a minimal repro, concurrency is hard.

Please just fix it ❤️

[alexander-akait]

@cha0s it is hard to fix something in anything, can you provide screen this about problem?

[mathieumg]

@evilebottnawi It seems @kmannislands provided the console output of the error, so pretty much equivalent to a screenshot. Basically, you start the Webpack bundler with proxying, and if any client disconnects it crashes the whole process for the reason mentioned in the original issue, above:

Happens on Node v10.x.

[hiroppy]

@mathieumg Only Node v10.x? Node.js v12 has been released today so I want to know what occurs using v12. Thanks.

[mathieumg]

@hiroppy I presume it will be the same on v12, but I can install it and test it too. Either way, this still needs to be fixed for v10.

[mathieumg]

@hiroppy A separate bug prevents me from testing it in v12, but I'm not on the latest version of the Dev Server either:

Starting the development server...

internal/buffer.js:928
class FastBuffer extends Uint8Array {}
^

RangeError: Invalid typed array length: -4095
    at new Uint8Array (<anonymous>)
    at new FastBuffer (internal/buffer.js:928:1)
    at Handle.onStreamRead [as onread] (internal/stream_base_commons.js:165:17)
    at Stream.<anonymous> (/Users/mathieumg/project/frontend/node_modules/handle-thing/lib/handle.js:120:12)
    at Stream.emit (events.js:201:15)
    at endReadableNT (/Users/mathieumg/project/frontend/node_modules/readable-stream/lib/_stream_readable.js:1010:12)
    at processTicksAndRejections (internal/process/task_queues.js:83:17)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

[hiroppy]

see nodejs/node#24097

OK, let's talk about this in another issue.

[hiroppy]

@mathieumg Could you submit a reproducible repo?

[mathieumg]

Could you submit a reproducible repo?

Yes I can try to muster up a repro repo when I have a spare second sometime this week!

[dhardtke]

I tried reproducing this error in a minimal repo, but I can't. Tried with NodeJS 10, 11 and 12. NodeJS 12 doesn't work with WDS though (getting a weird error message different than the one posted above - maybe because I'm on windows).

Maybe you guys can use the repo I created. I have definitely seen this behavior in our project using NodeJS 11.x as well:

https://github.com/dhardtke/wds-ws-proxy-crash

[kmannislands]

IIRC, this also may have depended on a specific chrome version. Perhaps chrome has changed its behavior again.

To clarify my original ticket, this is non-blocking because WDS allows users to pass through config to the proxy package. So if anyone reading this is blocked, all you need to do is add an error handler on upgrade to the WDS config in your webpack config:

{
  // ... other config
  devServer: {
    proxy: {
      '/your_path': {
        target: 'https://localhost:XXX',
        ws: true,

        // Add this:
        onError(err) {
          console.log('Suppressing WDS proxy upgrade error:', err);
        },
      },
    },
  },
}

[alexander-akait]

hm, i think we should add logger by default as described above, feel free to send a PR, it is easy to fix

[markmcdowell]

This has just started happening to me but I'm unsure of the change that caused it.

• Operating System: macOs 10.14.5
• Node Version: 10.15.3
• NPM Version: 6.9.0
• webpack Version: 4.31.0
• webpack-dev-server Version: 3.3.1

The onError addition above doesn't make a difference I still get the crash with:

events.js:174                                                                       
       throw er; // Unhandled 'error' event
       ^
Error: read ECONNRESET
     at TCP.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 process._tickCallback (internal/process/next_tick.js:63:19)

It happens at the point I try to open a websocket through the proxy (it doesn't always happen):

proxy: {
            "/api": {
                target: "http://localhost:8181",
            },
            '/api/realtime': {
                target: "ws://localhost:8181",
                ws: true
            },
            "/auth": {
                target: "http://localhost:8181",
            },
        }

[alexander-akait]

@markmcdowell can you create minimum reproducible test repo?

[alexander-akait]

@veloek maybe you can provide full stack trace of the error?

[veloek]

@alexander-akait This is the error message printed to the console when I recycle the IIS app pool causing WDS to crash:

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

Error: read ECONNRESET
    at TCP.onStreamRead (internal/stream_base_commons.js:209:20)
Emitted 'error' event on Socket instance at:
    at emitErrorNT (internal/streams/destroy.js:106:8)
    at emitErrorCloseNT (internal/streams/destroy.js:74:3)
    at processTicksAndRejections (internal/process/task_queues.js:82:21) {
  errno: -4077,
  code: 'ECONNRESET',
  syscall: 'read'
}

[0x53A]

I'm also regularly seeing my webpack-dev-server die until a restart with the same callstack:

[0] webpack 5.52.1 compiled successfully in 10501 ms
[0] <e> [webpack-dev-server] [HPM] Error occurred while proxying request vm10:8081/bridge to undefined [ECONNRESET] (https://nodejs.org/api/errors.html#errors_common_system_errors)
[0] <i> [webpack-dev-middleware] wait until bundle finished: /carousel-view-3d/live?view=ProgressB240
[0] <i> [webpack-dev-middleware] wait until bundle finished: /carousel-view-3d/live?view=ProgressB240
[0] <i> [webpack-dev-middleware] wait until bundle finished: /static/js/carouselview-3d-worker.da5a157d1b114be000e3.hot-update.json
[0] <i> [webpack-dev-server] [HPM] Upgrading to WebSocket
[0] <e> [webpack-dev-server] [HPM] Error occurred while proxying request vm10:8081/bridge to undefined [ECONNREFUSED] (https://nodejs.org/api/errors.html#errors_common_system_errors)
[0] node:events:371
[0]       throw er; // Unhandled 'error' event
[0]       ^
[0]
[0] Error: read ECONNRESET
[0]     at TLSWrap.onStreamRead (node:internal/stream_base_commons:220:20)
[0] Emitted 'error' event on TLSSocket instance at:
[0]     at emitErrorNT (node:internal/streams/destroy:157:8)
[0]     at emitErrorCloseNT (node:internal/streams/destroy:122:3)
[0]     at processTicksAndRejections (node:internal/process/task_queues:83:21) {
[0]   errno: -4077,
[0]   code: 'ECONNRESET',
[0]   syscall: 'read'
[0] }

my setup looks like this and the error happens whenever the api server is restarted:

[alexander-akait]

@0x53A Can you provide example to reproduce? What is webpack-dev-server version? I think it should be solved for v4, if not, we need investigate, fix should be easy, just need understand where it happens

[0x53A]

@alexander-akait

I was on

webpack-dev-server: 4.2.0
http-proxy-middleware: 2.0.1

nodejs: v16.8.0

and updated to webpack-dev-server: 4.3.1.

With the latest version it does seem to work, thank you!
I didn't update before complaining because the issue was still open, sorry.

scratch that, just happened again with 4.3.1.

I'll see if I can create a repro tomorrow.

Both the communication from browser to WDS and WDS to api-server is https.
It seems to be easier (or only?) reproducible with Edge instead of Firefox.

---

Adding

         onError: err => {
              console.log('Suppressing WDS proxy upgrade error:', err);
            },

DOES help, then I can see

[0] Suppressing WDS proxy upgrade error: Error: read ECONNRESET
[0]     at TLSWrap.onStreamRead (node:internal/stream_base_commons:220:20) {
[0]   errno: -4077,
[0]   code: 'ECONNRESET',
[0]   syscall: 'read'
[0] }

in the log and WDS continues serving requests.

I think the real fix would be nodejs/node#36111.

[alexander-akait]

@0x53A Anyway, will be great to look there it happens, because it allows us do better checks, will be glad to debug

[markusschaber]

@alexander-akait

I think the real fix would be nodejs/node#36111.

I'm not sure about that - I run into this error without any TLS involved (as everything is on localhost):

√ Compiled successfully.
<e> [webpack-dev-server] [HPM] Error occurred while proxying request localhost:4200/api/JsonRpc to undefined [ECONNREFUSED] (https://nodejs.org/api/errors.html#errors_common_system_errors)  
node:events:498
      throw er; // Unhandled 'error' event
      ^

Error: read ECONNRESET
    at TCP.onStreamRead (node:internal/stream_base_commons:217:20)
Emitted 'error' event on Socket instance at:
    at emitErrorNT (node:internal/streams/destroy:157:8)
    at emitErrorCloseNT (node:internal/streams/destroy:122:3)
    at processTicksAndRejections (node:internal/process/task_queues:83:21) {
  errno: -4077,
  code: 'ECONNRESET',
  syscall: 'read'
}

[Delagen]

Issue about 3 years, but still patch lives in my sources in postinstall action

diff --git a/node_modules/webpack-dev-server/lib/Server.js b/node_modules/webpack-dev-server/lib/Server.js
--- a/node_modules/webpack-dev-server/lib/Server.js
+++ b/node_modules/webpack-dev-server/lib/Server.js
@@ -1840,7 +1840,10 @@ class Server {
       (this.server).on(
         "upgrade",
         /** @type {RequestHandler & { upgrade: NonNullable<RequestHandler["upgrade"]> }} */
-        (webSocketProxy).upgrade
+        (req, socket, ...args) => {
+            socket.on('error', (err) => console.error(err));
+            return webSocketProxy.upgrade(req, socket, ...args);
+        }
       );
     }, this);
   }

@alexander-akait Why do we still need this ugly hack?

Dev-Server: 4.7.3
NodeJS: 17.9.0

[alexander-akait]

@Delagen Can you send a PR? Will be great to have test case, I tried to reproduce but no luck

[chimurai]

Hi @alexander-akait.

Might have found the issue while working on the next version of http-proxy-middleware.

I noticed http-proxy has an undocumented econnreset event.

Added a default handler to econnreset; Hopefully this will prevent the server from crashing. 🤞
Haven't been able to replicate this bug as well.

Just published http-proxy-middleware@2.0.5

Curious to hear if v2.0.5 fixes this gnarly bug

[alexander-akait]

@chimurai Big thanks

@Delagen Can you test it without hack and with a new version?

[maknapp]

I can confirm this fixes the issue with webpack-dev-server@4.7.4 and http-proxy-middleware@2.0.5

[Delagen]

@chimurai Thanks for fast fix. It works now.

[alexander-akait]

Great! I think we can close it, anyway if you faced with this issue - feel free to open an issue, hope we covered all cases

[chimurai]

Glad to hear the fix is working.

Thanks @alexander-akait for helping out in this thread. 💪

[maknapp]

It seems I spoke too soon. I am still getting the error. Somehow the ECONNRESET error is thrown without being caught. I see the error handler in http-proxy-middleware being added, but it does not catch anything.

[maknapp]

@chimurai The econnreset event appears to be only for http and not websocket. The uncaught error causing the crash when websockets are involved is an "error" event, not an "econnreset" event. Looking at https://github.com/http-party/node-http-proxy/blob/master/lib/http-proxy/passes/ws-incoming.js, the uncaught "error" event is coming from socket. There is a proxySocket.on("error") that re-emits to the "error" handler in http-proxy-middleware, but there is none for socket.on("error").

The only way I see to catch the error is to add a default proxyReqWs handler that then adds an error handler to socket. The problem with that is someone with a proxyReqWs handler will not know to handle socket errors.

[maknapp]

In http-proxy, there is an error handler on socket that is added only after the websocket upgrade. The ECONNRESET error causing the crash occurs before the upgrade (since there is no destination) and throws an error that nobody catches. This seems to be fixed by http-party/node-http-proxy#1439, but it looks like there is nobody there to merge it.

The following in http-proxy-middleware fixes all crashing for me and allows users to still have an onProxyReqWs handler:

proxy.on('econnreset', (err, req, res, target) => {
    logger.error(`[HPM] ECONNRESET: %s`, err.message);
});
proxy.on('proxyReqWs', (proxyReq, req, socket, options, head) => {
    socket.on('error', (error) => {
        logger.error(`[HPM] WebSocket error: %s`, error.message);
    });
});

Note that I changed the econnreset handler to log err.message, not err since it printed the object.

[chimurai]

Appreciate for your feedback @maknapp!

Published http-proxy-middleware@2.0.6 with your suggestion.

Let me know if this fix helps.

Bit challenging without a reproduction of the bug 😬

[veloek]

@chimurai I can confirm that version 2.0.6 solves the issue. It used to crash for me every time the app pool recycled (IIS), but now it just logs this and keeps running:

[webpack-dev-server] [HPM] WebSocket error: Error: read ECONNRESET
    at TCP.onStreamRead (node:internal/stream_base_commons:217:20) {
  errno: -4077,
  code: 'ECONNRESET',
  syscall: 'read'
}

So now I can remove the workaround, great work! 🙂

[maknapp]

v2.0.6 is working for me as well. Thanks @chimurai!

[VitaliyR]

Is there any way to silent exactly these messages?

[alexander-akait]

You can setup logging for http-proxy-middleware, there are options for it