Node-http-proxy: How can I filter socket hang up errors?

Created on 27 Apr 2015 · 23Comments · Source: http-party/node-http-proxy

I'm using node-http-proxy for a while now, and I think I'm using it quite right catching errors like this

proxy.on('error', function(err, req, res) {
    // log
    // ...
    // handle
    if (!res.headersSent) {
        res.writeHead(500, { 'content-type': 'application/json' });
    }
    res.end(JSON.stringify({ error: 'proxy_error', reason: err.message }));
});

But I can have two cases that make a "socket hang up" error happen.

Crash behind
Client cancelling the request

Watching at my log I can make no differences between those two cases and it's quite frustrating. Before knowing about request abort (thank #527) I really though I have an error going on.

So, what's the best practice to filter the request.abort case out?

needs-investigation

Source

JSteunou

Most helpful comment

Ok for the record, in case anyone was interested at doing the same thing

How to catch request client cancelled and abort ongoing proxied request

proxy.on('proxyReq', function(proxyReq, req) {
    // keep a ref
    req._proxyReq = proxyReq;
});

proxy.on('error', function(err, req, res) {
  if (req.socket.destroyed && err.code === 'ECONNRESET') {
    req._proxyReq.abort();
  }
});

JSteunou on 2 Dec 2015

👍5

All 23 comments

theoephraim on 28 Apr 2015

FoghostCn on 16 Jul 2015

You should be able to use the proxy error event or the error callback and check for res.socket.destroyed === true and error.code === ECONNRESET which means the request was canceled

vvo on 1 Dec 2015

Merci @vvo ;) Will try that asap

JSteunou on 2 Dec 2015

It works very fine, thank you again @vvo

@jcrugzz should I let this issue open? Maybe http-proxy should filter those error inside the library, or maybe it's better to let it unopinionated...

JSteunou on 2 Dec 2015

or maybe it's better to let it unopinionated...

Yes it depends on the use case, sometime you will want the error to pop out if the client canceled the request.

vvo on 2 Dec 2015

What I thought. Closing it then.

JSteunou on 2 Dec 2015

Quick question again: I want to use request abort to prevent the underlying server to handle that client cancelled request. But, very surprisingly, req.abort is undefined O_O

Any thoughts?

JSteunou on 2 Dec 2015

nope sorry,

vvo on 2 Dec 2015

I think the req I got is the received request, not the request currently sent by http-proxy

JSteunou on 2 Dec 2015

Ok for the record, in case anyone was interested at doing the same thing

How to catch request client cancelled and abort ongoing proxied request

proxy.on('proxyReq', function(proxyReq, req) {
    // keep a ref
    req._proxyReq = proxyReq;
});

proxy.on('error', function(err, req, res) {
  if (req.socket.destroyed && err.code === 'ECONNRESET') {
    req._proxyReq.abort();
  }
});

JSteunou on 2 Dec 2015

👍5

This is a bug and should be fixed within http-proxy itself. Currently we are emitting an error on the client request when we should only be cancelling the proxy request when this happens.

jcrugzz on 2 Dec 2015

Which one? Getting all errors even from client cancelled request or be forced to tricky save proxyReq to be able to abort?

JSteunou on 2 Dec 2015

@JSteunou To put it simply, your workaround here should be handled within http-proxy for the req.on('error')

jcrugzz on 2 Dec 2015

Would be nice :)

JSteunou on 2 Dec 2015

@JSteunou Pull requests always welcome if you want to take a stab at it! :)

jcrugzz on 2 Dec 2015

also @vvo if you want the error handled, you could handle the error directly on req.on('error'). Im of the opinion that we need to cleanup our proxyReq regardless and if you want it as an error in your code the error can be handled there. Does this seem reasonable?

jcrugzz on 2 Dec 2015

Yep makes sense

vvo on 2 Dec 2015

@jcrugzz I'm afraid I have the necessary solution and motivation but not the time to do it.

JSteunou on 2 Dec 2015

@JSteunou it would be a simple change right around here. You may be overestimating the time :). It would be a good opportunity for first contribution

jcrugzz on 2 Dec 2015

Fairly old thread, but still an issue afaik. Resolving it by comparing the error code to ECONNRESET and ignoring the error as suggested before.

Would be happy to submit a PR to handle this exception internally, but aren't there any use cases where you'd actually want to handle this manually? At the very least you'd need to emit a new event instead (abort perhaps?). Any thoughts?