Skip to content

Commit 06d45e7

Browse files
authored
fix: prevent fetch abort errors propagating to user error boundaries (#86277)
When navigating away from a page due to an MPA navigation or browser refresh, we were aborting any pending RSC requests. However, in the case where the abort was due to an MPA navigation, the browser would restore the execution context via the built-in bfcache, and the aborted fetch would cause React to get in an errored state, leading to "BodyStreamBuffer was aborted". Rather than aborting the pending requests, we instead mark that we would have aborted due to the page unloading. That still solves the original reason the flag was added (suppressing the fetch errors that get logged) Fixes NAR-519
1 parent dbfb6c4 commit 06d45e7

File tree

9 files changed

+148
-12
lines changed

9 files changed

+148
-12
lines changed

packages/next/src/client/components/router-reducer/fetch-server-response.ts

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -102,21 +102,20 @@ function doMpaNavigation(url: string): FetchServerResponseResult {
102102
return urlToUrlWithoutFlightMarker(new URL(url, location.origin)).toString()
103103
}
104104

105-
let abortController = new AbortController()
105+
let isPageUnloading = false
106106

107107
if (typeof window !== 'undefined') {
108-
// Abort any in-flight requests when the page is unloaded, e.g. due to
109-
// reloading the page or performing hard navigations. This allows us to ignore
110-
// what would otherwise be a thrown TypeError when the browser cancels the
111-
// requests.
108+
// Track when the page is unloading, e.g. due to reloading the page or
109+
// performing hard navigations. This allows us to suppress error logging when
110+
// the browser cancels in-flight requests during page unload.
112111
window.addEventListener('pagehide', () => {
113-
abortController.abort()
112+
isPageUnloading = true
114113
})
115114

116-
// Use a fresh AbortController instance on pageshow, e.g. when navigating back
117-
// and the JavaScript execution context is restored by the browser.
115+
// Reset the flag on pageshow, e.g. when navigating back and the JavaScript
116+
// execution context is restored by the browser.
118117
window.addEventListener('pageshow', () => {
119-
abortController = new AbortController()
118+
isPageUnloading = false
120119
})
121120
}
122121

@@ -197,8 +196,7 @@ export async function fetchServerResponse(
197196
url,
198197
headers,
199198
fetchPriority,
200-
shouldImmediatelyDecode,
201-
abortController.signal
199+
shouldImmediatelyDecode
202200
)
203201

204202
const responseUrl = urlToUrlWithoutFlightMarker(new URL(res.url))
@@ -287,7 +285,7 @@ export async function fetchServerResponse(
287285
debugInfo: flightResponsePromise._debugInfo ?? null,
288286
}
289287
} catch (err) {
290-
if (!abortController.signal.aborted) {
288+
if (!isPageUnloading) {
291289
console.error(
292290
`Failed to fetch RSC payload for ${originalUrl}. Falling back to browser navigation.`,
293291
err
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import React from 'react'
2+
3+
export default function RootLayout({
4+
children,
5+
}: {
6+
children: React.ReactNode
7+
}) {
8+
return (
9+
<html>
10+
<body>{children}</body>
11+
</html>
12+
)
13+
}
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
'use client'
2+
3+
import { useRouter } from 'next/navigation'
4+
5+
export function TriggerMpaNavigation() {
6+
const router = useRouter()
7+
return (
8+
<button
9+
id="trigger-navigation"
10+
onClick={async () => {
11+
router.push('/slow-page')
12+
await new Promise((resolve) => setTimeout(resolve, 500))
13+
router.push('/other-root')
14+
}}
15+
>
16+
Trigger Navigation
17+
</button>
18+
)
19+
}
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
import { TriggerMpaNavigation } from './mpa'
2+
3+
export default function StartPage() {
4+
return (
5+
<div style={{ padding: '20px' }}>
6+
<h1 id="start-page">Start Page</h1>
7+
<TriggerMpaNavigation />
8+
</div>
9+
)
10+
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
import { connection } from 'next/server'
2+
import { Suspense } from 'react'
3+
4+
async function SlowData() {
5+
// Artificial delay to simulate slow RSC fetch
6+
await connection()
7+
await new Promise((resolve) => setTimeout(resolve, 5_000))
8+
return (
9+
<div
10+
id="slow-data-loaded"
11+
style={{ padding: '10px', background: '#e0e0ff' }}
12+
>
13+
Slow data loaded successfully!
14+
</div>
15+
)
16+
}
17+
18+
export default function SlowPage() {
19+
return (
20+
<div id="slow-page">
21+
<Suspense
22+
fallback={<div id="loading-slow-data">Loading slow data...</div>}
23+
>
24+
<SlowData />
25+
</Suspense>
26+
</div>
27+
)
28+
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import React from 'react'
2+
3+
export default function RootLayout({
4+
children,
5+
}: {
6+
children: React.ReactNode
7+
}) {
8+
return (
9+
<html>
10+
<body>{children}</body>
11+
</html>
12+
)
13+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
export default function StartPage() {
2+
return <div id="root-2">Root 2</div>
3+
}
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
'use client' // Error boundaries must be Client Components
2+
3+
export default function GlobalError({
4+
error,
5+
reset,
6+
}: {
7+
error: Error & { digest?: string }
8+
reset: () => void
9+
}) {
10+
return (
11+
// global-error must include html and body tags
12+
<html>
13+
<body>
14+
<h2 id="global-error">Something went wrong!</h2>
15+
<button onClick={() => reset()}>Try again</button>
16+
</body>
17+
</html>
18+
)
19+
}
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
import { nextTestSetup } from 'e2e-utils'
2+
3+
const describeHeaded = process.env.HEADLESS ? describe.skip : describe
4+
5+
describeHeaded('fetch-abort-on-refresh', () => {
6+
const { next } = nextTestSetup({
7+
files: __dirname,
8+
})
9+
10+
it('should not show abort error in global error boundary when restoring from bfcache', async () => {
11+
// This test ensures that when restoring a page from the browser bfcache that was pending RSC data,
12+
// that the abort does not propagate to a user's error boundary.
13+
const browser = await next.browser('/', { headless: false })
14+
15+
await browser.elementById('trigger-navigation').click()
16+
17+
await browser.waitForElementByCss('#root-2')
18+
19+
// Go back to trigger bfcache restoration
20+
// we overwrite the typical waitUntil: 'load' option here as the event is never being triggered if we hit the bfcache
21+
await browser.back({ waitUntil: 'commit' })
22+
23+
// Check that we're back on the slow page (the page that was first redirected to, before the MPA, not the error boundary)
24+
// We use element checks instead of eval() because eval() triggers waitForLoadState which times out with bfcache
25+
const hasSlowPage = await browser.hasElementByCss('#slow-page')
26+
const hasGlobalError = await browser.hasElementByCss(
27+
'h2:has-text("Something went wrong!")'
28+
)
29+
30+
expect(hasSlowPage).toBe(true)
31+
expect(hasGlobalError).toBe(false)
32+
})
33+
})

0 commit comments

Comments
 (0)