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

Errors in prepareProxyRequest not handled for websockets #822

Open
2 tasks done
nwalters512 opened this issue Sep 4, 2022 · 0 comments · May be fixed by #823
Open
2 tasks done

Errors in prepareProxyRequest not handled for websockets #822

nwalters512 opened this issue Sep 4, 2022 · 0 comments · May be fixed by #823

Comments

@nwalters512
Copy link

nwalters512 commented Sep 4, 2022

Checks

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:

this.handleUpgrade(req, socket, head);

server.on('upgrade', this.handleUpgrade);

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:

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?

macOS 12.5.1; Node v16.17.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant