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

Re-emit 'start', 'forward' and 'end' events in RoutingProxy #216

Merged
merged 4 commits into from May 10, 2012
Merged

Re-emit 'start', 'forward' and 'end' events in RoutingProxy #216

merged 4 commits into from May 10, 2012

Conversation

coderarity
Copy link
Contributor

Fixes #214. This will re-emit more events from RoutingProxy so that you can listen for those events yourself. Be VERY careful for the following case, which will add a new listener for every request:

//DO NOT USE THIS CODE! IT'S BROKEN!
var httpProxy = require('http-proxy');

httpProxy.createServer(function (req, res, proxy) {

    var buffer = httpProxy.buffer(req);

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

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

}).listen(8000);

It would be good if we could fix this case! This is a very easy problem to run into!

@coderarity
Copy link
Contributor Author

The reason that this case exists is because you only have access to the RoutingProxy inside of the server function. The RoutingProxy never gets recreated between requests, so it will end up adding a new listener every time a request is made.

One solution to get around this right now is to not use httpProxy.createServer at all, instead making your own server, like so:

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

var proxy = new httpProxy.RoutingProxy();

proxy.on('end', function() {
  console.log('proxied');
})

http.createServer(function (req, res) {
  proxy.proxyRequest(req, res, {
    host: 'localhost',
    port: 8080
  });
}).listen(8000);

You could also use the server.proxy object to do this:

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: 8080,
      buffer: buffer
    });

});

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

server.listen(8000);

I'm not sure how to fix the case with listening for events on the proxy object inside the server function. You could remove the old listener every request, but adding and removing a listener on every request is very inefficient. If it recreated the RoutingProxy every request, that would almost be as inefficient. I'm not sure if it's even reasonable to eliminate this case - although it is likely to be a common mistake.

@subnetmarco
Copy link
Contributor

+1

@coderarity
Copy link
Contributor Author

This last commit fixes many problems people have had with RoutingProxy hanging. It will now return a real error from the matched HttpProxy to the client. Fixes #185 and will provide error information for a bunch of issues that have reported node-http-proxy to be hanging. I added it to this pull request since it changes how I handle some of the event's redirection in the first round of commits.

@indexzero
Copy link
Contributor

@coderarity Looks good to me. Merge at will.

coderarity added a commit that referenced this pull request May 10, 2012
Re-emit 'start', 'forward' and 'end' events in RoutingProxy, and fix some hanging issues.
@coderarity coderarity merged commit f223ce8 into http-party:master May 10, 2012
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 this pull request may close these issues.

res.on('end', callback) not working
3 participants