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

fix(fromTraffic): clone response for subsequent requests #67

Merged
merged 2 commits into from
Oct 29, 2024

Conversation

danielegarciav
Copy link
Contributor

Repeated calls to unique request handlers would return unreadable responses that were already consumed. By cloning the response before returning, each returned response is able to be read as normal.

Fixes #66

@@ -33,4 +33,11 @@ it('respects the response sequence when repeatedly requesting the same endpoint'
},
},
])
// Any subsequent requests to the same endpoint should return a readable response.
const repeatedHandlerResponse = await handlers[1]!.run({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's put this in a separate test!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, created new separate test, response-cloning.test.ts, with it's own archive and request script (duplicates of request-timing).

@@ -69,7 +69,7 @@ export function fromTraffic(
await delay(entry.time)
}

return response
return isUniqueHandler ? response.clone() : response
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are the repercussions of always cloning the response?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have detailed the repercussions of cloning (and not cloning) the response in this comment in the original issue. In brief summary, "cloning" a response does not duplicate the response data in memory, it points to the same internal buffer.

The response data is held in memory until there are no more unread responses. This should be fine since we are already keeping response data in memory for all generated handlers when we call toResponse.

@danielegarciav danielegarciav force-pushed the clone-unique-handler-responses branch 2 times, most recently from 17352bb to 187d01d Compare October 25, 2024 18:59
Repeated calls to unique request handlers would return unreadable
responses that were already consumed. By cloning the response before
returning, each returned response is able to be read as normal.

Fixes mswjs#66
@kettanaito kettanaito force-pushed the clone-unique-handler-responses branch from ba81836 to c6ff13e Compare October 29, 2024 11:09
@kettanaito kettanaito changed the title fix(fromTraffic): clone unique handler responses fix(fromTraffic): clone response for subsequent requests Oct 29, 2024
Copy link
Member

@kettanaito kettanaito left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great to me! 👏
I've added withHandlers() in tests to make the requests to say consistent.

@kettanaito kettanaito merged commit 169f416 into mswjs:main Oct 29, 2024
1 check passed
@kettanaito
Copy link
Member

Released: v0.3.2 🎉

This has been released in v0.3.2!

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


Predictable release automation by @ossjs/release.

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

Successfully merging this pull request may close these issues.

Generated HAR handlers cannot respond to the same URL more than once
2 participants