mswjs / msw

Seamless REST/GraphQL API mocking library for browser and Node.js.
https://mswjs.io
MIT License
15.7k stars 503 forks source link

Consistent "TypeError: Failed to execute 'text' on 'Response': body stream already read" since v1.2.2 #1640

Closed JKapitein closed 1 year ago

JKapitein commented 1 year ago

Prerequisites

Environment check

Browsers

Chromium (Chrome, Brave, etc.)

Reproduction repository

https://github.com/JKapitein/msw-bodystream

Reproduction steps

yarn cypress-serve Can reach it locally from this point if you want, or yarn cypress-run

Current behavior

Failed to execute 'text' on 'Response': body stream already read

// src/utils/logging/serializeResponse.ts
var import_headers_polyfill9 = require("headers-polyfill");
async function serializeResponse(response2) {
  return {
    status: response2.status,
    statusText: response2.statusText,
    headers: (0, import_headers_polyfill9.flattenHeadersObject)((0, import_headers_polyfill9.headersToObject)(response2.headers)),
    body: await response2.text()
  };
}

Expected behavior

The request completes without throwing an error as in 1.2.1

wigsnes commented 1 year ago

I faced this issue also in version 1.2.1

Are you sure you did not just install the 1.2.2? If you use the ^ character in "msw": "^1.2.1" it will download 1.2.2. The bug disappeared for me when using 1.2.1.

AnielloFalcone commented 1 year ago

I faced this issue also in version 1.2.1

Are you sure you did not just install the 1.2.2? If you use the ^ character in "msw": "^1.2.1" it will download 1.2.2. The bug disappeared for me when using 1.2.1.

Thanks for replying. First of all sorry if I deleted the message but I didn't see you answer.

Second you're right, I am in a monorepo and didn't notice one of the packages was using the 1.2.2 and all the versions were then resolved to the last one.

I confirm the bug relates only to the 1.2.2

Joe-Degler commented 1 year ago

For what it's worth, this happens very consistently to me when using Storybook, but if I use the built version yarn build-storybook and serve that, it does not happen.

janwidmer commented 1 year ago

I also consistently have this bug using either version 1.2.2 (node 16.19.1) or next (node 18.16.0). Works correctly with version 1.2.1.

tencyle-sahar commented 1 year ago

I got the issue on any environment. downgrading to 1.2.1 (no carrot) fixed the issue.

kevelopment commented 1 year ago

Having the same issue when using msw version 1.2.2. with @storybook/test-runner. No errors when using msw 1.2.1 🤷

kettanaito commented 1 year ago

Thanks for reporting this. I believe it's caused by #1622 and the way the library reads the response body when logging it out (see this). We need to look closer at the response instance here, for during my observation it wasn't an actual Fetch API Response but it seems that we still need to clone it nonetheless (although calling .clone() on that internal response instance throws due to not being implemented).

mattcosta7 commented 1 year ago

Thanks for reporting this. I believe it's caused by #1622 and the way the library reads the response body when logging it out (see this). We need to look closer at the response instance here, for during my observation it wasn't an actual Fetch API Response but it seems that we still need to clone it nonetheless (although calling .clone() on that internal response instance throws due to not being implemented).

Is the stream the same between them rather than a copy of the stream? Maybe that's why it can happen?

yj-ang commented 1 year ago

I'm having the same error in v1.2.1. When I'm trying to fetch the same HTTP requests with different URL params (multiple useSWR hooks), the error occurred. If I keep only one HTTP request, then the error will be gone.

emrum01 commented 1 year ago

I am also getting the same error with v1.2.1. In my case, the error occurs when I make different HTTP requests, same path parameters and use multiple useQuery hooks at the same time. The error went away when I combined the two mock APIs used in the component error occured into one, but I'm having trouble because I originally wanted to separate these two APIs.

kettanaito commented 1 year ago

@mattcosta7, that may be the case, great suggestion. I haven't got to check this yet.

mairead commented 1 year ago

1622

I have the same issue. I've downgraded to 1.2.1. I see this TypeError: Failed to execute 'text' on 'Response': body stream already read when I mock multiple differently named queries that a return a response back to the same object in the Apollo cache. I assume it's trying to tell me that it can't write a response to the same object at the same time?

I managed to resolve the issue in another test by combining both queries into a single mock

But I was hoping to be able to mock all the requests in the same way my React app is making them, so I can make general requests to get large amounts in the handlers and then mock smaller changes in the actual test with worker.use to test different scenarios

kettanaito commented 1 year ago

I've opened a pull request to fix this: https://github.com/mswjs/msw/pull/1662. It's not perfect but I don't have the capacity right now to maintain 1.x at all. My effort goes to 2.x and this issue doesn't exist there.

Reviews are welcome. We can merge this this week.

kettanaito commented 1 year ago

Released: v1.2.3 🎉

This has been released in v1.2.3!

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.

janwidmer commented 1 year ago

@kettanaito Thanks for the fix. Are you sure, that it does not happen with 2.x though? If I recall it correctly, I also had the problem with the next release which would be version 2 right?

kettanaito commented 1 year ago

@janwidmer, that shouldn't happen because that entire function doesn't exist in v2. We also clone responses correctly in that version as I've learn a lot about when request/response instances should be cloned to make it work in all the areas MSW consumes them (resolvers, logging, events).

Could you give it a try at msw@next right now? I can confirm through tests this exact issue doesn't happen but perhaps you have a slightly different use case that causes it? That'd be nice to catch and fix. Thanks.

janwidmer commented 1 year ago

@kettanaito I will try it with the next release and check if i still have this problem.

janwidmer commented 1 year ago

@kettanaito I guess I was wrong with my comment, that I had it also with next. I rechecked my code and I saw that I have not triednext yet, so I guess, I had the problem with both node 16 and 18, but just with mws version 1.2.2.