"proxyError" event emitted twice #364

Closed
thefosk opened this Issue Jan 10, 2013 · 8 comments

Comments

Projects
None yet
6 participants
@thefosk
Contributor

thefosk commented Jan 10, 2013

I created a simple proxy server that tries to proxy the request to a non existent server. I've written this to return an error message when the final host is unreachable.
After I make the request, when the proxyError is caught, I keep getting two times the same error:

{ [Error: connect ECONNREFUSED]
  code: 'ECONNREFUSED',
  errno: 'ECONNREFUSED',
  syscall: 'connect' }
{ [Error: connect ECONNREFUSED]
  code: 'ECONNREFUSED',
  errno: 'ECONNREFUSED',
  syscall: 'connect' }

I'm expecting the error to be thrown only once. This is the code I'm using:

var httpProxy = require('http-proxy');

var server = httpProxy.createServer(function (req, res, proxy) {
  var buffer = httpProxy.buffer(req);

  proxy.proxyRequest(req, res, {
    host: '127.0.0.1',
    port: 10000,
    buffer: buffer
  });

});

server.proxy.on('proxyError', function (err, req, res) {
    // This error is printed two times!
    console.log(err);
});

server.proxy.on('end', function() {
  console.log("The request was proxied.");
});

server.listen(9000);

Assuming that nothing is running on 127.0.0.1:10000, trying to execute:

curl http://127.0.0.1:9000

prints two times the same error message to the console.

I'm running:

  • node v0.8.17
  • node-http-proxy v0.8.7
@mmalecki

This comment has been minimized.

Show comment
Hide comment
@mmalecki

mmalecki Jan 10, 2013

Contributor

This is caused by handling an error on both socket and request.

Contributor

mmalecki commented Jan 10, 2013

This is caused by handling an error on both socket and request.

@thefosk

This comment has been minimized.

Show comment
Hide comment
@thefosk

thefosk Jan 10, 2013

Contributor

Is this my fault or the lib's fault?

Contributor

thefosk commented Jan 10, 2013

Is this my fault or the lib's fault?

@mmalecki

This comment has been minimized.

Show comment
Hide comment
@mmalecki

mmalecki Jan 10, 2013

Contributor

Our fault. I'll get to fixing it soon.

Contributor

mmalecki commented Jan 10, 2013

Our fault. I'll get to fixing it soon.

@abarre

This comment has been minimized.

Show comment
Hide comment
@abarre

abarre Jan 21, 2013

+1.

abarre commented Jan 21, 2013

+1.

@angn

This comment has been minimized.

Show comment
Hide comment
@angn

angn Feb 6, 2013

+1

angn commented Feb 6, 2013

+1

@erasmospunk

This comment has been minimized.

Show comment
Hide comment
@erasmospunk

erasmospunk Feb 10, 2013

Contributor

The offending code is in http-proxy.py:314-317 (and something similar for proxyWebSocketRequest on lines 732-735).

  //
  // Handle 'error' events from the `reverseProxy`.
  //
  reverseProxy.once('error', proxyError);
  reverseProxy.once('socket', function (socket) {
    socket.once('error', proxyError);
  });

Is it possible that one is fired and the other no?

Contributor

erasmospunk commented Feb 10, 2013

The offending code is in http-proxy.py:314-317 (and something similar for proxyWebSocketRequest on lines 732-735).

  //
  // Handle 'error' events from the `reverseProxy`.
  //
  reverseProxy.once('error', proxyError);
  reverseProxy.once('socket', function (socket) {
    socket.once('error', proxyError);
  });

Is it possible that one is fired and the other no?

erasmospunk added a commit to erasmospunk/node-http-proxy that referenced this issue Feb 10, 2013

@erasmospunk

This comment has been minimized.

Show comment
Hide comment
@erasmospunk

erasmospunk Feb 10, 2013

Contributor

After some research this part is causing the problem:

reverseProxy.once('socket', function (socket) {
  socket.once('error', proxyError);
});

This is not needed as node.js' ClientRequest object (here the reverseProxy) attaches a listener to the socket 'error'

socket.on('error', socketErrorListener);

The socketErrorListener just re-emits the error from the ClientRequest object.

Look in https://github.com/joyent/node/blob/master/lib/http.js for the socketErrorListener references.

Contributor

erasmospunk commented Feb 10, 2013

After some research this part is causing the problem:

reverseProxy.once('socket', function (socket) {
  socket.once('error', proxyError);
});

This is not needed as node.js' ClientRequest object (here the reverseProxy) attaches a listener to the socket 'error'

socket.on('error', socketErrorListener);

The socketErrorListener just re-emits the error from the ClientRequest object.

Look in https://github.com/joyent/node/blob/master/lib/http.js for the socketErrorListener references.

erasmospunk added a commit to erasmospunk/node-http-proxy that referenced this issue Feb 12, 2013

indexzero added a commit that referenced this issue Mar 9, 2013

Merge pull request #374 from erasmospunk/master
fixed issue #364 'proxyError' event emitted twice
@indexzero

This comment has been minimized.

Show comment
Hide comment
@indexzero

indexzero Mar 9, 2013

Member

Fixed in #374

Member

indexzero commented Mar 9, 2013

Fixed in #374

@indexzero indexzero closed this Mar 9, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment