Skip to content

Commit

Permalink
pages bugfix: prevent cancellation errors from being logged (#69766)
Browse files Browse the repository at this point in the history
When another data request is received by the pages router, a
cancellation error is thrown and is meant to be caught by `change` to
signal that the navigation request was aborted. However we were logging
the error right before we handled the logic to properly bubble the error
up. This meant that it was logging an exception to the console that
wasn't actionable.

Fixes #40554
Closes NEXT-3731
  • Loading branch information
ztanner authored Sep 6, 2024
1 parent e329415 commit a5f30e6
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 2 deletions.
4 changes: 2 additions & 2 deletions packages/next/src/shared/lib/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1893,8 +1893,6 @@ export default class Router implements BaseRouter {
routeProps: RouteProperties,
loadErrorFail?: boolean
): Promise<CompletePrivateRouteInfo> {
console.error(err)

if (err.cancelled) {
// bubble up cancellation errors
throw err
Expand All @@ -1919,6 +1917,8 @@ export default class Router implements BaseRouter {
throw buildCancellationError()
}

console.error(err)

try {
let props: Record<string, any> | undefined
const { page: Component, styleSheets } =
Expand Down
2 changes: 2 additions & 0 deletions test/e2e/getserversideprops/app/pages/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ const Page = ({ world, time, url }) => {
<Link href="/something?another=thing" id="something-query">
to something?another=thing
</Link>
<br />
<Link href="/redirect-page">to redirect-page</Link>
</>
)
}
Expand Down
18 changes: 18 additions & 0 deletions test/e2e/getserversideprops/app/pages/redirect-page.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import Router from 'next/router'

const RedirectPage = () => {
return <h1>Redirect Page</h1>
}

RedirectPage.getInitialProps = (context) => {
const { req, res } = context
if (req) {
res.writeHead(302, { Location: '/normal' })
res.end()
} else {
Router.push('/normal')
}
return {}
}

export default RedirectPage
Binary file not shown.
17 changes: 17 additions & 0 deletions test/e2e/getserversideprops/test/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { createNext, FileRef } from 'e2e-utils'
import escapeRegex from 'escape-string-regexp'
import {
check,
retry,
fetchViaHTTP,
getBrowserBodyText,
getRedboxHeader,
Expand Down Expand Up @@ -669,6 +670,21 @@ const runTests = (isDev = false, isDeploy = false) => {
expect(curRandom).toBe(initialRandom + '')
})

it('should not trigger an error when a data request is cancelled due to another navigation', async () => {
const browser = await webdriver(next.url, ' /')

await browser.elementByCss("[href='/redirect-page']").click()

// redirect-page will redirect to /normal
await retry(async () => {
expect(await getBrowserBodyText(browser)).toInclude('a normal page')
})

// there should not be any console errors
const logs = await browser.log()
expect(logs.filter((log) => log.source === 'error').length).toBe(0)
})

it('should dedupe server data requests', async () => {
const browser = await webdriver(next.url, '/')
await waitFor(2000)
Expand Down Expand Up @@ -851,6 +867,7 @@ describe('getServerSideProps', () => {
next = await createNext({
files: {
pages: new FileRef(join(appDir, 'pages')),
public: new FileRef(join(appDir, 'public')),
'world.txt': new FileRef(join(appDir, 'world.txt')),
'next.config.js': new FileRef(join(appDir, 'next.config.js')),
},
Expand Down

0 comments on commit a5f30e6

Please sign in to comment.