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

Crash when navigating back to a page using the browser's back button, after the page has been revalidated #61336

Closed
jviide opened this issue Jan 29, 2024 · 27 comments · Fixed by #61687 or #62383
Labels
bug Issue was opened via the bug report template. linear: next Confirmed issue that is tracked by the Next.js team. locked

Comments

@jviide
Copy link
Contributor

jviide commented Jan 29, 2024

Link to the code that reproduces this issue

https://github.com/jviide/my-minimal-nextjs-issue-reproduction

To Reproduce

The reproduction is also available at CodeSandbox: https://codesandbox.io/p/github/jviide/my-minimal-nextjs-issue-reproduction/main

  1. Install the repository dependencies with npm i and start the dev server with npm run dev.
  2. Open the root page / on the browser.
  3. Click the link on the root page, taking you to the /foo page.
  4. Click the button on the /foo page. This submits the form and runs an action that calls revalidatePath("/", "layout").
  5. After the form has been submitted succesfully, navigate back to the root page / by using the browser's back button.
    • At this point the browser makes numerous GET requests to / which can be observed in the browser's inspector.
  6. After some seconds (usually around 5 in my environment) the following error is shown: "Application error: a client-side exception has occurred (see the browser console for more information)."

Current vs. Expected behavior

When the browser's back button is pressed after submitting the form, the root page at / should be opened.

Instead the browser makes a burst of GET calls to the root page (observable on the browser's network inspector), failing after a while.

Provide environment information

Operating System:
  Platform: linux
  Arch: arm64
  Version: #1 SMP Fri Jan 19 08:53:17 UTC 2024
Binaries:
  Node: 20.11.0
  npm: 10.2.4
  Yarn: 1.22.21
  pnpm: 8.15.0
Relevant Packages:
  next: 14.1.1-canary.20 // Latest available version is detected (14.1.1-canary.20).
  eslint-config-next: N/A
  react: 18.2.0
  react-dom: 18.2.0
  typescript: N/A
Next.js Config:
  output: N/A

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

App Router

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

next dev (local), next start (local)

Additional context

The issue seems to reproduce more easily when root page's data fetching is not instantaneous. The root page implementation simulates this by artificially sleeping for 500 milliseconds.

Tested on Chrome 121 and Safari 17.3 on macOS 14.3.

Next.js 14.0.0 was the earliest version I tested, and the issue also appears there.
Update: The issue can't be reproduced with 14.0.5-canary.5, but can be reproduced with 14.0.5-canary.6.

NEXT-2438

@jviide jviide added the bug Issue was opened via the bug report template. label Jan 29, 2024
@lswinblad
Copy link

lswinblad commented Jan 31, 2024

I've got a similar issue. Opening up your example in a normal browser window, going to /foo and hitting refresh (as opposed to submitting the form), then using the browser's back button the application crashes with the following error message

Uncaught Error: async/await is not yet supported in Client Components, only Server Components.
This error is often caused by accidentally adding 'use client' to a module that was originally written for the server.

@yami-beta
Copy link

I tried with the following repository.
https://github.com/yami-beta/nextjs-reload-back-issue

RSC Payload fetch seems to be looping when press browser's back after reloading page.

demo.mov

@mordechaim
Copy link

Same issue, happens to me when setting a cookie in a server action.

Seems to be happening when the router cache is cleared (which happens both when setting a cookie or revalidating).

@CarlosBalladares
Copy link

CarlosBalladares commented Feb 2, 2024

I am seeing this issue too i go back after refreshing and i get swarmed with requests that crashed my page and my backend, it happens when the page awaits for too long when going back (i replaced some code with an awaited timeout). For example you can wait for a fetch request if it takes too long the client side starts spamming requests.

image

@CarlosBalladares
Copy link

CarlosBalladares commented Feb 2, 2024

A workaround i found was to reduce the number of async function i await for before rendering the page and move my vercel functions to the region closest to my database with the regions attribute in vercel.json

@mordechaim
Copy link

Workaround: Use a loading.js file for that route.

@ztanner
Copy link
Member

ztanner commented Feb 6, 2024

Hi there -- thanks for the report. I just verified this against the latest canary and it seems to be resolved by #61687 (despite this reproduction being different, it appears to have the same underlying cause).
Let me know if you're still encountering this after upgrading and I can take another look! However I did check before/after that commit and noticed it only crashed in the before case.

@jviide
Copy link
Contributor Author

jviide commented Feb 6, 2024

@ztanner Thank you! I updated my original reproduction code to use the latest canary version (14.1.1-canary.38), and couldn't see bursts of duplicate requests anymore 👍 The "Application error: a client-side exception has occurred (see the browser console for more information)" issue also seemed to be resolved until I increased the 500ms loading delay just a bit. Then the error started to appear again.

Here is the modified reproduction code using 14.1.1-canary.38 with the loading delay increased to 1500ms for good measure: https://github.com/jviide/next-issue-reproduction2. The corresponding CodeSandbox can be found at https://codesandbox.io/p/github/jviide/next-issue-reproduction2/main. On CodeSandbox it's probably easiest to open the rendered page in a new browser tab so the back button works a bit more consistently.

The reproduction steps remain the same:

  1. Open the root page / on the browser.
  2. Click the link on the root page, taking you to the /foo page.
  3. Click the button on the /foo page. This submits the form and runs an action that calls revalidatePath("/", "layout").
  4. After the form has been submitted succesfully, navigate back to the root page / by using the browser's back button.
  5. After some seconds (usually around 5 in my environment) the following error is shown: "Application error: a client-side exception has occurred (see the browser console for more information)."

For me the error message appears most of the time, but every now and then there is no error. In such cases retrying the reproduction steps usually errors again.

@ztanner
Copy link
Member

ztanner commented Feb 6, 2024

Thanks for the updated repro @jviide! I'll reopen this for now while we investigate the cause

@ztanner ztanner reopened this Feb 6, 2024
@feedthejim feedthejim added the linear: next Confirmed issue that is tracked by the Next.js team. label Feb 12, 2024
@xvvvyz
Copy link

xvvvyz commented Feb 18, 2024

Suggestions so far don't work for me.

I'm using parallel route modals and am getting this error on router.back() after the router cache is cleared. Current thought is to use router.push() but the only way that I know of to do this for modals that can be opened from any page is to use headers().get('referer') to determine the previous page which is probably not reliable.

Edit: I suppose I could manually set a back query param.

@Wei102193
Copy link

same issue godamn it drives me nuts

@mordechaim
Copy link

@Wei102193 I suggested above to add a loading.js file for the affected route. Did you observe this helping or not?

@Wei102193
Copy link

@Wei102193 I suggested above to add a loading.js file for the affected route. Did you observe this helping or not?

seems to work, why? also don't really want that whole page to be loading.

@mordechaim
Copy link

I don't know precisely why, but I'm guessing it's some conflict with the client-side router cache trying to display content that has been purged and somehow running into a fetch loop.

By adding the loading.js file it somehow immediately falls back to the loading screen and stops the infinite loop.

All speculation, I have no understanding of the internals.

@Wei102193
Copy link

I don't know precisely why, but I'm guessing it's some conflict with the client-side router cache trying to display content that has been purged and somehow running into a fetch loop.

By adding the loading.js file it somehow immediately falls back to the loading screen and stops the infinite loop.

All speculation, I have no understanding of the internals.

Actually loading.js is the only way that works for me atm. Gonna use loading till they fix this issue. Thanks!

@kamilkazui
Copy link

Next.js 14.0.0 was the earliest version I tested, and the issue also appears there.

I've also been dealing with this issue on next@14.1.0 but when I rolled back to next@14.0.0 problem just went away. Also @jviide second sandbox works fine on next@14.0.0 for me. Intereseting that it breaks for you.

revalidatePath.mov

Also fun fact - Firefox works ok on desktop on both next versions (macOS).

@jviide
Copy link
Contributor Author

jviide commented Feb 21, 2024

@kamilkazui You're right, it seems that I was mistaken. I can't reproduce the issue with 14.0.0 either. My apologies.

After some bisecting it seems that the earliest version that the issue can be reproduced with is 14.0.5-canary.6. I updated the original issue description accordingly.

ztanner added a commit that referenced this issue Feb 27, 2024
### What
When the popstate action is fired (as is the case in a browser back
button), if the page you're going back to has missing cache node data,
the router will crash.

### Why
Almost all router actions will suspend at the app-router level with the
exception of `ACTION_RESTORE`. This was to address an issue where
suspending in the router would add enough delay for browser scroll
restoration behavior not to work.

As a result, when going back to the page with missing data, app-router
wouldn't suspend but layout-router would suspend on the missing data
while triggering a lazy fetch. We trigger a server-patch with the
applied lazy data, but when React replays the render, it will replay the
branch without the cache node data applied. This results in the router
getting caught in a loop of suspending, applying the cache node,
replaying the branch without the cache node, and eventually crashing due
to an error thrown by React to prevent re-suspending indefinitely.

### How
This adds a property to the cache node to signal if the lazy data has
been resolved. If it has been, we won't call the server patch action
again.

Fixes #61336
Closes NEXT-2438
@xvvvyz
Copy link

xvvvyz commented Feb 27, 2024

Hype. Thank you @ztanner!

@jviide
Copy link
Contributor Author

jviide commented Feb 27, 2024

Thank you, @ztanner! I can confirm that this fixes the issue in my environment as well.

@AndKenneth
Copy link

Amazing @ztanner, I thought I was going mad with this one. Cheers for the fix!

ztanner added a commit that referenced this issue Feb 28, 2024
When the popstate action is fired (as is the case in a browser back
button), if the page you're going back to has missing cache node data,
the router will crash.

Almost all router actions will suspend at the app-router level with the
exception of `ACTION_RESTORE`. This was to address an issue where
suspending in the router would add enough delay for browser scroll
restoration behavior not to work.

As a result, when going back to the page with missing data, app-router
wouldn't suspend but layout-router would suspend on the missing data
while triggering a lazy fetch. We trigger a server-patch with the
applied lazy data, but when React replays the render, it will replay the
branch without the cache node data applied. This results in the router
getting caught in a loop of suspending, applying the cache node,
replaying the branch without the cache node, and eventually crashing due
to an error thrown by React to prevent re-suspending indefinitely.

This adds a property to the cache node to signal if the lazy data has
been resolved. If it has been, we won't call the server patch action
again.

Fixes #61336
Closes NEXT-2438
@dariusrosendahl
Copy link

dariusrosendahl commented Mar 3, 2024

Really happy I found this.

Workaround: Use a loading.js file for that route.

This works for me too. No more crashing websites but am I missing something? I'm on 14.1.1, also tried 14.1.2-canary.2 and I'm not seeing a permanent fix like you are all apparently seeing from @ztanner without a loading component when navigating back with the browser button on some pages. 🙈

I can see the network tab flooding until my global-error kicks in:

Screenshot 2024-03-03 at 13 45 56

@Wei102193
Copy link

Wei102193 commented Mar 3, 2024 via email

@dariusrosendahl
Copy link

the latest release has fixed this issue

Which is why I'm asking if I'm missing something 🙂 I'm still seeing my network going into an infinite loop on next@14.1.1, also tried next@14.1.2-canary.2

@ztanner
Copy link
Member

ztanner commented Mar 4, 2024

@dariusrosendahl if you're able to isolate the bug to a minimal reproduction and open a new issue I'd be happy to take a look. Otherwise it'll be difficult to debug without more information.

@dariusrosendahl
Copy link

@dariusrosendahl if you're able to isolate the bug to a minimal reproduction and open a new issue I'd be happy to take a look. Otherwise it'll be difficult to debug without more information.

Hi @ztanner Hopefully this is minimal enough, I'm not sure if it's the way I'm fetching data, if it's the CMS I'm using or something else: https://stackblitz.com/edit/stackblitz-starters-mx9ook?file=app%2Fcomponents%2FOverview.tsx

Maciekek pushed a commit to Maciekek/shopping-list that referenced this issue Mar 9, 2024
@ztanner
Copy link
Member

ztanner commented Mar 12, 2024

Hi @dariusrosendahl -- I took a look at your repro and I believe the issue is coming from your usage of StoryBlok. It looks it's trying to render an async component inside of a client component (specifically, your Overview.tsx component) which is not currently supported as indicated by the React error. I found someone on StackOverflow with a similar question, hopefully the answer is helpful as I'm not familiar with this library!

Copy link
Contributor

This closed issue has been automatically locked because it had no new activity for 2 weeks. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue was opened via the bug report template. linear: next Confirmed issue that is tracked by the Next.js team. locked
Projects
None yet