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.

Sign up for GitHub

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

Include regular stack trace in serialized errors from Fizz #28684

Merged
merged 3 commits into from
Mar 30, 2024
Merged

Include regular stack trace in serialized errors from Fizz #28684

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
567b90d
Include regular stack trace in serialized errors from Fizz
sebmarkbage Mar 29, 2024
73a50cb
Bump iteration loop count
sebmarkbage Mar 29, 2024
e4d2591
Add a prefix to errors serialized from server rendering
sebmarkbage Mar 29, 2024
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
11 changes: 9 additions & 2 deletions packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1226,22 +1226,29 @@ export function isSuspenseInstanceFallback(

export function getSuspenseInstanceFallbackErrorDetails(
instance: SuspenseInstance,
): {digest: ?string, message?: string, stack?: string} {
): {
digest: ?string,
message?: string,
stack?: string,
componentStack?: string,
} {
const dataset =
instance.nextSibling && ((instance.nextSibling: any): HTMLElement).dataset;
let digest, message, stack;
let digest, message, stack, componentStack;
if (dataset) {
digest = dataset.dgst;
if (__DEV__) {
message = dataset.msg;
stack = dataset.stck;
componentStack = dataset.cstck;
}
}
if (__DEV__) {
return {
message,
digest,
stack,
componentStack,
};
} else {
// Object gets DCE'd if constructed in tail position and matches callsite destructuring
Expand Down
1 change: 1 addition & 0 deletions packages/react-dom-bindings/src/server/ReactDOMServerExternalRuntime.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ function handleNode(node_: Node) {
dataset['dgst'],
dataset['msg'],
dataset['stck'],
dataset['cstck'],
);
node.remove();
} else if (dataset['rri'] != null) {
Expand Down
50 changes: 40 additions & 10 deletions packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3809,6 +3809,8 @@ const clientRenderedSuspenseBoundaryError1B =
stringToPrecomputedChunk(' data-msg="');
const clientRenderedSuspenseBoundaryError1C =
stringToPrecomputedChunk(' data-stck="');
const clientRenderedSuspenseBoundaryError1D =
stringToPrecomputedChunk(' data-cstck="');
const clientRenderedSuspenseBoundaryError2 =
stringToPrecomputedChunk('></template>');

Expand Down Expand Up @@ -3851,7 +3853,8 @@ export function writeStartClientRenderedSuspenseBoundary(
destination: Destination,
renderState: RenderState,
errorDigest: ?string,
errorMesssage: ?string,
errorMessage: ?string,
errorStack: ?string,
errorComponentStack: ?string,
): boolean {
let result;
Expand All @@ -3869,19 +3872,27 @@ export function writeStartClientRenderedSuspenseBoundary(
);
}
if (__DEV__) {
if (errorMesssage) {
if (errorMessage) {
writeChunk(destination, clientRenderedSuspenseBoundaryError1B);
writeChunk(
destination,
stringToChunk(escapeTextForBrowser(errorMesssage)),
stringToChunk(escapeTextForBrowser(errorMessage)),
);
writeChunk(
destination,
clientRenderedSuspenseBoundaryErrorAttrInterstitial,
);
}
if (errorComponentStack) {
if (errorStack) {
writeChunk(destination, clientRenderedSuspenseBoundaryError1C);
writeChunk(destination, stringToChunk(escapeTextForBrowser(errorStack)));
writeChunk(
destination,
clientRenderedSuspenseBoundaryErrorAttrInterstitial,
);
}
if (errorComponentStack) {
writeChunk(destination, clientRenderedSuspenseBoundaryError1D);
writeChunk(
destination,
stringToChunk(escapeTextForBrowser(errorComponentStack)),
Expand Down Expand Up @@ -4244,6 +4255,7 @@ const clientRenderData1 = stringToPrecomputedChunk(
const clientRenderData2 = stringToPrecomputedChunk('" data-dgst="');
const clientRenderData3 = stringToPrecomputedChunk('" data-msg="');
const clientRenderData4 = stringToPrecomputedChunk('" data-stck="');
const clientRenderData5 = stringToPrecomputedChunk('" data-cstck="');
const clientRenderDataEnd = dataElementQuotedEnd;

export function writeClientRenderBoundaryInstruction(
Expand All @@ -4252,8 +4264,9 @@ export function writeClientRenderBoundaryInstruction(
renderState: RenderState,
id: number,
errorDigest: ?string,
errorMessage?: string,
errorComponentStack?: string,
errorMessage: ?string,
errorStack: ?string,
errorComponentStack: ?string,
): boolean {
const scriptFormat =
!enableFizzExternalRuntime ||
Expand Down Expand Up @@ -4284,7 +4297,7 @@ export function writeClientRenderBoundaryInstruction(
writeChunk(destination, clientRenderScript1A);
}

if (errorDigest || errorMessage || errorComponentStack) {
if (errorDigest || errorMessage || errorStack || errorComponentStack) {
if (scriptFormat) {
// ,"JSONString"
writeChunk(destination, clientRenderErrorScriptArgInterstitial);
Expand All @@ -4301,7 +4314,7 @@ export function writeClientRenderBoundaryInstruction(
);
}
}
if (errorMessage || errorComponentStack) {
if (errorMessage || errorStack || errorComponentStack) {
if (scriptFormat) {
// ,"JSONString"
writeChunk(destination, clientRenderErrorScriptArgInterstitial);
Expand All @@ -4318,6 +4331,23 @@ export function writeClientRenderBoundaryInstruction(
);
}
}
if (errorStack || errorComponentStack) {
// ,"JSONString"
if (scriptFormat) {
writeChunk(destination, clientRenderErrorScriptArgInterstitial);
writeChunk(
destination,
stringToChunk(escapeJSStringsForInstructionScripts(errorStack || '')),
);
} else {
// " data-stck="HTMLString
writeChunk(destination, clientRenderData4);
writeChunk(
destination,
stringToChunk(escapeTextForBrowser(errorStack || '')),
);
}
}
if (errorComponentStack) {
// ,"JSONString"
if (scriptFormat) {
Expand All @@ -4329,8 +4359,8 @@ export function writeClientRenderBoundaryInstruction(
),
);
} else {
// " data-stck="HTMLString
writeChunk(destination, clientRenderData4);
// " data-cstck="HTMLString
writeChunk(destination, clientRenderData5);
writeChunk(
destination,
stringToChunk(escapeTextForBrowser(errorComponentStack)),
Expand Down
2 changes: 2 additions & 0 deletions packages/react-dom-bindings/src/server/ReactFizzConfigDOMLegacy.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ export function writeStartClientRenderedSuspenseBoundary(
// flushing these error arguments are not currently supported in this legacy streaming format.
errorDigest: ?string,
errorMessage: ?string,
errorStack: ?string,
errorComponentStack: ?string,
): boolean {
if (renderState.generateStaticMarkup) {
Expand All @@ -231,6 +232,7 @@ export function writeStartClientRenderedSuspenseBoundary(
renderState,
errorDigest,
errorMessage,
errorStack,
errorComponentStack,
);
}
Expand Down
2 changes: 1 addition & 1 deletion ...m-bindings/src/server/fizz-instruction-set/ReactDOMFizzInstructionSetInlineCodeStrings.js
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion ...es/react-dom-bindings/src/server/fizz-instruction-set/ReactDOMFizzInstructionSetShared.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export function clientRenderBoundary(
suspenseBoundaryID,
errorDigest,
errorMsg,
errorStack,
errorComponentStack,
) {
// Find the fallback's first element.
Expand All @@ -36,7 +37,8 @@ export function clientRenderBoundary(
const dataset = suspenseIdNode.dataset;
if (errorDigest) dataset['dgst'] = errorDigest;
if (errorMsg) dataset['msg'] = errorMsg;
if (errorComponentStack) dataset['stck'] = errorComponentStack;
if (errorStack) dataset['stck'] = errorStack;
if (errorComponentStack) dataset['cstck'] = errorComponentStack;
// Tell React to retry it if the parent already hydrated.
if (suspenseNode['_reactRetry']) {
suspenseNode['_reactRetry']();
Expand Down
62 changes: 44 additions & 18 deletions packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -786,7 +786,8 @@ describe('ReactDOMFizzServer', () => {
errors,
[
[
theError.message,
'Switched to client rendering because the server rendering errored:\n\n' +
theError.message,
expectedDigest,
componentStack(['Lazy', 'Suspense', 'div', 'App']),
],
Expand Down Expand Up @@ -909,7 +910,8 @@ describe('ReactDOMFizzServer', () => {
errors,
[
[
theError.message,
'Switched to client rendering because the server rendering errored:\n\n' +
theError.message,
expectedDigest,
componentStack(['Lazy', 'Suspense', 'div', 'App']),
],
Expand Down Expand Up @@ -992,7 +994,8 @@ describe('ReactDOMFizzServer', () => {
errors,
[
[
theError.message,
'Switched to client rendering because the server rendering errored:\n\n' +
theError.message,
expectedDigest,
componentStack([
'Erroring',
Expand Down Expand Up @@ -1078,7 +1081,8 @@ describe('ReactDOMFizzServer', () => {
errors,
[
[
theError.message,
'Switched to client rendering because the server rendering errored:\n\n' +
theError.message,
expectedDigest,
componentStack(['Lazy', 'Suspense', 'div', 'App']),
],
Expand Down Expand Up @@ -1404,13 +1408,15 @@ describe('ReactDOMFizzServer', () => {
errors,
[
[
'The server did not finish this Suspense boundary: The render was aborted by the server without a reason.',
'Switched to client rendering because the server rendering aborted due to:\n\n' +
'The render was aborted by the server without a reason.',
expectedDigest,
// We get the stack of the task when it was aborted which is why we see `h1`
componentStack(['h1', 'Suspense', 'div', 'App']),
],
[
'The server did not finish this Suspense boundary: The render was aborted by the server without a reason.',
'Switched to client rendering because the server rendering aborted due to:\n\n' +
'The render was aborted by the server without a reason.',
expectedDigest,
componentStack(['Suspense', 'main', 'div', 'App']),
],
Expand Down Expand Up @@ -2145,7 +2151,8 @@ describe('ReactDOMFizzServer', () => {
errors,
[
[
theError.message,
'Switched to client rendering because the server rendering errored:\n\n' +
theError.message,
expectedDigest,
componentStack([
'AsyncText',
Expand Down Expand Up @@ -3431,12 +3438,14 @@ describe('ReactDOMFizzServer', () => {
errors,
[
[
'The server did not finish this Suspense boundary: foobar',
'Switched to client rendering because the server rendering aborted due to:\n\n' +
'foobar',
'a digest',
componentStack(['Suspense', 'p', 'div', 'App']),
],
[
'The server did not finish this Suspense boundary: foobar',
'Switched to client rendering because the server rendering aborted due to:\n\n' +
'foobar',
'a digest',
componentStack(['Suspense', 'span', 'div', 'App']),
],
Expand Down Expand Up @@ -3512,12 +3521,14 @@ describe('ReactDOMFizzServer', () => {
errors,
[
[
'The server did not finish this Suspense boundary: uh oh',
'Switched to client rendering because the server rendering aborted due to:\n\n' +
'uh oh',
'a digest',
componentStack(['Suspense', 'p', 'div', 'App']),
],
[
'The server did not finish this Suspense boundary: uh oh',
'Switched to client rendering because the server rendering aborted due to:\n\n' +
'uh oh',
'a digest',
componentStack(['Suspense', 'span', 'div', 'App']),
],
Expand Down Expand Up @@ -3991,7 +4002,8 @@ describe('ReactDOMFizzServer', () => {
errors,
[
[
theError.message,
'Switched to client rendering because the server rendering errored:\n\n' +
theError.message,
expectedDigest,
componentStack(['Erroring', 'Suspense', 'div', 'App']),
],
Expand Down Expand Up @@ -6772,7 +6784,14 @@ describe('ReactDOMFizzServer', () => {

expect(recoverableErrors).toEqual(
__DEV__
? ['server error', 'replay error', 'server error']
? [
'Switched to client rendering because the server rendering errored:\n\n' +
'server error',
'Switched to client rendering because the server rendering errored:\n\n' +
'replay error',
'Switched to client rendering because the server rendering errored:\n\n' +
'server error',
]
: [
'The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.',
'The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.',
Expand Down Expand Up @@ -6931,8 +6950,10 @@ describe('ReactDOMFizzServer', () => {
expect(recoverableErrors).toEqual(
__DEV__
? [
'The server did not finish this Suspense boundary: aborted',
'The server did not finish this Suspense boundary: aborted',
'Switched to client rendering because the server rendering aborted due to:\n\n' +
'aborted',
'Switched to client rendering because the server rendering aborted due to:\n\n' +
'aborted',
]
: [
'The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.',
Expand Down Expand Up @@ -7103,8 +7124,10 @@ describe('ReactDOMFizzServer', () => {
// It surfaced in two different suspense boundaries.
__DEV__
? [
'The server did not finish this Suspense boundary: replay error',
'The server did not finish this Suspense boundary: replay error',
'Switched to client rendering because the server rendering errored:\n\n' +
'replay error',
'Switched to client rendering because the server rendering errored:\n\n' +
'replay error',
]
: [
'The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.',
Expand Down Expand Up @@ -7230,7 +7253,10 @@ describe('ReactDOMFizzServer', () => {

expect(recoverableErrors).toEqual(
__DEV__
? ['server error']
? [
'Switched to client rendering because the server rendering errored:\n\n' +
'server error',
]
: [
'The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.',
],
Expand Down
Loading