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

Make sure to handle failing to load _error #10617

Merged
merged 1 commit into from Feb 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
111 changes: 59 additions & 52 deletions packages/next/next-server/lib/router/router.ts
Expand Up @@ -500,6 +500,64 @@ export default class Router implements BaseRouter {
return Promise.resolve(cachedRouteInfo)
}

const handleError = (
err: Error & { code: any; cancelled: boolean },
loadErrorFail?: boolean
) => {
return new Promise(resolve => {
if (err.code === 'PAGE_LOAD_ERROR' || loadErrorFail) {
// If we can't load the page it could be one of following reasons
// 1. Page doesn't exists
// 2. Page does exist in a different zone
// 3. Internal error while loading the page

// So, doing a hard reload is the proper way to deal with this.
window.location.href = as

// Changing the URL doesn't block executing the current code path.
// So, we need to mark it as a cancelled error and stop the routing logic.
err.cancelled = true
// @ts-ignore TODO: fix the control flow here
return resolve({ error: err })
}

if (err.cancelled) {
// @ts-ignore TODO: fix the control flow here
return resolve({ error: err })
}

resolve(
this.fetchComponent('/_error')
.then(Component => {
const routeInfo: RouteInfo = { Component, err }
return new Promise(resolve => {
this.getInitialProps(Component, {
err,
pathname,
query,
} as any).then(
props => {
routeInfo.props = props
routeInfo.error = err
resolve(routeInfo)
},
gipErr => {
console.error(
'Error in error page `getInitialProps`: ',
gipErr
)
routeInfo.error = err
routeInfo.props = {}
resolve(routeInfo)
}
)
}) as Promise<RouteInfo>
})
.catch(err => handleError(err, true))
)
}) as Promise<RouteInfo>
}

return (new Promise((resolve, reject) => {
if (cachedRouteInfo) {
return resolve(cachedRouteInfo)
Expand Down Expand Up @@ -542,58 +600,7 @@ export default class Router implements BaseRouter {
return routeInfo
})
})
.catch(err => {
return new Promise(resolve => {
if (err.code === 'PAGE_LOAD_ERROR') {
// If we can't load the page it could be one of following reasons
// 1. Page doesn't exists
// 2. Page does exist in a different zone
// 3. Internal error while loading the page

// So, doing a hard reload is the proper way to deal with this.
window.location.href = as

// Changing the URL doesn't block executing the current code path.
// So, we need to mark it as a cancelled error and stop the routing logic.
err.cancelled = true
// @ts-ignore TODO: fix the control flow here
return resolve({ error: err })
}

if (err.cancelled) {
// @ts-ignore TODO: fix the control flow here
return resolve({ error: err })
}

resolve(
this.fetchComponent('/_error').then(Component => {
const routeInfo: RouteInfo = { Component, err }
return new Promise(resolve => {
this.getInitialProps(Component, {
err,
pathname,
query,
} as any).then(
props => {
routeInfo.props = props
routeInfo.error = err
resolve(routeInfo)
},
gipErr => {
console.error(
'Error in error page `getInitialProps`: ',
gipErr
)
routeInfo.error = err
routeInfo.props = {}
resolve(routeInfo)
}
)
}) as Promise<RouteInfo>
})
)
}) as Promise<RouteInfo>
})
.catch(handleError)
}

set(
Expand Down
7 changes: 7 additions & 0 deletions test/integration/error-load-fail/pages/broken.js
@@ -0,0 +1,7 @@
const Page = () => 'oops'

Page.getInitialProps = () => {
throw new Error('oops')
}

export default Page
9 changes: 9 additions & 0 deletions test/integration/error-load-fail/pages/index.js
@@ -0,0 +1,9 @@
import Link from 'next/link'

export default () => (
<>
<Link href="/broken">
<a id="to-broken">to oops</a>
</Link>
</>
)
36 changes: 36 additions & 0 deletions test/integration/error-load-fail/test/index.test.js
@@ -0,0 +1,36 @@
/* eslint-env jest */
/* global jasmine */
import path from 'path'
import webdriver from 'next-webdriver'
import {
nextBuild,
nextStart,
findPort,
killApp,
waitFor,
} from 'next-test-utils'

jasmine.DEFAULT_TIMEOUT_INTERVAL = 1000 * 60 * 1
const appDir = path.join(__dirname, '..')

describe('Failing to load _error', () => {
it('handles failing to load _error correctly', async () => {
await nextBuild(appDir)
const appPort = await findPort()
const app = await nextStart(appDir, appPort)

const browser = await webdriver(appPort, '/')
await browser.eval(`window.beforeNavigate = true`)

await browser.elementByCss('#to-broken').moveTo()
await browser.waitForElementByCss('script[src*="broken.js"')

// stop app so that _error can't be loaded
await killApp(app)

await browser.elementByCss('#to-broken').click()
await waitFor(2000)

expect(await browser.eval('window.beforeNavigate')).toBeFalsy()
})
})