From 68fb39a034d1b5d7eb8f2af23ca563956f1fa3be Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Tue, 30 Aug 2022 16:14:12 -0700 Subject: [PATCH] Fix handling with custom _error and pages/500 (#40110) 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: https://github.com/vercel/next.js/issues/40065 Fixes: https://github.com/vercel/next.js/issues/39952 --- packages/next/server/base-server.ts | 23 ++++-- packages/next/server/request-meta.ts | 1 + .../production/custom-error-500/index.test.ts | 77 +++++++++++++++++++ 3 files changed, 96 insertions(+), 5 deletions(-) create mode 100644 test/production/custom-error-500/index.test.ts diff --git a/packages/next/server/base-server.ts b/packages/next/server/base-server.ts index a6da2bd95780..236f1fad44ff 100644 --- a/packages/next/server/base-server.ts +++ b/packages/next/server/base-server.ts @@ -1609,11 +1609,16 @@ export default abstract class Server { } 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 ( @@ -1625,6 +1630,10 @@ export default abstract class Server { } this.logError(getProperError(err)) } + const response = await this.renderErrorToResponse( + ctx, + isWrappedError ? (err as WrappedBuildError).innerError : err + ) return response } @@ -1713,7 +1722,11 @@ export default abstract class Server { } 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) { diff --git a/packages/next/server/request-meta.ts b/packages/next/server/request-meta.ts index 68ddb22a7280..39985a491848 100644 --- a/packages/next/server/request-meta.ts +++ b/packages/next/server/request-meta.ts @@ -63,6 +63,7 @@ type NextQueryMetadata = { __nextSsgPath?: string _nextBubbleNoFallback?: '1' __nextDataReq?: '1' + __nextCustomErrorRender?: '1' } export type NextParsedUrlQuery = ParsedUrlQuery & diff --git a/test/production/custom-error-500/index.test.ts b/test/production/custom-error-500/index.test.ts new file mode 100644 index 000000000000..2dba5785f3cf --- /dev/null +++ b/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

index page

+ } + `, + 'pages/500.js': ` + export default function Custom500() { + return ( + <> +

pages/500

+ + ) + } + `, + 'pages/_error.js': ` + function Error({ hasError }) { + return ( + <> +

/_error

+ + ) + } + + 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

custom 404 page

+ } + ` + ) + await next.start() + + const html = await renderViaHTTP(next.url, '/') + expect(html).toContain('pages/500') + + await check(() => next.cliOutput, /called Error\.getInitialProps true/) + }) +})