Can't use `proxy()` twice in an Express middleware stack

[chriscalo]

Steps to reproduce:

1. Clone chriscalo/express-http-proxy-bug and peek at start.js to see that it's proxying to two APIs running in separate processes.
2. Install and run with npm i && npm start
3. Open localhost:8000/api1 and notice that it correctly returns "Hello from API 1"
4. Next, open localhost:8000/api2 and notice that a 404 is returned

Observed result:

• It seems that when there's a sequence of handleProxy(req, res, next) functions, the first one works, but the second is broken.
• Try commenting out the line that proxies to API 1 (app.use(proxyWithFallthrough(ports.api1));) and notice that visiting /api2 now works when it previously didn't.

Expected result:

• I expect that if the first handleProxy(req, res, next) function returns a 404 response (because I'm using skipToNextHandlerFilter()), the next handleProxy(req, res, next) function should be able to respond.

Notes

• there's a reason why I'm not able to prefix each proxy middleware with a single path /api1 or /api2

[chriscalo]

~Might be related to #390~

[chriscalo]

From the info in #390, I tried adding no-op layers to the Express stack immediately after the proxy() layers, but this didn't change the behavior at all.

// DOESN'T FIX THE PROBLEM
const app = express();
app.use(proxyWithFallthrough(ports.api1));
app.use((req, res, next) => next());
app.use(proxyWithFallthrough(ports.api2));
app.use((req, res, next) => next());

[chriscalo]

I'm not fully confident I would be able to fix this, but if anyone could point me to the right place in the source code and offer any guidance on what the right solution might look like, I'd be happy to try.

[Aharonr92]

@chriscalo the example did not change the behavior because of the extra call of the next function inside the no-op layers. this one did work for me 👇

const app = express();
app.use(proxyWithFallthrough(ports.api1));
app.use((req, res, next) => {});
app.use(proxyWithFallthrough(ports.api2));
app.use((req, res, next) => {});

and i think this one should solve it c5660c2

[chriscalo]

Thanks so much for the fix, @Aharonr92! 🙏🏼

This unblocks an architecture I was trying to get working, so I'll give it another shot 👍🏼

[chriscalo]

@Aharonr92, I haven't tried the fix yet, but as I looked more closely at the code snippet you shared, it's not making sense to me.

If you had the following configuration for an Express app, the second call to proxyWithFallthrough)(), marked // 3 would NEVER get called because any time the handler function marked // 2 gets called, it never calls next() to pass along to the next handler (// 3).

const app = express();
app.use(proxyWithFallthrough(ports.api1)); // 1
app.use((req, res, next) => {});           // 2
app.use(proxyWithFallthrough(ports.api2)); // 3
app.use((req, res, next) => {});           // 4

Is there something I'm missing? From my understanding of Express layers, doesn't the following HAVE TO be how it is configured for all handlers to have a chance of being called?

const app = express();
app.use(proxyWithFallthrough(ports.api1)); // 1
app.use((req, res, next) => next());       // 2
app.use(proxyWithFallthrough(ports.api2)); // 3
app.use((req, res, next) => next());       // 4

To clarify, what I'm trying to set up is an app where:

1. the request is first proxied to the server listening on ports.api1
2. and if that server responds with a 404, then it gets proxied to the server listening on ports.api2

[Aharonr92]

@chriscalo, you are absolutely right each handler should call the next function in order to continue to the next handler, but in our case the problem is that when the skipToNextHandlerFilter is applied it will call the next function twice, once in the maybeSkipToNextHandler line 15 and since it rejected it will call the next function again in the handleProxy function line 56

so the first next call will pass it to the function marked // 2 and the second call will pass it to the function marked // 3 but if you call the next function in the function marked // 2 it will be passed to the function marked // 4 and the function marked // 3 will be skipped.

[chriscalo]

Thanks for the explanation.

Has the double-calling been fixed such that the following will now work?

const app = express();
app.use(proxyWithFallthrough(ports.api1));
app.use((req, res, next) => next());
app.use(proxyWithFallthrough(ports.api2));
app.use((req, res, next) => next());

I'll give this a try soon to be sure.

[chriscalo]

This still doesn't work, even after patching in the change c5660c2.

In the test repo express-http-proxy-bug, instead of the getting a 404 for the 2nd API call, it just hangs forever.

[chriscalo]

For anyone else running into the same problem, I ended up switching to using http-proxy-middleware to be able to proxy requests to a server running at another port or address and when a 404 is returned from a proxied request, happily ignore it and continue on to the next handler in the Express stack.

I created the following example repo to show how it works:

https://github.com/chriscalo/express-proxy-fallthrough-example

[monkpow]

I believe I've fixed this with #529. Thanks for such detailed use case descriptions.