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

Create hash digest for errors in app in production #41559

Merged
merged 7 commits into from Oct 19, 2022
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
23 changes: 12 additions & 11 deletions packages/next/server/app-render.tsx
Expand Up @@ -42,6 +42,7 @@ import { DYNAMIC_ERROR_CODE } from '../client/components/hooks-server-context'
import { NOT_FOUND_ERROR_CODE } from '../client/components/not-found'
import { HeadManagerContext } from '../shared/lib/head-manager-context'
import { Writable } from 'stream'
import stringHash from 'next/dist/compiled/string-hash'

const INTERNAL_HEADERS_INSTANCE = Symbol('internal for headers readonly')

Expand Down Expand Up @@ -176,21 +177,21 @@ function createErrorHandler(
_source: string,
capturedErrors: Error[]
) {
return (err: any) => {
return (err: any): string => {
if (
// TODO-APP: Handle redirect throw
err.digest !== DYNAMIC_ERROR_CODE &&
err.digest !== NOT_FOUND_ERROR_CODE &&
!err.digest?.startsWith(REDIRECT_ERROR_CODE)
err.digest === DYNAMIC_ERROR_CODE ||
err.digest === NOT_FOUND_ERROR_CODE ||
err.digest?.startsWith(REDIRECT_ERROR_CODE)
) {
// Used for debugging error source
// console.error(_source, err)
console.error(err)
capturedErrors.push(err)
return err.digest || err.message
return err.digest
}

return err.digest
// Used for debugging error source
// console.error(_source, err)
console.error(err)
capturedErrors.push(err)
// TODO-APP: look at using webcrypto instead. Requires a promise to be awaited.
return stringHash(err.message + err.stack + (err.digest || '')).toString()
}
}

Expand Down
@@ -0,0 +1,9 @@
export const config = {
revalidate: 0,
}

export default function Page() {
const err = new Error('this is a test')
err.digest = 'custom'
throw err
}
13 changes: 13 additions & 0 deletions test/e2e/app-dir/app/app/error/server-component/error.js
@@ -0,0 +1,13 @@
'use client'

export default function ErrorBoundary({ error, reset }) {
return (
<>
<p id="error-boundary-message">{error.message}</p>
<p id="error-boundary-digest">{error.digest}</p>
<button id="reset" onClick={() => reset()}>
Try again
</button>
</>
)
}
7 changes: 7 additions & 0 deletions test/e2e/app-dir/app/app/error/server-component/page.js
@@ -0,0 +1,7 @@
export const config = {
revalidate: 0,
}

export default function Page() {
throw new Error('this is a test')
}
34 changes: 32 additions & 2 deletions test/e2e/app-dir/index.test.ts
Expand Up @@ -1553,8 +1553,7 @@ describe('app dir', () => {

if (isDev) {
expect(await hasRedbox(browser)).toBe(true)
console.log('getRedboxHeader', await getRedboxHeader(browser))
// expect(await getRedboxHeader(browser)).toMatch(/An error occurred: this is a test/)
expect(await getRedboxHeader(browser)).toMatch(/this is a test/)
} else {
await browser
expect(
Expand All @@ -1566,6 +1565,37 @@ describe('app dir', () => {
}
})

it('should trigger error component when an error happens during server components rendering', async () => {
const browser = await webdriver(next.url, '/error/server-component')

if (isDev) {
expect(
await browser
.waitForElementByCss('#error-boundary-message')
.elementByCss('#error-boundary-message')
.text()
).toBe('this is a test')
expect(
await browser.waitForElementByCss('#error-boundary-digest').text()
// Digest of the error message should be stable.
).not.toBe('')
// TODO-APP: ensure error overlay is shown for errors that happened before/during hydration
// expect(await hasRedbox(browser)).toBe(true)
// expect(await getRedboxHeader(browser)).toMatch(/this is a test/)
} else {
await browser
expect(
await browser.waitForElementByCss('#error-boundary-message').text()
).toBe(
'An error occurred in the Server Components render. The specific message is omitted in production builds to avoid leaking sensitive details. A digest property is included on this error instance which may provide additional details about the nature of the error.'
)
expect(
await browser.waitForElementByCss('#error-boundary-digest').text()
// Digest of the error message should be stable.
).not.toBe('')
}
})

it('should use default error boundary for prod and overlay for dev when no error component specified', async () => {
const browser = await webdriver(
next.url,
Expand Down