Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ResponseInterceptor recipe types mismatch #632

Closed
2 tasks done
hotsummmer opened this issue Jul 1, 2021 · 7 comments · Fixed by #882
Closed
2 tasks done

ResponseInterceptor recipe types mismatch #632

hotsummmer opened this issue Jul 1, 2021 · 7 comments · Fixed by #882
Labels

Comments

@hotsummmer
Copy link

yarn why http-proxy-middleware OR npm ls http-proxy-middleware output (mask private folder names with *****)

comet@0.0.1 *****
├─┬ @types/http-proxy-middleware@1.0.0
│ └── http-proxy-middleware@2.0.0 deduped
└── http-proxy-middleware@2.0.0

Describe the bug (be clear and concise)

TL; DR;
When using responseInterceptor on onProxyRes, types doesn't match.

In detail...

I wanted to handle response. So I used a function on [onProxyRes] (#97 (comment)).

but I didn't get the response as I wanted..
(Response was broken and I think it's related to decompressing... it's not the issue I wanna report anyways)

So, instead of using this

proxyRes.on('data', ....) 

I used responseInterceptor with your recipe

I'm using Typescript also, and unfortunately,
responseInterceptor's return value doesn't comply to onProxyRes.

Step-by-step reproduction instructions

I attached my code here.

https://gist.github.com/hotsummmer/387aba337e4a62ee0182a4587995f3db

Expected behavior (be clear and concise)

responseInterceptor expects arguments as below

(buffer: Buffer, proxyRes: http.IncomingMessage, req: http.IncomingMessage, res: http.ServerResponse)

but if I want to use it directly onProxyReq,
types doesn't match.

Because onProxyRes expects this type.

onProxyRes: (
        proxyRes: IncomingMessage,
        req: Request,
        res: Response,
    ): void 

It works if I use any as expected type... or convert types before passing it to responseInterceptor.
But wanted to ask you first about this issue

What http-proxy-middleware configuration are you using?

Line 16~33.

https://gist.github.com/hotsummmer/387aba337e4a62ee0182a4587995f3db

What OS/version and node/version are you seeing the problem?

MacOS 11.4, Node 12.18.3

Additional context (optional)

I appreciate your work and wonderful recipes!
Happy to use it and contribute :) thanks ❤️

@hotsummmer hotsummmer added the bug label Jul 1, 2021
@chimurai
Copy link
Owner

chimurai commented Jul 1, 2021

Thanks for the kind words!

Think this has been fixed in this PR #60.

But it hasn't been published yet.

@chimurai
Copy link
Owner

chimurai commented Jul 1, 2021

Just published v2.0.1.

onProxyRes is now typed as:

onProxyRes: (
  proxyRes: http.IncomingMessage,
  req: http.IncomingMessage,
  res: http.ServerResponse
): void 

Hopefully it won't give conflicts anymore.

Let me know if the issue persists.

@hotsummmer
Copy link
Author

Just published v2.0.1.

onProxyRes is now typed as:

onProxyRes: (
  proxyRes: http.IncomingMessage,
  req: http.IncomingMessage,
  res: http.ServerResponse
): void 

Hopefully it won't give conflicts anymore.

Let me know if the issue persists.

Wow. Thanks for taking care of it right away 👍 I will check on it!

@alsoscotland
Copy link

alsoscotland commented Jul 10, 2021

Due to the change of the req type from Request to req: http.IncomingMessage,
Typescript now complains when using req.get to retrieve header values when using the middleware with Expressjs. Could the types possibly be extended here so that both are compatible?

@chimurai
Copy link
Owner

@alsoscotland Could you share the code snippet you're using?

@alsoscotland
Copy link

@chimurai sure. Thanks for the quick response:
Here is a simplified version. Essentially for the case where I want to pass some headers through from the original request to the proxy request

export const onProxyReqFn = (
        proxyReq: ClientRequest,
        req: Request
        // res: Response
    ) => {
    const authHeader = req.get('authorization');
    if (authHeader) {
        proxyReq.setHeader('authorization', authHeader);
    }
};

@BinarySpike
Copy link

Does this break the documentation? I'm getting '[object Object]' when running const response = responseBuffer.toString('utf8');

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants