http-party / node-http-proxy

A full-featured http proxy for node.js
https://github.com/http-party/node-http-proxy
Other
13.87k stars 1.96k forks source link

Crash on unsuccessful websocket request #1468

Open jmattheis opened 4 years ago

jmattheis commented 4 years ago
const x = require('http-proxy');
const srv = x.createServer({ target: 'http://echo.websocket.org', ws: true, changeOrigin: true }).listen(5555);
srv.on('error', () => {
    console.log('err');
})

After connecting to a path that doesn't exist, the server crashes

$ wscat -c 'ws://localhost:5555/a'

Error:

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

Error: read ECONNRESET
    at TCP.onStreamRead (internal/stream_base_commons.js:207:27)
Emitted 'error' event on Socket instance at:
    at emitErrorNT (internal/streams/destroy.js:100:8)
    at emitErrorCloseNT (internal/streams/destroy.js:68:3)
    at processTicksAndRejections (internal/process/task_queues.js:80:21) {
  errno: -104,
  code: 'ECONNRESET',
  syscall: 'read'
}
carl-vbn commented 4 years ago

This is not a bug, according to README.md:

We do not do any error handling of messages passed between client and proxy, and messages passed between proxy and target, so it is recommended that you listen on errors and handle them.

So you have to listen to the 'error' event and do the handling yourself.

jmattheis commented 4 years ago

@carl-vbn Thanks for your answer. Could you give me an example on how to do this? I've added such a listener above. Still, my node process crashes/exits after an error occurs, and I'd have to restart my node server to get the proxy in a working state.

carl-vbn commented 4 years ago

I'm sorry I didn't properly look through your code. I ran it myself and got the same problem. From the stack trace it doesn't look like the error is thrown by http-proxy itself, so it might really be a bug. Sorry for overlooking part of your problem, I'll let you know if I find a solution.

fromtexas commented 3 years ago

proxy.on('proxyReqWs', (proxyReq, req, socket, options, head) => { socket.on('error', (error) => { console.error(error); }); }); ☝️ Helped me to solve the same problem

stephanrotolante commented 3 years ago

@jmattheis Dude, if its not too late try this out....

const proxyConfig = { target: { host: "localhost", port: 3000, }, ws: true, // Add this... no special wss configuration is required. };

const proxy = httpProxy.createProxyServer(proxyConfig); proxy.listen(5000);

hitting it with this link -> ws://localhost:5000/graphql

Hope this helps, I found it snooping through some issues #1041 (comment)

jmattheis commented 3 years ago

@stephanrotolante The problem isn't that the proxying doesn't work, it does. But accessing a not websocket path results in a crash of the whole script.

@fromtexas This solution works! Thank you. Still I think this is a bug, a misconfigured server shouldn't result in a application crash.

javiergonzalezGenially commented 3 years ago

Got the same error and solved it the way @fromtexas said. I think that should be on the docs.

omgovich commented 4 months ago

Having the same issue. Very annoying