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

Doesn't seem to recognize non proxy routes (express) #94

Closed
newmanw opened this issue Jul 29, 2016 · 8 comments · Fixed by #98
Closed

Doesn't seem to recognize non proxy routes (express) #94

newmanw opened this issue Jul 29, 2016 · 8 comments · Fixed by #98
Labels

Comments

@newmanw
Copy link

newmanw commented Jul 29, 2016

Expected behavior

When setting up an express router, routes not in proxy context are not recognized. IE if filter function returns false, non proxy routes in sub application should be found.

Actual behavior

404 for routes in sub application

Setup

  • http-proxy-middleware: 0.17.0
  • server: express
  • other relevant modules

proxy middleware configuration

var filter = function (pathname, req) {
    return pathname.match(new RegExp('^/api'));
};
var apiProxy = proxy(filter, {target: 'http://www.example.org:8000'});

server mounting

var sub = new express.Router();
sub.use(proxy);

// main app, mount the sub application
var app = express();
app.use('/sub', sub);

other route in sub application

sub.get('/someroute',
   function(req, res, next) {
      // won't find route
   }
);
@chimurai
Copy link
Owner

chimurai commented Jul 29, 2016

In your filter function you are using a string instead of regex in the match function pathname.match('^/api').
Think it should be: pathname.match(/^\/api/) or pathname.match(new RegExp('^/api'))

This piece of code is also weird:
var proxy = proxy(filter, {});

  1. You are overriding the original proxy function... use a different variable namevar proxyExample = proxy(filter, {});
  2. proxy(filter, {}); is missing the target value: proxy(filter, {target: 'http://example.org'});

@newmanw
Copy link
Author

newmanw commented Jul 29, 2016

@chimurai you are correct, I did not copy my example and made a few mistakes when typing it in, sorry about that. Still having problems, if the filter function returns false, non proxy routes are not hit.

IE, in my example the sub app has a context of sub so if I hit 'http://myserver.com/sub/api', the proxy filter will return true and the request will be proxied.

However if I hit 'http://myserver.com/sub/someroute', the filter function returns false as it should, but the request does not go through to the subroute endpoint.

@chimurai
Copy link
Owner

chimurai commented Aug 1, 2016

@newmanw: Could be provide a minimal/small working sample app; showing the problem you described? This will help me debug this issue.

@newmanw
Copy link
Author

newmanw commented Aug 2, 2016

@chimurai absolutely.

https://github.com/newmanw/hpm-sub

Proxy works: 'http://localhost:3000/sub/api'
Cannot hit 'http://localhost:3000/sub/hello'

@chimurai
Copy link
Owner

chimurai commented Aug 7, 2016

Thanks for providing the example @newmanw.

Think this line is causing the issue: lib/index.js#L39

req.url = req.originalUrl;

It is modifying the req.url even when there is no 'match'.

Need to investigate if this is really causing the issue

@chimurai
Copy link
Owner

chimurai commented Aug 7, 2016

Hi @newmanw,
Think I've fixed the issue. Can you verify it?

Still need to write some tests for this use-case.

Edit: You can find the fix in PR #98

@newmanw
Copy link
Author

newmanw commented Aug 8, 2016

Works as I would expect. Awesome turn around time, thanks so much!

@chimurai
Copy link
Owner

Just pushed v0.17.1 with the fix.

Thanks for reporting the issue and helping out with debugging. Greatly appreciated!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants