mswjs / msw

Industry standard API mocking for JavaScript.
https://mswjs.io
MIT License
16.04k stars 525 forks source link

"Protocol "http:" not supported. Expected "https:"" when using superagent #632

Closed tomdohnal closed 3 years ago

tomdohnal commented 3 years ago

Describe the bug

When I try to use msw alongside superagent in Node, it throws an error:

UnhandledPromiseRejectionWarning: TypeError [ERR_INVALID_PROTOCOL]: Protocol "http:" not supported. Expected "https:"

(I'm aware of issues such as https://github.com/mswjs/msw/issues/209 but this one seems to be different)

Environment

Please also provide your browser version. n/a

To Reproduce

Clone the repository on https://github.com/tomdohnal/msw-superagent-repro or run this piece of code using node

const { rest } = require("msw");
const { setupServer } = require("msw/node");
const request = require("superagent");
const fetch = require("node-fetch");

setupServer(
  rest.get("https://google.com", (req, res, ctx) => {
    return res(ctx.status(200), ctx.text("Hi from google"));
  })
).listen();

request.get("https://google.com").end(); // -> throws an error

// fetch("https://google.com")
//   .then((res) => res.text())
//   .then(console.log); -> works just fine

Expected behavior

No error should be thrown and the call should be mocked

Screenshots

n/a

kettanaito commented 3 years ago

Hey, @tomdohnal. Thanks for reporting this.

Once again we're dealing with million ways of creating a request in Node.js. I suspect that the request params of ClientRequest are not parsed properly for this particular use-case:

https://github.com/mswjs/node-request-interceptor/blob/0db6518f811a8c80155ed8f7c799beb8ed46db0f/src/interceptors/ClientRequest/utils/normalizeHttpRequestParams.ts#L38

If you're comfortable with debugging, you can clone node-request-interceptor, link it, and see what's getting parsed in this function when supertest makes a request.

Huge thanks for preparing a test repo! I will try to look into it.

tomdohnal commented 3 years ago

OK so I did some debugging and found out that the problem is caused by the fact that when superagent (the HTTP(S) client) makes an HTTPS request, it doesn't pass a protocol property to the request options.

As per https://nodejs.org/api/https.html#https_https_request_options_callback the default value of the protocol option passed to the https.request call is (not surprisingly 😄) https so there is no need to be explicit about it.

However, as the normalizeHttpRequestParams only accept the request options object as an argument (and doesn't accept the info about if the request is being made using the http or https module), it falls back to creating a URL beginning with http which then causes an error to be thrown in the msw library.

One possible solution that comes to my mind would be to propagate the protocol variable all the way from the handleRequestFunction (in src/interceptors/ClientRequest/index.ts) to the normalizeHttpRequestParams function and use the protocol variable to put together the correct URL (if there is no protocol variable on the request options object).

I'm happy to send a PR with the fix but first I wanted to discuss how to approach this 🙂

kettanaito commented 3 years ago

Hey, @tomdohnal. Thank you for the investigation!

You're right, as there's a way to create a ClientRequest with options that don't include an explicit protocol, the latter must be inferred from the request-issuing module. https://github.com/mswjs/node-request-interceptor/pull/94 adds that inference. We will release this in the next version.

tomdohnal commented 3 years ago

Thanks a lot for the fix @kettanaito ! Was happy to help

basicdays commented 3 years ago

Just got bit by this one. Excited to try out the fix though, any ideas when it may get published?

kettanaito commented 3 years ago

This should get published in the next version (#660).

kettanaito commented 3 years ago

Released in 0.28.0.