chimurai / http-proxy-middleware

:zap: The one-liner node.js http-proxy middleware for connect, express, next.js and more
MIT License
10.7k stars 834 forks source link

Service async function on onProxyReq #318

Open pawelszczerbicki opened 5 years ago

pawelszczerbicki commented 5 years ago

Having proxy configured with onProxyReq like:

proxy(gatewayUrl, {
  target: destinationUrl,
  pathRewrite: { [`^${gatewayUrl}`]: '' },
  changeOrigin: true,
  logProvider: () => logger,
  ...options,
  onProxyReq: async (proxyReq, req) => await addAuthHeader(proxyReq, req, redis),
});

Middleware is finishing request before promise is resolved. It is possible to service promise in proxy functions ?

julkue commented 5 years ago

@pawelszczerbicki Having the same question. I will probably use node-http-proxy instead, as there's a documented example for this case.

magnusp commented 5 years ago

I was also looking at making external calls to add/verify authorization headers but came to the same conclusion. I'm guessing that a viable option is to have another middleware do the external call and have it mutate incoming request headers (would transfer over to onProxyReq). But it would be really nice to be able to do this within http-proxy-middleware having matched the route/pattern of the incoming request.

chimurai commented 5 years ago

Looks like there is some demand for async listeners. I'll consider implementation after some investigation.

@julmot Can you point to the documented node-http-proxy example?

@magnusp Think you can to achieve the same result by chaining middlewares. Assuming you use express you can write a authValidator and mount it in express like: app.use('/some/endpoint', authValidator, proxy)

magnusp commented 5 years ago

Im using it via webpack-dev-server which has a configuration property for http-proxy-middleware. The devserver also has a way to setup additional middleware via the "before" and "after" configuration properties. This leaves me having a choice of using my middleware+http-proxy-middleware in a "before" or only setting up my middleware in "before" and using the dedicated "proxy" configuration property. Im mostly mentioning this here for other people stumbling upon the issue and letting them see the options available. But async listeners is an awesome feature. I remember wanting it when implementing a chaos monkey where delays on the proxy were introduced randomly and configurably, at runtime.

SreeBS commented 5 years ago

@chimurai I have to deal with promises in both proxyReq and proxyRes functions. Would be very helpful to have to have these functions support promises.

montera82 commented 5 years ago

+1 one for async

how can I achieve using the result of an async call there?

i am doing something like this

onst buildingConfProxyOptions = {
  target: TARGET_BASE_URL,
  changeOrigin: true,
  pathRewrite: {
    '^/building-conf': ''
  },
  async onProxyReq(proxyReq: any, req: any, res: any) {
    const token = req.headers['authorization'];
    const userSvc: UserService = Container.get(UserService);
    const result = await userSvc.verify(token); // Its ending here
// BUT I NEED TO USE RESULT HERE!

  },
  loglevel: 'debug'
};
shian15810 commented 5 years ago

For temporary workaround, I'm doing it like this with promise:

onProxyReq: (proxyReq, req, res) => {
  proxyReq.socket.pause();
  aPromise(a, b).then((c) => {
    proxyReq.setHeader('x-added', c);
    proxyReq.socket.resume();
  }).catch((err) => {
    console.error(err);
    res.sendStatus(500);
  });
},

Or with callback function:

onProxyReq: (proxyReq, req, res) => {
  proxyReq.socket.pause();
  aFunctionWithCallback(a, b, (err, c) => {
    if (err) {
      console.error(err);
      res.sendStatus(500);
      return;
    }
    proxyReq.setHeader('x-added', c);
    proxyReq.socket.resume();
  });
},

Is this plausible at all?

boxcee commented 5 years ago

@shian15810 Did you try? Does it work?

shian15810 commented 5 years ago

Yeah, it works! By plausible I mean does this really make sense?

magnusp commented 5 years ago

Seems reasonable. I'm guessing that the proxyReq.socket should probably be aborted when an error occurs otherwise it could be left hanging until aborted by an timeout in either end.

shian15810 commented 5 years ago

@magnusp Do you mean something like proxyReq.socket.end() after res.sendStatus(500)?

Or should it be better off if I get rid of res.sendStatus(500) and use proxyReq.socket.end('HTTP/1.1 500 Internal Server Error\r\n\r\n') instead?

Edited 1:

Just check the source code in express, express's res.sendStatus will trigger res.send within itself and in the end trigger node's http res.end.

Edited 2:

Just tested, socket.resume is not necessary when using res.sendStatus.

darewreck54 commented 5 years ago

@shian15810 How did you set the header. When i try to set the header via proxyReq.setHeader('a','g'), i get an exception throwing

Cannot set header after they are sent to the client.

how did you get around that part? I followed the first promise example

Also not sure what the deal is but the res object has not sendStatus() method.....

Thanks!

shian15810 commented 5 years ago

@darewreck54 I was having this issue too, and after looking into the source code, at least for me, I couldn't find any way to circumvent this situation, even when using req.headers[customHeader].

But having custom header is essential to the functioning of my application. So finally I decided to change the whole architecture to microservice-based. Sorry, couldn't be much of your help.

If you don't need custom header for your application to be functional, the hack I proposed earlier is still a feasible way to get around the original issue.

Using req.headers[customHeader] won't give you that annoying error, but the header won't reach the upstream server anyway.

darewreck54 commented 5 years ago

I actually end up creating my own express middleware that execute the promise and when its successful it will trigger next(). I'll store the updated header in the request and send it to the proxy middleware

kavitalikesfajitas commented 5 years ago

@chimurai did you create a solution to this? Also if not can you provide an example of the authValidator you mentioned?

mihir83in commented 5 years ago

For temporary workaround, I'm doing it like this with promise:

onProxyReq: (proxyReq, req, res) => {
  proxyReq.socket.pause();
  aPromise(a, b).then((c) => {
    proxyReq.setHeader('x-added', c);
    proxyReq.socket.resume();
  }).catch((err) => {
    console.error(err);
    res.sendStatus(500);
  });
},

Or with callback function:

onProxyReq: (proxyReq, req, res) => {
  proxyReq.socket.pause();
  aFunctionWithCallback(a, b, (err, c) => {
    if (err) {
      console.error(err);
      res.sendStatus(500);
      return;
    }
    proxyReq.setHeader('x-added', c);
    proxyReq.socket.resume();
  });
},

Is this plausible at all?

This works fine on onProxyReq but doesn't work on onProxyRes when trying to set the header. The header never reaches browser.

@julmot any links to the example in http-proxy will be great.

@chimurai any work around / solution on getting this working with onProxyRes?

dpetrov commented 4 years ago

@shian15810 I somehow can't get it quite to work:

function onProxyReq(proxyReq, req, res) {
        proxyReq.socket.pause();
        setTimeout(function() {
                proxyReq.setHeader('X-AUTH-USERNAME', 'admin');
                proxyReq.socket.resume();
        }, 1000);
}

which results to:

throw new ERR_HTTP_HEADERS_SENT('set');
^
Error [ERR_HTTP_HEADERS_SENT]: Cannot set headers after they are sent to the client
    at ClientRequest.setHeader (_http_outgoing.js:470:11)

Is anything that I might be missing?

dpetrov commented 4 years ago

So for the others that might be looking, I've ended up as well writing a middleware that stores the value into the req. For example:

app.use(function (req, res, next) {
  aFunctionWithCallback((err, res) => {
    req._username = res.username;
    next();
  });
});

and later on:

  "onProxyReq": function(proxyReq, req) {
    proxyReq.setHeader('X-AUTH-USERNAME', req._username);
  },

Probably not plausible at all, but it does work.

revolunet commented 4 years ago

Thanks @dpetrov this works here too. you dont even have to use onProxyReq, headers will get forwarded by the proxy later on

server.use((req, res, next) => {
  if (req.path === "/graphql" && req.method === "POST") {
    getSomethingAsync(req.session.id, (err, data) => {
      if (data) {
        req.headers["Authorization"] = `Bearer ${data.user.token}`
      }
      next()
    })
  } else {
    next()
  }
})
kwiss commented 4 years ago

no way @revolunet you are here too

Austio commented 4 years ago

Are there any constraints in the library that would block a community PR for this @chimurai?

Was able to work around with a middleware that fires before my proxy middleware

smileatom commented 3 years ago

There was a PR last year on http-proxy for this. And without async support in handlers even though it might be somewhat trivial to implement, I didnt really get a warm fuzzy when I looked at the internals.

Ill just use a library with keep alive and call it good.

maapteh commented 3 years ago

@chimurai Steven bouw deze feature even erin man :)

we also want the async option, just before we stream back the results we want to alter header with async data.

i will checkout the mentioned node-http-proxy because it has selfHandleResponse option

shihshen commented 3 years ago

I tried with selfHandleResponse option, and it seems working in this middleware as well.

maapteh commented 3 years ago

@chimurai Steven, ik heb hem werkend :)

So i have this setup where i can still alter the response header AFTER i get the response from the proxied endpoint and do async stuff. In this sample i also included the fake server im mocking to (3001). Maybe nice for your recipe? Its setting response header on the response AFTER getting the response and do async stuff.

https://codesandbox.io/s/holy-resonance-yz552?file=/src/index.js

curl -I http://localhost:3000 gives now:

HTTP/1.1 200 OK
X-Powered-By: Express
mpth-2: da
Date: Thu, 19 Nov 2020 21:53:43 GMT
Connection: keep-alive

where mpth-2 is set async.

all i had to do was use selfHandleResponse: true, and proxyRes.pipe(res); just by digging in the code :(

thanks @powpowshen for telling it is possible, it made me look again :)

zhuibo66 commented 3 years ago

感谢@dpetrov这里也行得通。你甚至不必使用, 头稍后会由代理转发onProxyReq

server.use((req, res, next) => {
  if (req.path === "/graphql" && req.method === "POST") {
    getSomethingAsync(req.session.id, (err, data) => {
      if (data) {
        req.headers["Authorization"] = `Bearer ${data.user.token}`
      }
      next()
    })
  } else {
    next()
  }
})

your answer is good

sarumont commented 3 years ago

Just a note to anyone else looking - the posted workaround which pauses the socket seems to not work with a Websocket upgrade.

Edit:

I did find a workaround here - handle the server's upgrade and call proxy.upgrade from your async handler:

const server = expressApp.listen(8080);
server.on('upgrade', async (req: Request, socket: Socket, head: Buffer) => {
  someAsyncFunction()
    .then(() => {
      // do what you need with the results of the async function here
      if (upstreamProxy.upgrade) {
        upstreamProxy.upgrade(req, socket, head);
      }
    });
});
blastshielddown commented 2 years ago

@dpetrov and @revolunet you are lifesavers. This worked for me 👍 Thank you

server.use((req, res, next) => {
  if (req.path === "/graphql" && req.method === "POST") {
    getSomethingAsync(req.session.id, (err, data) => {
      if (data) {
        req.headers["Authorization"] = `Bearer ${data.user.token}`
      }
      next()
    })
  } else {
    next()
  }
})
Hosstell commented 2 months ago

@pawelszczerbicki delete http-proxy-middleware and use just http-proxy

my example:

import http from 'http';
import httpProxy from 'http-proxy';

var proxy = httpProxy.createProxyServer({});

const handler = async (req: any, res: any) => {

    const status = await checker(req)
    if (!status) {

       res.statusCode = 403
       res.write("Forbidden")
       res.end()
    }

    proxy.web(req, res, {
        target: target_host
    });
}

var server = http.createServer(handler);

server.listen(3000);
dschweinbenz commented 1 month ago

2024: Still no async/await support :/