fastify / fastify-reply-from

fastify plugin to forward the current http request to another server
MIT License
148 stars 76 forks source link

onResponse res instead of res.stream #334

Closed Swizz closed 7 months ago

Swizz commented 8 months ago

Prerequisites

🚀 Feature Proposal

onReponse method provide access only to the res.stream but we are facing a scénario where it might be useful to use the whole res instead.

And we found no means to get access to it. To do not be a breaking change, onReponse could provide both res.stream and res.

Motivation

Hi, thank you for bringing us fastify-reply-from.

We using it to simplify our API for end customers. But in some scenario we have to merge two APIs in one because we have a lot of multi steps APIs.

Be used fastify-reply-from so far with some little tweaks to get access to the response of the step 1 for passing it to the step 2.

But in one of our scenario an API reply with a 204 that is not an Error, but result in a complete end of the original request instead of moving to the step 2. We are able to override the reply status code, but we would like a way to know if it is a 204 originally or any other codes.

We understand that reply-from main goal is to proxy only one request. But in our case, it would be useful to have access to the whole response as well.

Cheers,

Example

Instead of onResponse(request, reply, resBody) it could be onResponse(request, reply, resBody, res)

Uzlopak commented 8 months ago

I assume that res.stream is used for higher throughput. Maybe we should implement an option to select if res or stream is passed through. This would empower others to get what they need.

mcollina commented 8 months ago

Given that we had a regression for 204 recently, could you include a reproduction for the 204 problem you are mentioning?

I'm not sure I understand your use case. I think your code would be easier to read and maintain if you just used undici.request directly to orchestrate your APIs?

Swizz commented 8 months ago

Hi, thanks for your inquiry.

Here is a repo that try to reproduce our use case : Swizz/reply-from-dummy Because of a legacy codebase, we are still using node 14 and fastify v2 in production.

By running npm run start, you will be provided a swagger interface at /docs. All routes are described in the readme, and more comments in the code will enlight you about what we are trying to achieve. The special routes /post/100 will mimic the 204 behavior.

We tried to use undici, but you did a great job on reply-from to relay directly headers such as authentication or request status for errored routes that saved us for a lot of headaches.

But we would totally understand if your are against such changes because opening to more features could add you maintenance on subjects your package was not intented for.

mcollina commented 8 months ago

I'm extremely surprised that the latest version of this module works with such an old version of Fastify.

Composing multiple endpoint into out is somewhat out of the one of @fastify/reply-from. I'd recommend to use undici directly and forward the headers.

If you are using OpenAPI, you can check out the automated client we created in Platformatic. It has facilities to forward headers etc.

https://docs.platformatic.dev/docs/reference/client/introduction/#openapi

If you need professional help with the migration or anything similar, feel free to ping me via email, happy to have a chat.

Swizz commented 7 months ago

Thank you for your heads up. We will try to move on undici then.

I think we can close this one, then.

Thanks a lot for your time.