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

[Flight] Implement error digests for Flight runtime and expose errorInfo in getDerivedStateFromError #25302

Merged
merged 7 commits into from
Sep 23, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 48 additions & 5 deletions packages/react-client/src/ReactFlightClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ function createBlockedChunk<T>(response: Response): BlockedChunk<T> {

function createErrorChunk<T>(
response: Response,
error: Error,
error: ErrorWithDigest,
): ErroredChunk<T> {
// $FlowFixMe Flow doesn't support functions as constructors
return new Chunk(ERRORED, null, error, response);
Expand Down Expand Up @@ -628,21 +628,64 @@ export function resolveSymbol(
chunks.set(id, createInitializedChunk(response, Symbol.for(name)));
}

export function resolveError(
type ErrorWithDigest = Error & {digest?: string};
export function resolveErrorProd(
response: Response,
id: number,
digest: string,
): void {
if (__DEV__) {
// These errors should never make it into a build so we don't need to encode them in codes.json
// eslint-disable-next-line react-internal/prod-error-codes
throw new Error(
'resolveErrorProd should never be called in development mode. Use resolveErrorDev instead. This is a bug in React.',
);
}
const error = new Error(
'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.',
);
error.stack = '';
(error: any).digest = digest;
const errorWithDigest: ErrorWithDigest = (error: any);
const chunks = response._chunks;
const chunk = chunks.get(id);
if (!chunk) {
chunks.set(id, createErrorChunk(response, errorWithDigest));
} else {
triggerErrorOnChunk(chunk, errorWithDigest);
}
}

export function resolveErrorDev(
response: Response,
id: number,
digest: string,
message: string,
stack: string,
): void {
if (!__DEV__) {
// These errors should never make it into a build so we don't need to encode them in codes.json
// eslint-disable-next-line react-internal/prod-error-codes
throw new Error(
'resolveErrorDev should never be called in production mode. Use resolveErrorProd instead. This is a bug in React.',
);
}
// eslint-disable-next-line react-internal/prod-error-codes
const error = new Error(message);
const error = new Error(
message ||
'An error occurred in the Server Components render but no message was provided',
);
error.stack = stack;
(error: any).digest = digest;
const errorWithDigest: ErrorWithDigest = (error: any);
const chunks = response._chunks;
const chunk = chunks.get(id);
if (!chunk) {
chunks.set(id, createErrorChunk(response, error));
chunks.set(id, createErrorChunk(response, errorWithDigest));
} else {
triggerErrorOnChunk(chunk, error);
triggerErrorOnChunk(chunk, errorWithDigest);
}
}

Expand Down
15 changes: 13 additions & 2 deletions packages/react-client/src/ReactFlightClientStream.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ import {
resolveModel,
resolveProvider,
resolveSymbol,
resolveError,
resolveErrorProd,
resolveErrorDev,
createResponse as createResponseBase,
parseModelString,
parseModelTuple,
Expand Down Expand Up @@ -62,7 +63,17 @@ function processFullRow(response: Response, row: string): void {
}
case 'E': {
const errorInfo = JSON.parse(text);
resolveError(response, id, errorInfo.message, errorInfo.stack);
if (__DEV__) {
resolveErrorDev(
response,
id,
errorInfo.digest,
errorInfo.message,
errorInfo.stack,
);
} else {
resolveErrorProd(response, id, errorInfo.digest);
}
return;
}
default: {
Expand Down
18 changes: 15 additions & 3 deletions packages/react-client/src/__tests__/ReactFlight-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,19 @@ describe('ReactFlight', () => {
componentDidMount() {
expect(this.state.hasError).toBe(true);
expect(this.state.error).toBeTruthy();
expect(this.state.error.message).toContain(this.props.expectedMessage);
if (__DEV__) {
expect(this.state.error.message).toContain(
this.props.expectedMessage,
);
expect(this.state.error.digest).toBe('a dev digest');
} else {
expect(this.state.error.message).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(this.state.error.digest).toContain(this.props.expectedMessage);
}
}
render() {
if (this.state.hasError) {
Expand Down Expand Up @@ -371,8 +383,8 @@ describe('ReactFlight', () => {
}

const options = {
onError() {
// ignore
onError(x) {
return __DEV__ ? 'a dev digest' : `digest("${x.message}")`;
},
};
const event = ReactNoopFlightServer.render(<EventHandlerProp />, options);
Expand Down
20 changes: 17 additions & 3 deletions packages/react-server-dom-relay/src/ReactFlightDOMRelayClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ import {
resolveModel,
resolveModule,
resolveSymbol,
resolveError,
resolveErrorDev,
resolveErrorProd,
close,
getRoot,
} from 'react-client/src/ReactFlightClient';
Expand All @@ -34,7 +35,20 @@ export function resolveRow(response: Response, chunk: RowEncoding): void {
// $FlowFixMe: Flow doesn't support disjoint unions on tuples.
resolveSymbol(response, chunk[1], chunk[2]);
} else {
// $FlowFixMe: Flow doesn't support disjoint unions on tuples.
resolveError(response, chunk[1], chunk[2].message, chunk[2].stack);
if (__DEV__) {
resolveErrorDev(
response,
chunk[1],
// $FlowFixMe: Flow doesn't support disjoint unions on tuples.
chunk[2].digest,
// $FlowFixMe: Flow doesn't support disjoint unions on tuples.
chunk[2].message || '',
// $FlowFixMe: Flow doesn't support disjoint unions on tuples.
chunk[2].stack || '',
);
} else {
// $FlowFixMe: Flow doesn't support disjoint unions on tuples.
resolveErrorProd(response, chunk[1], chunk[2].digest);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,9 @@ export type RowEncoding =
'E',
number,
{
message: string,
stack: string,
digest: string,
message?: string,
stack?: string,
...
},
];
Original file line number Diff line number Diff line change
Expand Up @@ -60,16 +60,48 @@ export function resolveModuleMetaData<T>(

export type Chunk = RowEncoding;

export function processErrorChunk(
export function processErrorChunkProd(
request: Request,
id: number,
digest: string,
): Chunk {
if (__DEV__) {
// These errors should never make it into a build so we don't need to encode them in codes.json
// eslint-disable-next-line react-internal/prod-error-codes
throw new Error(
'processErrorChunkProd should never be called while in development mode. Use processErrorChunkDev instead. This is a bug in React.',
);
}

return [
'E',
id,
{
digest,
},
];
}

export function processErrorChunkDev(
request: Request,
id: number,
digest: string,
message: string,
stack: string,
): Chunk {
if (!__DEV__) {
// These errors should never make it into a build so we don't need to encode them in codes.json
// eslint-disable-next-line react-internal/prod-error-codes
throw new Error(
'processErrorChunkDev should never be called while in production mode. Use processErrorChunkProd instead. This is a bug in React.',
);
}

return [
'E',
id,
{
digest,
message,
stack,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,13 @@ describe('ReactFlightDOM', () => {

function MyErrorBoundary({children}) {
return (
<ErrorBoundary fallback={e => <p>{e.message}</p>}>
<ErrorBoundary
fallback={e => (
<p>
{__DEV__ ? e.message + ' + ' : null}
{e.digest}
</p>
)}>
{children}
</ErrorBoundary>
);
Expand Down Expand Up @@ -434,6 +440,7 @@ describe('ReactFlightDOM', () => {
{
onError(x) {
reportedErrors.push(x);
return __DEV__ ? 'a dev digest' : `digest("${x.message}")`;
},
},
);
Expand Down Expand Up @@ -477,11 +484,14 @@ describe('ReactFlightDOM', () => {
await act(async () => {
rejectGames(theError);
});
const expectedGamesValue = __DEV__
? '<p>Game over + a dev digest</p>'
: '<p>digest("Game over")</p>';
expect(container.innerHTML).toBe(
'<div>:name::avatar:</div>' +
'<p>(loading sidebar)</p>' +
'<p>(loading posts)</p>' +
'<p>Game over</p>', // TODO: should not have message in prod.
expectedGamesValue,
);

expect(reportedErrors).toEqual([theError]);
Expand All @@ -495,7 +505,7 @@ describe('ReactFlightDOM', () => {
'<div>:name::avatar:</div>' +
'<div>:photos::friends:</div>' +
'<p>(loading posts)</p>' +
'<p>Game over</p>', // TODO: should not have message in prod.
expectedGamesValue,
);

// Show everything.
Expand All @@ -506,7 +516,7 @@ describe('ReactFlightDOM', () => {
'<div>:name::avatar:</div>' +
'<div>:photos::friends:</div>' +
'<div>:posts:</div>' +
'<p>Game over</p>', // TODO: should not have message in prod.
expectedGamesValue,
);

expect(reportedErrors).toEqual([]);
Expand Down Expand Up @@ -611,6 +621,8 @@ describe('ReactFlightDOM', () => {
{
onError(x) {
reportedErrors.push(x);
const message = typeof x === 'string' ? x : x.message;
return __DEV__ ? 'a dev digest' : `digest("${message}")`;
},
},
);
Expand All @@ -626,7 +638,13 @@ describe('ReactFlightDOM', () => {

await act(async () => {
root.render(
<ErrorBoundary fallback={e => <p>{e.message}</p>}>
<ErrorBoundary
fallback={e => (
<p>
{__DEV__ ? e.message + ' + ' : null}
{e.digest}
</p>
)}>
<Suspense fallback={<p>(loading)</p>}>
<App res={response} />
</Suspense>
Expand All @@ -638,7 +656,13 @@ describe('ReactFlightDOM', () => {
await act(async () => {
abort('for reasons');
});
expect(container.innerHTML).toBe('<p>Error: for reasons</p>');
if (__DEV__) {
expect(container.innerHTML).toBe(
'<p>Error: for reasons + a dev digest</p>',
);
} else {
expect(container.innerHTML).toBe('<p>digest("for reasons")</p>');
}

expect(reportedErrors).toEqual(['for reasons']);
});
Expand Down Expand Up @@ -772,7 +796,8 @@ describe('ReactFlightDOM', () => {
webpackMap,
{
onError(x) {
reportedErrors.push(x);
reportedErrors.push(x.message);
return __DEV__ ? 'a dev digest' : `digest("${x.message}")`;
},
},
);
Expand All @@ -789,15 +814,27 @@ describe('ReactFlightDOM', () => {

await act(async () => {
root.render(
<ErrorBoundary fallback={e => <p>{e.message}</p>}>
<ErrorBoundary
fallback={e => (
<p>
{__DEV__ ? e.message + ' + ' : null}
{e.digest}
</p>
)}>
<Suspense fallback={<p>(loading)</p>}>
<App res={response} />
</Suspense>
</ErrorBoundary>,
);
});
expect(container.innerHTML).toBe('<p>bug in the bundler</p>');
if (__DEV__) {
expect(container.innerHTML).toBe(
'<p>bug in the bundler + a dev digest</p>',
);
} else {
expect(container.innerHTML).toBe('<p>digest("bug in the bundler")</p>');
}

expect(reportedErrors).toEqual([]);
expect(reportedErrors).toEqual(['bug in the bundler']);
});
});
Loading