Repository: chimurai/http-proxy-middleware (ts)

Issue:
Errors in prepareProxyRequest not handled for websockets
### Checks

- [X] I understand project setup issues should be asked on [StackOverflow](https://stackoverflow.com/questions/tagged/http-proxy-middleware) or in [GitHub Discussions](https://github.com/chimurai/http-proxy-middleware/discussions).
- [X] I updated to latest `http-proxy-middleware`.

### Describe the bug (be clear and concise)

The callsites of `handleUpgrade` do not handle errors thrown from `prepareProxyRequest`. For a user, this means that any error thrown in `router` or `pathRewrite` will result in an `unhandledRejection`.

We can see that the two call sites for `handleUpgrade` neither `await` the returned promise, nor use `.catch()` to handle errors:

https://github.com/chimurai/http-proxy-middleware/blob/79c9885eb2b8ed1bc201182175b733b738873f5e/src/http-proxy-middleware.ts#L34

https://github.com/chimurai/http-proxy-middleware/blob/79c9885eb2b8ed1bc201182175b733b738873f5e/src/http-proxy-middleware.ts#L88

### Step-by-step reproduction instructions

Add a `router` or `pathRewrite` function that throws an error, then try to make a websocket request. Observe that the error is not handled. E.g., if you add an `onError` handler to the proxy config, it is not called, and if you `process.on('unhandledRejection', (e) => console.error(e))`, that error is logged.

### Expected behavior (be clear and concise)

`onError` should be invoked so that I can handle the error appropriately.

### How is http-proxy-middleware used in your project?

It is installed as a direct dependency.

### What http-proxy-middleware configuration are you using?

Here's a pared-down version:

```typescript
const options = {
  router: async () => {
    throw new Error('testing');
  },
  onError: (err, req, res) => {
    // Not called for websocket requests
    res.status(500).send();
  },
};
```

### What OS/version and node/version are you seeing the problem?

```shell
macOS 12.5.1; Node v16.17.0
```

Pull request description:
fix(websocket): handle errors in handleUpgrade
<!--- Provide a general summary of your changes in the Title above -->

## Description

When there is an error from user-provided code (`pathFilter`, `rewritePath`, or `router`), that error is now handled and exposed to the user via `proxy.emit('error', ...)`.

`error-response-plugin` was also updated to handle the fact that it might receive a socket. If it does, it will call `socket.destroy()`.

## Motivation and Context

See #822 for a description of the issue being fixed. This PR closes #822.

## How has this been tested?

I added a new test case. It fails on master, and works correctly with my new changes.

## Types of changes

I'm split on whether or not this should be considered a breaking change. Folks could have written code assuming that the error handler would always get a `Response` as the third argument, whereas now it would get a `Socket`. That said, the TypeScript types from `http-proxy` do reflect that that could be a `Socket`, so it shouldn't come as too much of a surprise.

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)

## Checklist:

<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->

- [ ] My change requires a change to the documentation.
- [ ] I have updated the documentation accordingly.


<a href="https://gitpod.io/#https://github.com/chimurai/http-proxy-middleware/pull/823"><img src="https://gitpod.io/button/open-in-gitpod.svg"/></a>



Patch:
diff --git a/src/http-proxy-middleware.ts b/src/http-proxy-middleware.ts
index 54f054f..597c3cb 100644
--- a/src/http-proxy-middleware.ts
+++ b/src/http-proxy-middleware.ts
@@ -98,10 +98,16 @@ export class HttpProxyMiddleware<TReq, TRes> {
   };
 
   private handleUpgrade = async (req: http.IncomingMessage, socket: net.Socket, head: Buffer) => {
-    if (this.shouldProxy(this.proxyOptions.pathFilter, req)) {
-      const activeProxyOptions = await this.prepareProxyRequest(req);
-      this.proxy.ws(req, socket, head, activeProxyOptions);
-      debug('server upgrade event received. Proxying WebSocket');
+    try {
+      if (this.shouldProxy(this.proxyOptions.pathFilter, req)) {
+        const activeProxyOptions = await this.prepareProxyRequest(req);
+        this.proxy.ws(req, socket, head, activeProxyOptions);
+        debug('server upgrade event received. Proxying WebSocket');
+      }
+    } catch (err) {
+      // This error does not include the URL as the fourth argument as we won't
+      // have the URL if `this.prepareProxyRequest` throws an error.
+      this.proxy.emit('error', err, req, socket);
     }
   };
 
diff --git a/src/plugins/default/error-response-plugin.ts b/src/plugins/default/error-response-plugin.ts
index 4d9113d..f13a815 100644
--- a/src/plugins/default/error-response-plugin.ts
+++ b/src/plugins/default/error-response-plugin.ts
@@ -1,6 +1,16 @@
+import type * as http from 'http';
+import type { Socket } from 'net';
 import { getStatusCode } from '../../status-code';
 import { Plugin } from '../../types';
 
+function isResponseLike(obj: any): obj is http.ServerResponse {
+  return obj && typeof obj.writeHead === 'function';
+}
+
+function isSocketLike(obj: any): obj is Socket {
+  return obj && typeof obj.write === 'function' && !('writeHead' in obj);
+}
+
 export const errorResponsePlugin: Plugin = (proxyServer, options) => {
   proxyServer.on('error', (err, req, res, target?) => {
     // Re-throw error. Not recoverable since req & res are empty.
@@ -8,12 +18,16 @@ export const errorResponsePlugin: Plugin = (proxyServer, options) => {
       throw err; // "Error: Must provide a proper URL as target"
     }
 
-    if ('writeHead' in res && !res.headersSent) {
-      const statusCode = getStatusCode((err as unknown as any).code);
-      res.writeHead(statusCode);
-    }
+    if (isResponseLike(res)) {
+      if (!res.headersSent) {
+        const statusCode = getStatusCode((err as unknown as any).code);
+        res.writeHead(statusCode);
+      }
 
-    const host = req.headers && req.headers.host;
-    res.end(`Error occurred while trying to proxy: ${host}${req.url}`);
+      const host = req.headers && req.headers.host;
+      res.end(`Error occurred while trying to proxy: ${host}${req.url}`);
+    } else if (isSocketLike(res)) {
+      res.destroy();
+    }
   });
 };


Test patch:
diff --git a/test/e2e/websocket.spec.ts b/test/e2e/websocket.spec.ts
index 9dbd985..7dde476 100644
--- a/test/e2e/websocket.spec.ts
+++ b/test/e2e/websocket.spec.ts
@@ -97,15 +97,14 @@ describe('E2E WebSocket proxy', () => {
 
   describe('with router and pathRewrite', () => {
     beforeEach(() => {
-      // override
-      proxyServer = createApp(
+      const proxyMiddleware = createProxyMiddleware({
         // cSpell:ignore notworkinghost
-        createProxyMiddleware({
-          target: 'ws://notworkinghost:6789',
-          router: { '/socket': `ws://localhost:${WS_SERVER_PORT}` },
-          pathRewrite: { '^/socket': '' },
-        }),
-      ).listen(SERVER_PORT);
+        target: 'ws://notworkinghost:6789',
+        router: { '/socket': `ws://localhost:${WS_SERVER_PORT}` },
+        pathRewrite: { '^/socket': '' },
+      });
+
+      proxyServer = createApp(proxyMiddleware).listen(SERVER_PORT);
 
       proxyServer.on('upgrade', proxyMiddleware.upgrade);
     });
@@ -124,4 +123,29 @@ describe('E2E WebSocket proxy', () => {
       ws.send('foobar');
     });
   });
+
+  describe('with error in router', () => {
+    beforeEach(() => {
+      const proxyMiddleware = createProxyMiddleware({
+        // cSpell:ignore notworkinghost
+        target: `http://notworkinghost:6789`,
+        router: async () => {
+          throw new Error('error');
+        },
+      });
+
+      proxyServer = createApp(proxyMiddleware).listen(SERVER_PORT);
+
+      proxyServer.on('upgrade', proxyMiddleware.upgrade);
+    });
+
+    it('should handle error', (done) => {
+      ws = new WebSocket(`ws://localhost:${SERVER_PORT}/socket`);
+
+      ws.on('error', (err) => {
+        expect(err).toBeTruthy();
+        done();
+      });
+    });
+  });
 });