Skip to content

Commit

Permalink
Fix handling with custom _error and pages/500 (#40110)
Browse files Browse the repository at this point in the history
This ensures we are properly calling `getInitialProps` in `_error` before serving `pages/500` as this has been the expected behavior since we introduced the process exiting behavior when deployed. This also ensures we don't attempt serving the `pages/500` before logging the error and exiting as this file isn't always expected to be present when statically optimized. 

Test deployments from provided reproduction can be seen here: 
- https://next-500-issue-ijdlh0e9y-ijjk-testing.vercel.app/
- https://next-500-issue-acn2vi68j-ijjk-testing.vercel.app/

## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have helpful link attached, see `contributing.md`

Fixes: #40065
Fixes: #39952
  • Loading branch information
ijjk committed Aug 30, 2022
1 parent dddc60d commit 68fb39a
Show file tree
Hide file tree
Showing 3 changed files with 96 additions and 5 deletions.
23 changes: 18 additions & 5 deletions packages/next/server/base-server.ts
Expand Up @@ -1609,11 +1609,16 @@ export default abstract class Server<ServerOptions extends Options = Options> {
}

res.statusCode = 500

// if pages/500 is present we still need to trigger
// /_error `getInitialProps` to allow reporting error
if (await this.hasPage('/500')) {
ctx.query.__nextCustomErrorRender = '1'
await this.renderErrorToResponse(ctx, err)
delete ctx.query.__nextCustomErrorRender
}

const isWrappedError = err instanceof WrappedBuildError
const response = await this.renderErrorToResponse(
ctx,
isWrappedError ? (err as WrappedBuildError).innerError : err
)

if (!isWrappedError) {
if (
Expand All @@ -1625,6 +1630,10 @@ export default abstract class Server<ServerOptions extends Options = Options> {
}
this.logError(getProperError(err))
}
const response = await this.renderErrorToResponse(
ctx,
isWrappedError ? (err as WrappedBuildError).innerError : err
)
return response
}

Expand Down Expand Up @@ -1713,7 +1722,11 @@ export default abstract class Server<ServerOptions extends Options = Options> {
}
let statusPage = `/${res.statusCode}`

if (!result && STATIC_STATUS_PAGES.includes(statusPage)) {
if (
!ctx.query.__nextCustomErrorRender &&
!result &&
STATIC_STATUS_PAGES.includes(statusPage)
) {
// skip ensuring /500 in dev mode as it isn't used and the
// dev overlay is used instead
if (statusPage !== '/500' || !this.renderOpts.dev) {
Expand Down
1 change: 1 addition & 0 deletions packages/next/server/request-meta.ts
Expand Up @@ -63,6 +63,7 @@ type NextQueryMetadata = {
__nextSsgPath?: string
_nextBubbleNoFallback?: '1'
__nextDataReq?: '1'
__nextCustomErrorRender?: '1'
}

export type NextParsedUrlQuery = ParsedUrlQuery &
Expand Down
77 changes: 77 additions & 0 deletions test/production/custom-error-500/index.test.ts
@@ -0,0 +1,77 @@
import { createNext } from 'e2e-utils'
import { NextInstance } from 'test/lib/next-modes/base'
import { check, renderViaHTTP } from 'next-test-utils'

describe('custom-error-500', () => {
let next: NextInstance

beforeAll(async () => {
next = await createNext({
files: {
'pages/index.js': `
export function getServerSideProps() {
throw new Error('custom error')
}
export default function Page() {
return <p>index page</p>
}
`,
'pages/500.js': `
export default function Custom500() {
return (
<>
<p>pages/500</p>
</>
)
}
`,
'pages/_error.js': `
function Error({ hasError }) {
return (
<>
<p>/_error</p>
</>
)
}
Error.getInitialProps = ({ err }) => {
console.log(\`called Error.getInitialProps \${!!err}\`)
return {
hasError: !!err
}
}
export default Error
`,
},
dependencies: {},
})
})
afterAll(() => next.destroy())

it('should correctly use pages/500 and call Error.getInitialProps', async () => {
const html = await renderViaHTTP(next.url, '/')
expect(html).toContain('pages/500')

await check(() => next.cliOutput, /called Error\.getInitialProps true/)
})

it('should work correctly with pages/404 present', async () => {
await next.stop()
await next.patchFile(
'pages/404.js',
`
export default function Page() {
return <p>custom 404 page</p>
}
`
)
await next.start()

const html = await renderViaHTTP(next.url, '/')
expect(html).toContain('pages/500')

await check(() => next.cliOutput, /called Error\.getInitialProps true/)
})
})

0 comments on commit 68fb39a

Please sign in to comment.