terribleplan/next.js
1
0
Fork 0
mirror of https://github.com/terribleplan/next.js.git synced 2024-01-19 02:48:18 +00:00
Code Issues Releases Wiki Activity

Implement proper error handling (#3749)

Browse source 
* Render error on the client without fetching additional scripts.

* Fix test cases.

* Remove unused '_document' page in ensurePage logic

* Remove console.error when page is not found
This commit is contained in:
Arunoda Susiripala 2018-02-13 20:43:22 +05:30 committed by Tim Neutkens
parent 68f7e20477
commit fc3b3a4101
6 changed files with 57 additions and 31 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
client/index.js
View file

@ -21,6 +21,7 @@ const {
__NEXT_DATA__: { __NEXT_DATA__: {
props, props,
err, err,
page,
pathname, pathname,
query, query,
buildId, buildId,
@ -76,7 +77,7 @@ export default async ({ ErrorDebugComponent: passedDebugComponent, stripAnsi: pa
ErrorComponent = await pageLoader.loadPage('/_error') ErrorComponent = await pageLoader.loadPage('/_error')
try { try {
Component = await pageLoader.loadPage(pathname) Component = await pageLoader.loadPage(page)
} catch (err) { } catch (err) {
console.error(stripAnsi(`${err.message}\n${err.stack}`)) console.error(stripAnsi(`${err.message}\n${err.stack}`))
Component = ErrorComponent Component = ErrorComponent

6
server/document.js
View file

@ -89,7 +89,7 @@ export class Head extends Component {
return <head {...this.props}> return <head {...this.props}>
{(head || []).map((h, i) => React.cloneElement(h, { key: h.key || i }))} {(head || []).map((h, i) => React.cloneElement(h, { key: h.key || i }))}
{page !== '_error' && <link rel='preload' href={`${assetPrefix}/_next/${buildId}/page${pagePathname}`} as='script' />} {page !== '/_error' && <link rel='preload' href={`${assetPrefix}/_next/${buildId}/page${pagePathname}`} as='script' />}
<link rel='preload' href={`${assetPrefix}/_next/${buildId}/page/_error.js`} as='script' /> <link rel='preload' href={`${assetPrefix}/_next/${buildId}/page/_error.js`} as='script' />
{this.getPreloadDynamicChunks()} {this.getPreloadDynamicChunks()}
{this.getPreloadMainLinks()} {this.getPreloadMainLinks()}
@ -198,13 +198,13 @@ export class NextScript extends Component {
__NEXT_REGISTER_PAGE(${htmlescape(pathname)}, function() { __NEXT_REGISTER_PAGE(${htmlescape(pathname)}, function() {
var error = new Error('Page does not exist: ${htmlescape(pathname)}') var error = new Error('Page does not exist: ${htmlescape(pathname)}')
error.statusCode = 404 error.statusCode = 404
return { error: error } return { error: error }
}) })
`} `}
` `
}} />} }} />}
{page !== '_error' && <script async id={`__NEXT_PAGE__${pathname}`} type='text/javascript' src={`${assetPrefix}/_next/${buildId}/page${pagePathname}`} />} {page !== '/_error' && <script async id={`__NEXT_PAGE__${pathname}`} type='text/javascript' src={`${assetPrefix}/_next/${buildId}/page${pagePathname}`} />}
<script async id={`__NEXT_PAGE__/_error`} type='text/javascript' src={`${assetPrefix}/_next/${buildId}/page/_error.js`} /> <script async id={`__NEXT_PAGE__/_error`} type='text/javascript' src={`${assetPrefix}/_next/${buildId}/page/_error.js`} />
{staticMarkup ? null : this.getDynamicChunks()} {staticMarkup ? null : this.getDynamicChunks()}
{staticMarkup ? null : this.getScripts()} {staticMarkup ? null : this.getScripts()}

4
server/render.js
View file

@ -30,7 +30,7 @@ export async function renderError (err, req, res, pathname, query, opts) {
} }
export function renderErrorToHTML (err, req, res, pathname, query, opts = {}) { export function renderErrorToHTML (err, req, res, pathname, query, opts = {}) {
return doRender(req, res, pathname, query, { ...opts, err, page: '_error' }) return doRender(req, res, pathname, query, { ...opts, err, page: '/_error' })
} }
async function doRender (req, res, pathname, query, { async function doRender (req, res, pathname, query, {
@ -224,7 +224,7 @@ export function serveStatic (req, res, path) {
} }
async function ensurePage (page, { dir, hotReloader }) { async function ensurePage (page, { dir, hotReloader }) {
if (page === '_error' || page === '_document') return if (page === '/_error') return
await hotReloader.ensurePage(page) await hotReloader.ensurePage(page)
} }

1
server/require.js
View file

@ -58,7 +58,6 @@ export default function requirePage (page, {dir, dist}) {
try { try {
return require(pagePath) return require(pagePath)
} catch (err) { } catch (err) {
console.error(err)
throw pageNotFoundError(page) throw pageNotFoundError(page)
} }
} }

42
test/integration/basic/test/client-navigation.js
View file

@ -370,16 +370,8 @@ export default (context, render) => {
browser.close() browser.close()
}) })
it('should work with dir/index page ', async () => {
const browser = await webdriver(context.appPort, '/nested-cdm/index')
const text = await browser.elementByCss('p').text()
expect(text).toBe('ComponentDidMount executed on client.')
browser.close()
})
it('should work with dir/ page ', async () => { it('should work with dir/ page ', async () => {
const browser = await webdriver(context.appPort, '/nested-cdm/') const browser = await webdriver(context.appPort, '/nested-cdm')
const text = await browser.elementByCss('p').text() const text = await browser.elementByCss('p').text()
expect(text).toBe('ComponentDidMount executed on client.') expect(text).toBe('ComponentDidMount executed on client.')
@ -426,7 +418,7 @@ export default (context, render) => {
describe('with asPath', () => { describe('with asPath', () => {
describe('inside getInitialProps', () => { describe('inside getInitialProps', () => {
it('should show the correct asPath with a Link with as prop', async () => { it('should show the correct asPath with a Link with as prop', async () => {
const browser = await webdriver(context.appPort, '/nav/') const browser = await webdriver(context.appPort, '/nav')
const asPath = await browser const asPath = await browser
.elementByCss('#as-path-link').click() .elementByCss('#as-path-link').click()
.waitForElementByCss('.as-path-content') .waitForElementByCss('.as-path-content')
@ -437,7 +429,7 @@ export default (context, render) => {
}) })
it('should show the correct asPath with a Link without the as prop', async () => { it('should show the correct asPath with a Link without the as prop', async () => {
const browser = await webdriver(context.appPort, '/nav/') const browser = await webdriver(context.appPort, '/nav')
const asPath = await browser const asPath = await browser
.elementByCss('#as-path-link-no-as').click() .elementByCss('#as-path-link-no-as').click()
.waitForElementByCss('.as-path-content') .waitForElementByCss('.as-path-content')
@ -450,7 +442,7 @@ export default (context, render) => {
describe('with next/router', () => { describe('with next/router', () => {
it('should show the correct asPath', async () => { it('should show the correct asPath', async () => {
const browser = await webdriver(context.appPort, '/nav/') const browser = await webdriver(context.appPort, '/nav')
const asPath = await browser const asPath = await browser
.elementByCss('#as-path-using-router-link').click() .elementByCss('#as-path-using-router-link').click()
.waitForElementByCss('.as-path-content') .waitForElementByCss('.as-path-content')
@ -461,5 +453,31 @@ export default (context, render) => {
}) })
}) })
}) })
describe('with 404 pages', () => {
it('should 404 on not existent page', async () => {
const browser = await webdriver(context.appPort, '/non-existent')
expect(await browser.elementByCss('h1').text()).toBe('404')
expect(await browser.elementByCss('h2').text()).toBe('This page could not be found.')
browser.close()
})
it('should 404 for <page>/', async () => {
const browser = await webdriver(context.appPort, '/nav/about/')
expect(await browser.elementByCss('h1').text()).toBe('404')
expect(await browser.elementByCss('h2').text()).toBe('This page could not be found.')
browser.close()
})
it('should should not contain a page script in a 404 page', async () => {
const browser = await webdriver(context.appPort, '/non-existent')
const scripts = await browser.elementsByCss('script[src]')
for (const script of scripts) {
const src = await script.getAttribute('src')
expect(src.includes('/non-existent')).toBeFalsy()
}
browser.close()
})
})
}) })
} }

32
test/integration/basic/test/rendering.js
View file

@ -106,19 +106,27 @@ export default function ({ app }, suiteName, render, fetch) {
expect($('.as-path-content').text()).toBe('/nav/as-path?aa=10') expect($('.as-path-content').text()).toBe('/nav/as-path?aa=10')
}) })
test('error 404', async () => { describe('404', () => {
const $ = await get$('/non-existent') it('should 404 on not existent page', async () => {
expect($('h1').text()).toBe('404') const $ = await get$('/non-existent')
expect($('h2').text()).toBe('This page could not be found.') expect($('h1').text()).toBe('404')
}) expect($('h2').text()).toBe('This page could not be found.')
})
test('error 404 should not have page script', async () => { it('should 404 for <page>/', async () => {
const $ = await get$('/non-existent') const $ = await get$('/nav/about/')
$('script[src]').each((index, element) => { expect($('h1').text()).toBe('404')
const src = $(element).attr('src') expect($('h2').text()).toBe('This page could not be found.')
if (src.includes('/non-existent')) { })
throw new Error('Page includes page script')
} it('should should not contain a page script in a 404 page', async () => {
const $ = await get$('/non-existent')
$('script[src]').each((index, element) => {
const src = $(element).attr('src')
if (src.includes('/non-existent')) {
throw new Error('Page includes page script')
}
})
}) })
}) })