proxy needs to be last middleware

[leveneg]

Perhaps I'm using the library incorrectly, or unaware of its limitations- but I recently found that I've had to put http-proxy-middleware as the last middleware in my express config.

Expected behavior

If possible, http-proxy-middleware should call the next() middleware. If not, the behavior should be documented.

Actual behavior

middlewares that follow http-proxy-middleware are not executed

Setup

app.use('*', [
  (req, res, next) => {
    console.log('I do get executed');
    next();
  },

  proxy({ ...proxyOptions }),

  (req, res, next) => {
    console.log('I do not get executed');
    next();
  }
]);

• http-proxy-middleware: 0.17.4
• server: express + 4.16.2

proxy middleware configuration

var apiProxy = proxy({
  target:'http://www.example.org',
  changeOrigin:true,
  secure: false
});

server mounting

const https = require('https');
const express = require('express');
const app = express();
const certs = require('./getCertOptions')();
const middlewares = require('./getExpressMiddlewares')();

app.use('*', middlewares);

https.createServer({ certs }, app).listen(5443);

[chimurai]

Currently next() is only called when nothing is proxied. (The opposite of what you expected)

http-proxy-middleware/lib/index.js

Line 43 in 66396d4

next()

---

Can you patch it locally to call next() at all times? And see if it works for your setup?

I'll have to investigate what kind of side-effects this changes might have.

[leveneg]

@chimurai doing so does work for me. That said, my other middlewares don't modify request headers, which I imagine is where making this change could be tricky.

I absolutely understand if guaranteeing next() is problematic here, but if that's the case noting it in README or something would be really helpful :)

[bannier]

@chimurai do you know why next() isn't called ?

[NoraGithub]

Why not return a promise or directly next()? It would make other middleware to work abnormally.

[dobesv]

Middleware doesn't call next if it handles the request. This is the case here. This isn't a bug. Otherwise it would call next and the next handler would return 404 Not Found or whatever.

[dobesv]

It will call next if you set it up to only proxy certain paths, and the request doesn't match one of those paths.

[vinit-sarvade]

@chimurai @leveneg when the option: selfHandleResponse is set, then the onProxyRes can have the next as argument that if required can be called for post proxy middleware.

[aral]

Just hit this also with the following use case:

1. I’m proxying an app (Hugo)
2. If a URL does not exist there, I want to serve it from an older version of the site

But the proxy middleware doesn’t call next() on 404s so other middleware can’t try and handle the request.

Thoughts?

[leveneg]

@aral using the onProxyRes option might be a good fit:

onProxyRes: (proxyRes, req, res) => {
  if (proxyRes.statusCode === 200) {
    // don't do anything if response successful
    return;
  }

  if (proxyRes.path.includes('VX') {
    // don't do anything if we're already returning from version fallback
    return;
  }

  // otherwise, redirect to versioned instance
  res.redirect('/VX/...');
},

... or something like that?

[aral]

@leveneg Thanks, Grant. I did try fiddling with onProxyRes but hadn’t thought to try a redirect. With cascading archives, given that the idea is that the path doesn’t change, I don’t think this would help in my case. When I get a moment, I will try and add support for calling next() on 404 to http-proxy-middleware. (I‘ve deferred the feature in question for the time being as it isn’t essential to the current release but a nice to have.) Appreciate your help :)

[subiron]

I have another case, I want to add cache layer on top of proxy.
flow looks like this:
Check if url is in the cache store
if so then return response body from cache
if not forward request to proxy
after response from proxy put its body into the cache. <- because of current implementation I'm unable to do this.

edit:
found workaround for my case here
#97 (comment)
also I see that I have missed @vinit-sarvade comment in this thread.
still, imho this should behave as most of the others middlewares and call next() by default allowing to override all aspects of the response and be