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

globalThis.fetch patch setting property on self leads to 504s #65381

Open
mhart opened this issue May 5, 2024 · 0 comments
Open

globalThis.fetch patch setting property on self leads to 504s #65381

mhart opened this issue May 5, 2024 · 0 comments
Labels
bug Issue was opened via the bug report template. Runtime Related to Node.js or Edge Runtime with Next.js.

Comments

@mhart
Copy link

mhart commented May 5, 2024

Link to the code that reproduces this issue

https://github.com/mhart/nextjs-patch-bug

Live at: https://nextjs-bug-eight.vercel.app/ (try reloading a few times)

To Reproduce

  1. Include any function or module that monkey-patches globalThis.fetch after Next.js has already patched it
  2. Observe that Next.js will patch it again, because the __nextPatched property no longer exists on the patched fetch and that's what it checks for.
  3. So now globalThis.fetch is wrapped multiple times with the same Next.js patch, sandwiching any other patches in between.

See repo for example reproduction. Reloads result in 504s, errors in the log are:

Error: Your function was stopped as it did not return an initial response within 25s

Current vs. Expected behavior

Current behaviour is that fetch gets wrapped multiple times, potentially again and again each request in the edge runtime. One outcome of this is that fetch calls deadlock (unclear exactly why – something to do with the internals of the patch implementation), leading to 504 errors.

Expected behaviour is that it only wraps once, and then fetch calls (even if wrapped by another patch), will succeed.

Provide environment information

Operating System:
  Platform: darwin
  Arch: arm64
  Version: Darwin Kernel Version 23.4.0: Fri Mar 15 00:10:42 PDT 2024; root:xnu-10063.101.17~1/RELEASE_ARM64_T6000
  Available memory (MB): 32768
  Available CPU cores: 10
Binaries:
  Node: 20.12.2
  npm: 10.5.2
  Yarn: 4.1.1
  pnpm: 9.0.6
Relevant Packages:
  next: 14.2.3 // Latest available version is detected (14.2.3).
  eslint-config-next: N/A
  react: 18.3.1
  react-dom: 18.3.1
  typescript: 5.4.5
Next.js Config:
  output: N/A

Which area(s) are affected? (Select all that apply)

Runtime

Which stage(s) are affected? (Select all that apply)

Vercel (Deployed)

Additional context

Suggestion is to not set a property on the thing you're patching to then check if it's been patched or not. Perhaps set a symbol on globalThis or similar?

Also probably worth looking into why the patch can't be applied recursively in the first place (ie, what's causing the deadlocks).

Also, ideally, remove the patch altogether and provide the functionality in another way 🙏

@mhart mhart added the bug Issue was opened via the bug report template. label May 5, 2024
@github-actions github-actions bot added the Runtime Related to Node.js or Edge Runtime with Next.js. label May 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue was opened via the bug report template. Runtime Related to Node.js or Edge Runtime with Next.js.
Projects
None yet
Development

No branches or pull requests

1 participant