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

Streams: warning messaging assumes a Response was cloned #399

Closed
KianNH opened this issue Feb 24, 2023 · 1 comment
Closed

Streams: warning messaging assumes a Response was cloned #399

KianNH opened this issue Feb 24, 2023 · 1 comment

Comments

@KianNH
Copy link
Contributor

KianNH commented Feb 24, 2023

Original issue: #775

It looks like the current warning is always assuming it was a Response object that was cloned when the same message is thrown when Request objects are cloned needlessly.

if (!wasRead) {
auto msg =
"Your worker called response.clone(), but did not read the body of both clones. "
"This is wasteful, as it forces the system to buffer the entire response body in memory, "
"rather than streaming it through. This may cause your worker to be unexpectedly "
"terminated for going over the memory limit. If you only meant to copy the "
"response headers and metadata (e.g. in order to be able to modify them), "
"use `new Response(response.body, response)` instead.";

@KianNH
Copy link
Contributor Author

KianNH commented May 18, 2023

An issue from Wrangler where a user was confused by the messaging: cloudflare/workers-sdk#3259

jasnell added a commit that referenced this issue May 18, 2023
The warning text was confusing since it would specifically mention
response.clone() even if the unused branch was created in other
ways (such as cloning a request or calling tee().)

Refs: #399
jasnell added a commit that referenced this issue May 25, 2023
The warning text was confusing since it would specifically mention
response.clone() even if the unused branch was created in other
ways (such as cloning a request or calling tee().)

Refs: #399
@KianNH KianNH closed this as completed Oct 26, 2023
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

No branches or pull requests

1 participant