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

Exception thrown in logger when trying to build URL for message #1056

Open
2 tasks done
VadBary opened this issue Nov 7, 2024 · 3 comments
Open
2 tasks done

Exception thrown in logger when trying to build URL for message #1056

VadBary opened this issue Nov 7, 2024 · 3 comments

Comments

@VadBary
Copy link

VadBary commented Nov 7, 2024

Checks

Describe the bug (be clear and concise)

In fact, the solution for the issue has already been implemented for v3.0.2 But implemented fix doesn't work in case when target option is not set but router option is used instead.

Step-by-step reproduction instructions

1. Send a message to an Express route which then uses `http-proxy-middleware` to send a message on to another endpoint
Have an endpoint which sends a response where the `req` object doesn't include all the properties such as `protocol`. I believe mocking framework like `nock` would do this.
2. Use router option instead of target option.

Expected behavior (be clear and concise)

The logger can cope with bits of req not being present when trying to build a valid URL when router option is used instead of target.

How is http-proxy-middleware used in your project?

We are using `http-proxy-middleware `^3.0.3`with express router for routing.

What http-proxy-middleware configuration are you using?

createProxyMiddleware({
     router: (request) => this.route(request),
     agent,
     secure: false,
     changeOrigin: true,
     followRedirects: true,
     ignorePath: true,
     headers: { Connection: 'keep-alive' },
     plugins: [proxyRequestLoggerPlugin],
     on: {
       error: (err, req, res, target) => this.proxyError(err, req, res, target),
       proxyReq: (proxyReq, req) => {
         this.proxyReq(proxyReq, req);
       },
       proxyRes: (proxyRes, req) => {
         this.proxyRes(proxyRes, req);
       },
     },
   });

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

Node v20.15.1

Additional context (optional)

No response

@chimurai
Copy link
Owner

chimurai commented Nov 7, 2024

Could you provide a minimal working reproduction for investigation?

@VadBary
Copy link
Author

VadBary commented Nov 8, 2024

Hi @chimurai, my issue is the same like this 1035. I investigated it deeply and know the reason. There is a commit with a fix to resolve the issue, this commit was delivered to v.3.0.2 - fix(logger-plugin): handle undefined protocol and hostname. As part of this commit was added try...catch block to handle the situation when protocol is undefined, in my case, protocol is also undefined... But the solution doesn't work in my case because I don't define target as an option for createProxyMiddleware but set router option instead. If you pay attention on this catch block implementation in logger-plugin.ts target = new URL(options.target as URL); then you will see that options.target is required to build URL but as I mentioned this is possible that target is not set because router option is set instead. In this case, options.target is undefined and target = new URL(undefined as URL); throw a new Error. If avoid using options.target for building URL and use something like that target = new URL(${proxyRes.req.protocol}//${proxyRes.req.host}$proxyRes.req.path}); in catch block then everything would be ok.

@chimurai
Copy link
Owner

chimurai commented Nov 8, 2024

Thank you for the analysis.

As you might already have seen in #1035 (comment), I didn't got a reply on the minimal reproduction for further investigation.

Would really appreciate your help if you can provide a minimal example for investigation so the logger can be fixed properly. 🙏

Alternatively opening a PR would also be really helpful

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

No branches or pull requests

2 participants