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

Status 400 (OK) displaying when it should display (Bad Request) #70

Closed
kentcdodds opened this issue Mar 19, 2020 · 11 comments · Fixed by #75
Closed

Status 400 (OK) displaying when it should display (Bad Request) #70

kentcdodds opened this issue Mar 19, 2020 · 11 comments · Fixed by #75

Comments

@kentcdodds
Copy link
Contributor

Here's what a request looks like when I set the status to 400 with msw vs a real server sending a 400:

image

I'm not sure what could be causing this.

@kettanaito
Copy link
Member

My guess is that since MSW constructs a new Response it has statusText: 'OK' by default. I presume you set the status code alone (i.e. via ctx.status(400)), so it doesn't map it to any status text associated with that status code.

One way to solve it for now could be to provide an explicit status text alongside the status code:

ctx.status(400, 'Bad request')

I value the validity of requests and responses, but I wonder whether it's the library's responsibility to map status codes to texts implicitly. I'd expect this to be done by Request constructor. Would have to look into this in more detail.

@kentcdodds
Copy link
Contributor Author

That's fine. Thank you for helping me know what to do there.

@kentcdodds
Copy link
Contributor Author

I think that having a default statusText that's the same as what the statusText would be for a given status code would be useful. I could always override it if I want to, but having a default to what the browser does would be sensible IMO.

@kettanaito
Copy link
Member

I was thinking of keeping a status code-status text map internally in MSW and mapping it by default, as you described. I guess there's little harm in that, but a big gain in developer experience.

@kettanaito
Copy link
Member

I went with the solution that reuses the statuses package thats's used in Express to map status codes to status texts.

Starting from 0.9.3 MSW now implicitly maps given status codes to the associated status text, if such is present, and if no custom status text is provided.

@kentcdodds, could you please update to 0.9.3 and let me know it produced correct status texts for you? Thanks.

@kentcdodds
Copy link
Contributor Author

Confirmed. This worked. Thanks!

@pandrodo
Copy link

Sorry for bumping that old issue, but when I using setup server with such handler:
rest.post('http://127.0.0.1:3000/getUserPoints', (req, res, ctx) => res(ctx.status(401))),
I receiving response with status text OK. Its not only for 401, but for other statuses too.

@kettanaito
Copy link
Member

Hey, @pandrodo. That's perfectly fine.

Can you clarify a few things:

  1. Which version of msw are you using?
  2. What version of NodeJS are you on?
  3. How do you make an actual request? (include the code)

@pandrodo
Copy link

  1. Version of msv is 0.21.3.
  2. Version of NodeJS is 14.15.0. Running server in my jest tests (version 24.9.0 shipped with react-scripts 3.4.3).
  3. Code is slightly simplified, but response still same with wrong statusText

@kettanaito
Copy link
Member

@pandrodo, thanks for including that. The issue confirmed as reproducible, the fix pull request is issued. Regression test was added, so we don't miss it occurring again. Will be released in the next minor version of msw.

@pandrodo
Copy link

@kettanaito, thanks for so fast fix.

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

Successfully merging a pull request may close this issue.

3 participants