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

🐛 BUG: Your worker called response.clone(), but did not read the body of both clones #3259

Open
GeorgeTailor opened this issue May 17, 2023 · 51 comments · May be fixed by #4861 or #4916
Open

🐛 BUG: Your worker called response.clone(), but did not read the body of both clones #3259

GeorgeTailor opened this issue May 17, 2023 · 51 comments · May be fixed by #4861 or #4916
Assignees
Labels
bug Something that isn't working pages Relating to Pages pages-dev Relating to `pages dev` command regression Break in existing functionality as a result of a recent change

Comments

@GeorgeTailor
Copy link

Which Cloudflare product(s) does this pertain to?

Wrangler

What version of Wrangler are you using?

3.0.0

What operating system are you using?

Windows

Describe the Bug

After upgrading to 3.0.0 new message started appearing in the console:

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.  

However, it does not say which exactly part of code is to blame.
I scanned my code and I do not call response.clone() directly anywhere, also, I have multiple levels of functions, making it hard to pin-point where the issue is and whether the issue is there at all.

@GeorgeTailor GeorgeTailor added the bug Something that isn't working label May 17, 2023
@KianNH
Copy link
Contributor

KianNH commented May 18, 2023

response.clone() isn't exactly what would have been called unfortunately - it appears for any .clone() on a Request or Response object where both bodies were not used.

@GeorgeTailor
Copy link
Author

I do not have any clone() neither on Request nor on Response objects anywhere in my code.
This must be something else

@zwily
Copy link

zwily commented May 19, 2023

I do not have any clone() neither on Request nor on Response objects anywhere in my code.
This must be something else

Are you using any frameworks? I'm using Remix, and see the same thing on any POST requests with bodies, and having a hard time tracking down where the clone is happening...

Also, it looks like this will happen when cloning any ReadableStream (without reading both sides), I believe, based on where it originates in workerd: https://github.com/cloudflare/workerd/blob/ed4c67df1b231270410a72cb0161c203e55b0bb6/src/workerd/api/streams/internal.c%2B%2B#L291-L298

@GeorgeTailor
Copy link
Author

@zwily no, I do not use any frameworks.
All I have is a _middleware.ts which calls next:

const response = await context.next();
// get username from headers
return new HTMLRewriter()
		.on("header", new UserElementHandler(username))
		.transform(response);

that's basically it

@wydengyre
Copy link
Contributor

wydengyre commented May 21, 2023

Getting this nuisance message with this minimal example:

export const onRequestPost: PagesFunction = async (context) => { };

Every POST results in the message appearing twice.

For what it's worth, adding some code that uses context.request makes the warning only appear a single time.

@roman01la
Copy link

Seeing the same warning when using D1 with Wrangler v3

@houmark
Copy link

houmark commented May 23, 2023

Seeing the same warning when using D1 with Wrangler v3

Same...

@manstfu
Copy link

manstfu commented May 24, 2023

Seeing the same warning when using D1 with Wrangler v3

Same...

same, hop the train!

@RichiCoder1
Copy link

RichiCoder1 commented May 26, 2023

Same. I can confirm that I'm not cloning anywhere, seems to reproduce with just adding a pages function. Seems like this might be a bug in the pages functions logic?

Edit: is it maybe the pages asset server? https://github.com/cloudflare/workers-sdk/blob/main/packages/pages-shared/asset-server/handler.ts#L340

@shakibhasan09
Copy link

Hi, I'm getting the same warning, I'm using wrangler@3.0.1 os: Ubuntu 23.04

@pmbanugo
Copy link

pmbanugo commented Jun 7, 2023

any update on this @KianNH ?

@pmbanugo
Copy link

pmbanugo commented Jun 7, 2023

@RichiCoder1

Edit: is it maybe the pages asset server? https://github.com/cloudflare/workers-sdk/blob/main/packages/pages-shared/asset-server/handler.ts#L340

I think that might be the cause

@EliBates
Copy link

EliBates commented Jun 10, 2023

Having the same issue, using simple hono router, with v3 and D1 on MacOS.

@shakibhasan09
Copy link

@EliBates you can upgrade the wrangler sdk to @3.1.0

@zwily
Copy link

zwily commented Jun 10, 2023

@EliBates you can upgrade the wrangler sdk to @3.1.0

Was it addressed?

@pmbanugo
Copy link

@EliBates you can upgrade the wrangler sdk to @3.1.0

Nothing changed with regards to this in wrangler 3.1.0

@bpleco
Copy link

bpleco commented Jun 14, 2023

I'm on 3.1.0 and still the same issue in a bare bones project

@penalosa penalosa added the pages Relating to Pages label Jun 15, 2023
@bruceharrison1984
Copy link

bruceharrison1984 commented Jun 23, 2023

I'm seeing this as well with 3.1.1 and pages functions. Not cloning the response anywhere. The following function runs after a couple layers of _middleware.ts.

export const onRequestPost = async ({ data }) => {

  if (!data.userProfile.id) throw new Error('UserProfile ID is null!');

  const apiKey = await data.db
    .insert(userApiKeys)
    .values({
      ownerId: data.userProfile.id,
    })
    .returning()
    .get();

  return new Response(JSON.stringify(apiKey), {
    headers: { 'Content-Type': 'application/json' },
    status: 200,
  });
};

This error only seems to be thrown on a POST request, GET requests seem unaffected.

It also appears this message is generated at each level of _middleware that your request passes through. Simply reading the request body at each layer will squelch one instance of the message.

Something as simple as the following is enough:

const noopBody = request.json();

I was able to remove most of the recurrent message, but not all by using this method. Definitely seems like something is off here.

@joaoguidev
Copy link

joaoguidev commented Jul 18, 2023

Im having the same problem here since Remix 18.1 and can confirm that this problem persists on Remix 2.0. Any update?

@VicSmithVercel
Copy link

VicSmithVercel commented Jul 25, 2023

I am having the same issue:
Using the following npm packages.
@cfworker/cosmos
and
uuid

Do any of you having the issue have these in node_modules?

@eugene-haas
Copy link

I am having the same problem.
Updating from wrangler 3.3.0 to 3.4.0 has the same issue.
The usage code is below.
_middleware.js

const user = async ({ request, next, env }) => {
const url = new URL(request.url);
const cookie = request.headers.get('cookie');
return next();
};

export const onRequest = [user];

It is doubtful whether it can be applied to actual service in this state.

Has anyone solved it?

@shakibhasan09
Copy link

Nobody fixed that yet?

@alexmacarthur
Copy link

I'm having the same issue, no matter if I clone or .tee() the body.

@KrisBraun
Copy link

KrisBraun commented Aug 10, 2023

I'm getting five copies of this error on every call to a resource-only Remix action:

export const action = async () => { return new Response("Not implemented", { status: 501 }); };

Your worker created multiple branches of a single stream (for instance, by calling response.clone() or request.clone()) but did not read the body of both branches. This is wasteful, as it forces the system to buffer the entire stream of data 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 request or response headers and metadata (e.g. in order to be able to modify them), use the appropriate constructors instead (for instance, new Response(response.body, response), new Request(request), etc).

Edit: Found this Remix issue.

@danboyle8637
Copy link

I'm using Astro though that should not matter when the request gets into the functions context.

Using the property bodyUsed I confirmed I have only read my request body one time. But I still get this warning about creating multiple streams.

It has to be a Pages functions bug of some sort.

I will probably test creating a Worker separate from Pages and use bindings to forward the request to the Worker and see what happens.

I'll report back but it will have to be later today or tomorrow.

@klaemo
Copy link

klaemo commented Sep 11, 2023

We just tried wrangler 3.7.0 and can still observe this warning.

@GregBrimble @JacobMGEvans do you need anything from the community in order to be able to tackle this?

@OnurGvnc
Copy link

I think the lines below might be causing the problem, but I'm not sure.

request: new Request(request.clone()),

request: new Request(request.clone()),

@CarmenPopoviciu
Copy link
Contributor

this issue seems to have been fixed in wrangler version 3.15.0. Is anyone still experiencing issues?

@CarmenPopoviciu
Copy link
Contributor

Closing as this behaviour was fixed as of wrangler version 3.15.0. If you still experience issues, please feel free to re-open

@ivotkv
Copy link

ivotkv commented Dec 5, 2023

Hi @CarmenPopoviciu, I am running wrangler 3.18.0 and I am still having this issue. I have implemented code similar to this example:

https://developers.cloudflare.com/workers/examples/cache-post-request/

Specifically this line is causing the warnings:

const body = await request.clone().text();

However, removing the .clone() causes a TypeError: Body has already been used. error, so clearly there is no body left unread and the warning is a false positive.

Let me know if you need more information!

@tj-noor
Copy link

tj-noor commented Jan 6, 2024

Ran into a similar issue with Remix & Cloudflare Pages (wrangler@3.22.3) where your proposed fix works. If we can reopen this issue and you need anything, I'll be available to assist.

✘ [ERROR] Your worker created multiple branches of a single stream (for instance, by calling response.clone() or request.clone()) but did not read the body of both branches. This is wasteful, as it forces the system to buffer the entire stream of data 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 request or response headers and metadata (e.g. in order to be able to modify them), use the appropriate constructors instead (for instance, new Response(response.body, response), new Request(request), etc).

Remix has a pre-existing open issue @ remix-run/remix#7032

wrangler 3.22.3
@remix-run/dev 2.4.1
@remix-run/cloudflare-pages 2.4.1
@remix-run/cloudflare 2.4.1
@remix-run/css-bundle 2.4.1 
@remix-run/react 2.4.1

@saidelimam
Copy link

Looks like this error reappeared in version 3.22.3
I got the stream error message multiple times on a code than never calls response/request.clone()

@cryptiddddd
Copy link

same as @saidelimam, i'm on 3.22.4, at no point do i call response/request.clone(). i've been searching around for a while assuming this is my own error, but the only interaction i have with the request object is collecting the form data.

figured i should leave a comment to validate that the issue is still happening, or to maybe discover my own mistake.

@heathdutton
Copy link

heathdutton commented Jan 14, 2024

Can confirm, same issue in wrangler 3.22.3 / 3.22.4
Using functions with react-app and proxy locally. Does not matter what the function is I'm calling.

@davidysoards
Copy link

davidysoards commented Jan 21, 2024

If I roll back to 3.22.1 I don't see this error, but anything >=3.22.2 I'm getting it on every request to any Pages Function.

@davidysoards
Copy link

Can this issue still be re-opened or do we need to create a new issue? @CarmenPopoviciu

@WebShapedBiz
Copy link

I can confirm it's still happening with Wrangler v3.23.0 and the "Hello world" example from the Pages Functions docs.

@ozanmakes
Copy link

I can confirm the downgrading wrangler to 3.22.1 gets rid of the error. The change in #4185 does silence the error as well, but my app does not work anymore. I really hope this error can be fixed in wrangler-side, it's making local dev with CF Pages very cumbersome.

@aroman
Copy link

aroman commented Jan 26, 2024

@lrapoport-cf looks like this is a regression

per your comment here, you mentioned we should feel free to add the 'regression' label when appropriate ourselves, however, non-CF employees don't seem to have those permissions (probably a wise decision). so, tagging you here for visibility/so the tag can get added

@lrapoport-cf lrapoport-cf added the regression Break in existing functionality as a result of a recent change label Jan 26, 2024
@lrapoport-cf lrapoport-cf reopened this Jan 30, 2024
@lrapoport-cf
Copy link
Contributor

@aroman thanks for the ping :) i've added the regression label and re-opened the issue, and a fix is being worked on in #4861 👍

@russelldavis
Copy link

Here's a simple fix: https://gist.github.com/russelldavis/6b1a1bf15898655ec18037bd737d075d

If you have middleware that reads the body of the request, you'll need to update it to call request.clone().

It'd be great if cloudflare would fix this (it's a breaking change, but that's what major versions are for), but in the meanwhile, you can fix this in your own project by applying the patch above using patch-package or pnpm patch.

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