Remix redirect headers are broken (`x-remix-redirect`)

sebastien-comeau commented 2 months ago

Prerequisites

[X] I confirm my issue is not in the opened issues
[X] I confirm the Frequently Asked Questions didn't contain the answer to my issue

Environment check

[X] I'm using the latest msw version
[X] I'm using Node.js version 18 or higher

Node.js version

v20.17.0

Reproduction repository

https://github.com/sebastien-comeau/remix-with-msw

Reproduction steps

Start the application in development mode using npm run dev. Then, click the Go to movies button, which triggers a Remix action and returns a redirect response to /movies. This issue doesn't occur with MSW v2.4.3, so we suspect it was introduced with the @mswjs/interceptors update in #2268.

Current behavior

When redirecting to /movies from the server action, the browser URL is incorrectly set to /movies,%20/movies. The Remix redirect response header shows x-remix-redirect: /movies, /movies.

Expected behavior

The browser URL should correctly display /movies when redirecting from the server action. The Remix redirect response header should be x-remix-redirect: /movies.

BraedenKilburn commented 2 months ago

I found an issue where my response resolver would access the request body as seen in the docs here:

http.post('/post/:postId', async ({ request, params, cookies }) => {
  const { postId } = params
  const requestBody = await request.json()
})

What I had to do was add clone the request object like so:

http.post('/post/:postId', async ({ request, params, cookies }) => {
  const { postId } = params
  const requestBody = await request.clone().json()
})

This resolved my error. I found this from this issues thread here. I don't know if this will fix your problem but I wanted to share my findings on how to fix an error I had after updating to 2.4.4. An issue with this is that I lose the typings on my request body after I clone the request.

kettanaito commented 2 months ago

Thanks for reporting this.

I believe it's a bug on our end. We need to copy the request's body safely when redirecting with body. This is a task for interceptors.

Edit: This was not related to what I thought it was at all. See the root cause in https://github.com/mswjs/interceptors/issues/631.

stevensacks commented 2 months ago

Can confirm this is happening to me, as well.

export const action: ActionFunction = async ({request}) => {
  const formData = await request.formData();
  await updateThing(formData, request); // fetch to endpoint mocked by MSW
  return redirect('/things', {status: 303});
};

Should redirect from /things/1 to /things, but instead redirects to /things,%20/things

sndrem commented 2 months ago

I have the same behavior in my app, as described in the previous comments.

kettanaito commented 2 months ago

This is also likely caused by https://github.com/mswjs/interceptors/issues/631. Based on the reported broken Remix redirect header x-remix-redirect: /movies, /movies, it looks like the next location is being appended to the header value, instead of replacing it.

kettanaito commented 2 months ago

Released: v2.4.7 🎉

This has been released in v2.4.7!

📄 Release notes
📦 npm package

Make sure to always update to the latest version (npm i msw@latest) to get the newest features and bug fixes.

Predictable release automation by @ossjs/release.

iamtomcat commented 2 months ago

@kettanaito I'm still seeing the same behavior with the headers being swallowed by the interceptors somewhere.

kettanaito commented 2 months ago

@iamtomcat, please open a new issue and submit a reproduction repo. This bug has been fixed.

stevensacks commented 2 months ago

@kettanaito Unfortunately, it's not completely fixed. I upgraded to 2.4.8 and some of my endpoints are working (simple CRUD), but my login API call doesn't even make it to the MSW handler, and immediately throws the following error from the interceptors.

TypeError: Cannot set property headers of #<_Request> which has only a getter
      at Object.construct (file:///Users/stevensacks/Development/gaia/framework/remix/node_modules/@mswjs/interceptors/lib/node/chunk-RWGRRMVU.mjs:217:27)
      at Authenticator.authenticate (/Users/stevensacks/Development/gaia/framework/remix/node_modules/remix-auth/build/authenticator.js:68:41)

Downgrading back to 2.4.2 resolves the issue.

kettanaito commented 2 months ago

The originally reported bug is fixed, there's an automated test to prove that. If you are experiencing a related issue, I understand how it may seem the same on the surface.

Please see this: https://github.com/mswjs/msw/issues/2269#issuecomment-2355600332

stevensacks commented 2 months ago

@kettanaito Ok I've done that. I didn't know how to report the bug exactly, so I just named it after the error message.

https://github.com/mswjs/msw/issues/2290

mswjs / msw