express-http-proxy icon indicating copy to clipboard operation
express-http-proxy copied to clipboard

`next()` is called twice when using skipToNextHandlerFilter

Open iamdey opened this issue 6 years ago • 4 comments

after the fix for #377 I saw a 404 error poping in my case.

I suppose it is due to the reject which calls next here https://github.com/villadora/express-http-proxy/blob/master/app/steps/maybeSkipToNextHandler.js#L15

and here https://github.com/villadora/express-http-proxy/blob/master/index.js#L56

With only 2 routes are defined, it results in a 404 error since the second one might not be finished.

iamdey avatar Nov 07 '18 10:11 iamdey

@iamdey Thanks for this report. I suspect your analysis is on point. Could you elaborate -- ideally with sample code -- on the case 'With only 2 routes are defined'? I'll write to test to capture this case with the fix.

monkpow avatar Nov 07 '18 17:11 monkpow

quick 'n dirty, I try reproduce my use case (It might be simplified but you have the thing):

const proxy = require('express-http-proxy');
const app = require('express')();
const { Router: createRouter } = require('express');

const router = crreateRouter();

router.get('/foobar', proxy('www.google.com', {
  skipToNextHandlerFilter: () => true,
));

router.get('/foobar', (req, res, next) => {
  // this correctly called by the first next() in express-http-proxy
  setTimeout(() => {
    // at this point headers are already sent by second next that has no more "express layers" to execute
    res.send({ hello: 'world' });
  }, 2000);
};

app.use(router);

iamdey avatar Nov 07 '18 22:11 iamdey

Any notice of this? I also remove this extra next call

ldmarz avatar Jan 24 '19 16:01 ldmarz

I am facing the same issue pointing by @iamdey .

In my case, I am trying to make a call again if the first call returned a 401 due to a not valid access token anymore.

skipToNextHandlerFilter in myOptionsWithCachedToken is like this:

skipToNextHandlerFilter: function(proxyRes) {
   return proxyRes.statusCode === 401;
}
let firstCall  = proxy(process.env.ctpApiEndpoint, myOptionsWithCachedToken);
let secondCall = proxy(process.env.ctpApiEndpoint, myOptionsWithRenewedToken);

app.use(firstCall,secondCall); 

I am getting 404 as a response even with the changes done in #390

https://github.com/villadora/express-http-proxy/commit/c5660c22e607700f1d1d8a002262100f427ca634

rhm-fyayc avatar Mar 01 '19 14:03 rhm-fyayc