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 created multiple branches of a single stream #685

Open
1 task
Mecanik opened this issue Feb 27, 2024 · 13 comments
Open
1 task
Labels
blocked by external This can't be currently solved because of external factors (Vercel/Next, Pages, etc...) bug Something isn't working

Comments

@Mecanik
Copy link

Mecanik commented Feb 27, 2024

next-on-pages environment related information

I was sent here after the guys from Clerk checked this out, apparently this is an issue with CF/Next.

Description

This is happening when using Cloudflare Pages after logging in:

✨ Compiled Worker successfully
 ⛅️ wrangler 3.28.1
-------------------
Using vars defined in .dev.vars
Your worker has access to the following bindings:
- Vars:
  - NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY: "(hidden)"
  - CLERK_SECRET_KEY: "(hidden)"
βŽ” Starting local server...
Parsed 1 valid header rule.
[wrangler:inf] Ready on http://localhost:8788
✘ [ERROR] INFO: Clerk: The request to / is being redirected because there is no signed-in user, and the path is not included in `ignoredRoutes` or `publicRoutes`. To prevent this behavior, choose one of:


  1. To make the route accessible to both signed in and signed out users, pass `publicRoutes: ["/"]`
  to authMiddleware
  2. To prevent Clerk authentication from running at all, pass `ignoredRoutes:
  ["/((?!api|trpc))(_next.*|.+\.[\w]+$)", "/"]` to authMiddleware
  3. Pass a custom `afterAuth` to authMiddleware, and replace Clerk's default behavior of
  redirecting unless a route is included in publicRoutes

  For additional information about middleware, please visit https://clerk.com/docs/nextjs/middleware
  (This log only appears in development mode, or if `debug: true` is passed to authMiddleware)


[wrangler:inf] GET / 307 Temporary Redirect (293ms)
[wrangler:inf] GET / 401 Unauthorized (3ms)
[wrangler:inf] GET /favicon.ico 304 Not Modified (8ms)
[wrangler:inf] GET / 200 OK (5276ms)
✘ [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).


[wrangler:inf] GET /vercel.svg 304 Not Modified (6ms)
[wrangler:inf] GET /next.svg 304 Not Modified (7ms)
[wrangler:inf] GET /_next/static/chunks/983-c740277b6d3702da.js 200 OK (8ms)
[wrangler:inf] GET /_next/static/chunks/app/page-cf19a580b1879b0e.js 200 OK (6ms)

Reproduction

You can download this example project and test: https://github.com/Mecanik/clerk-nextjs-cloudflare

Pages Deployment Method

None

Pages Deployment ID

No response

Additional Information

Thanks

Would you like to help?

  • Would you like to help fixing this bug?
@Mecanik Mecanik added the bug Something isn't working label Feb 27, 2024
@Mecanik
Copy link
Author

Mecanik commented Feb 28, 2024

@dario-piotrowicz Where are you... πŸ˜ƒ

@Maronato
Copy link

This is a bug in wrangler, not next-on-pages:
cloudflare/workers-sdk#3259

@dario-piotrowicz
Copy link
Member

@Mecanik sorry I'm lately been pulled on some other projects and haven't had time to take care of issues here as much as I would like πŸ™‡ πŸ˜“

I'll try to find the time and give this a look soon πŸ™‡

By the way, I am not completely sure why but this "error" from our runtime is actually a warning since when it happens nothing is really broken πŸ˜• (but resources are likely being wasted), so even if you see it, everything should still work as you'd expect it to, no?

@dario-piotrowicz
Copy link
Member

dario-piotrowicz commented Feb 28, 2024

This is a bug in wrangler, not next-on-pages: cloudflare/workers-sdk#3259

The issue there is specific for when you use the functions/ directory (as the logic that takes that code and bundles it into a worker makes it so that requests get cloned more than they should).

Next-on-pages does not use the functions/ directory but uses the Pages advance mode instead, so I do not believe this to be an issue in wrangler, I think that the issue either resides in either:

  • next-on-pages (unlikely since we never clone requests)
  • clerk (a bit unlikely since they send Mecanik here....?)
  • Next.js (likely by exclusion?)

@dario-piotrowicz
Copy link
Member

quick update, I've started looking into this and by debugging it seems like the error is triggered when you call clerk's currentUser, the issue however seems to come form Next.js code, specifically this

what I think it's happening (I not 100% sure yet) is that currentUser performs a fetch and Next.js eagerly clones the response so that the app can read its body as many times as it wants, thus performing this cloning even when not needed, causing the error to show up

(this is actually quite similar to what happens in wrangler with the functions/ directory! the only difference being that we do that for the request and not the response)

@dario-piotrowicz
Copy link
Member

Ok, I think my above assessment was correct, please have a look at this repository: https://github.com/dario-piotrowicz/nextjs-cachedfetch-clone-issue-repro

It shows that the issue ultimately lies in Next.js' cachedFetch implementation.

I'll open an issue in the Next.js repo regarding this, but honestly I am not sure if they'll be interested in fixing this... we'll have to see πŸ˜“


Basically what's happening here is that Clerk is most likely fetching from the same endpoint more than once thus kicking off the cached fetch behavior offered by Next.js. This behavior however triggers the issue.

If Clerk could avoid fetching from the same endpoint more than once that would solve this issue, but I don't think they really should, as the root issue is really a Next.js one.

Needless to say, there isn't much that can be done regarding this on our side πŸ₯²

@dario-piotrowicz dario-piotrowicz added the blocked by external This can't be currently solved because of external factors (Vercel/Next, Pages, etc...) label Feb 29, 2024
@Mecanik
Copy link
Author

Mecanik commented Feb 29, 2024

Thanks @dario-piotrowicz . Things just get better and better...

So this issue occurs only in currentUser or every fetch they do? How does this affect the billing if you were to deploy a project like this?...

@dario-piotrowicz
Copy link
Member

dario-piotrowicz commented Mar 1, 2024

Hi @Mecanik, sorry above I was slightly mistaken, the issue is not that clerk fetches from the same endpoint but that they fetch at all! (see my revised reproduction: dario-piotrowicz/nextjs-cachedfetch-clone-issue-repro@0826162)

So yeah it's fully a Next.js issue that clerk can do nothing about


So this issue occurs only in currentUser or every fetch they do?

it occurs in every single fetch that happens on the server


I just checked with the team and I am bringing the possibility to remove this warning as, I believe is unnecessary and doesn't really help much (on the contrary it creates confusions and worries), I'll keep you posted

Also I checked with the wrangler team and this, comes from the runtime as a warning not a real error: https://github.com/cloudflare/workerd/blob/5c9519e2faaeaf0f0c7ad1e184af9c29d060b459/src/workerd/api/streams/internal.c%2B%2B#L380
but it is displayed as an error by wrangler... I am yet to understand if this is intentional of it should be fixed πŸ˜•


How does this affect the billing if you were to deploy a project like this?...

I'll have to check with the team, I think that the effect on billing should be negligible at most, again since I think that this is not a real issue, but I'll try to get more infos for you πŸ‘

@Mecanik
Copy link
Author

Mecanik commented Mar 1, 2024

Hi @Mecanik, sorry above I was slightly mistaken, the issue is not that clerk fetches from the same endpoint but that they fetch at all! (see my revised reproduction: dario-piotrowicz/nextjs-cachedfetch-clone-issue-repro@0826162)

So yeah it's fully a Next.js issue that clerk can do nothing about

So this issue occurs only in currentUser or every fetch they do?

it occurs in every single fetch that happens on the server

I just checked with the team and I am bringing the possibility to remove this warning as, I believe is unnecessary and doesn't really help much (on the contrary it creates confusions and worries), I'll keep you posted

Also I checked with the wrangler team and this, comes from the runtime as a warning not a real error: https://github.com/cloudflare/workerd/blob/5c9519e2faaeaf0f0c7ad1e184af9c29d060b459/src/workerd/api/streams/internal.c%2B%2B#L380 but it is displayed as an error by wrangler... I am yet to understand if this is intentional of it should be fixed πŸ˜•

How does this affect the billing if you were to deploy a project like this?...

I'll have to check with the team, I think that the effect on billing should be negligible at most, again since I think that this is not a real issue, but I'll try to get more infos for you πŸ‘

Thank you for looking into this, really appreciated. I do not really understand why "caching" necessary on a fetch... but I tried export const forceCache = 'force-no-store' without success since it won't compile.

I do not have this issue with Auth0, however I am not using Auth0 because it has many limitations in terms of branding, etc; and it's quite expensive when scaling. So that's why I am here trying to use Clerk, as an alternative for registration/authentication for NextJS.

But the absolute real reason I am doing this is because I created my own user system with next-on-pages, but I have a ton of "hydration" errors (which make no sense to me). I would have preferred to use my own system, with D1, where everything is under control and works properly...

If this cannot be resolved, I will have to let both Auth0 and Clerk and go with mine. Probably going to publish the repo so others can benefit and why not contribute.

Thanks again.

@anonymouscatcher
Copy link

I have same issue here as well, I'm also getting worker limit exceeded error after deployment and after refreshing the page for couple of times. Not sure if this is related to this worker thing

@Mecanik
Copy link
Author

Mecanik commented Mar 5, 2024

I have same issue here as well, I'm also getting worker limit exceeded error after deployment and after refreshing the page for couple of times. Not sure if this is related to this worker thing

Most likely because of the bug cloning responses...

@anonymouscatcher
Copy link

Hi @dario-piotrowicz
Do you think this is something that will be fixed any time soon? We are on paid plan on CF and this is blocking us from deploying to CF.

@dario-piotrowicz
Copy link
Member

Hi @anonymouscatcher, I'm sorry that you're being blocked

From your comment

I have same issue here as well, I'm also getting worker limit exceeded error after deployment and after refreshing the page for couple of times. Not sure if this is related to this worker thing

It seems like you're getting a worker limit exceeded error? that is, I think, likely something unrelated, could you maybe open a new issue sharing the error message you get and as much info as you can? (and I'll look into it right away)

(Regarding the multiple branches issue, I'll try to think of a solution, but I am afraid that that is likely not achievable and that it would require a breaking change in Next.js itself πŸ˜•)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked by external This can't be currently solved because of external factors (Vercel/Next, Pages, etc...) bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants