Skip to content

Commit

Permalink
fix bug in legacy dom server preventing some errors from propagating …
Browse files Browse the repository at this point in the history 
…to client
  • Loading branch information
@gnoff
gnoff committed Jun 6, 2022
1 parent e4b4365 commit 9214ca9
Show file tree
Hide file tree
Showing 9 changed files with 44 additions and 46 deletions.
44 changes: 22 additions & 22 deletions packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
View file
Expand Up @@ -465,7 +465,7 @@ describe('ReactDOMFizzServer', () => {
loggedErrors.push(x);
return 'Hash of (' + x.message + ')';
}
const expectedHash = onError(theError);
const expectedDigest = onError(theError);
loggedErrors.length = 0;

await act(async () => {
Expand Down Expand Up @@ -504,14 +504,14 @@ describe('ReactDOMFizzServer', () => {
[
[
theError.message,
expectedHash,
expectedDigest,
componentStack(['Lazy', 'Suspense', 'div', 'App']),
],
],
[
[
'The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.',
expectedHash,
expectedDigest,
],
],
);
Expand Down Expand Up @@ -568,7 +568,7 @@ describe('ReactDOMFizzServer', () => {
loggedErrors.push(x);
return 'hash of (' + x.message + ')';
}
const expectedHash = onError(theError);
const expectedDigest = onError(theError);
loggedErrors.length = 0;

function App({isClient}) {
Expand Down Expand Up @@ -624,14 +624,14 @@ describe('ReactDOMFizzServer', () => {
[
[
theError.message,
expectedHash,
expectedDigest,
componentStack(['Suspense', 'div', 'App']),
],
],
[
[
'The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.',
expectedHash,
expectedDigest,
],
],
);
Expand Down Expand Up @@ -675,7 +675,7 @@ describe('ReactDOMFizzServer', () => {
loggedErrors.push(x);
return 'hash(' + x.message + ')';
}
const expectedHash = onError(theError);
const expectedDigest = onError(theError);
loggedErrors.length = 0;

await act(async () => {
Expand Down Expand Up @@ -706,14 +706,14 @@ describe('ReactDOMFizzServer', () => {
[
[
theError.message,
expectedHash,
expectedDigest,
componentStack(['Erroring', 'Suspense', 'div', 'App']),
],
],
[
[
'The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.',
expectedHash,
expectedDigest,
],
],
);
Expand Down Expand Up @@ -744,7 +744,7 @@ describe('ReactDOMFizzServer', () => {
loggedErrors.push(x);
return 'hash(' + x.message + ')';
}
const expectedHash = onError(theError);
const expectedDigest = onError(theError);
loggedErrors.length = 0;

await act(async () => {
Expand Down Expand Up @@ -785,14 +785,14 @@ describe('ReactDOMFizzServer', () => {
[
[
theError.message,
expectedHash,
expectedDigest,
componentStack(['Lazy', 'Suspense', 'div', 'App']),
],
],
[
[
'The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.',
expectedHash,
expectedDigest,
],
],
);
Expand Down Expand Up @@ -1071,10 +1071,10 @@ describe('ReactDOMFizzServer', () => {
}

const loggedErrors = [];
const expectedHash = 'Hash for Abort';
const expectedDigest = 'Hash for Abort';
function onError(error) {
loggedErrors.push(error);
return expectedHash;
return expectedDigest;
}

let controls;
Expand Down Expand Up @@ -1106,11 +1106,11 @@ describe('ReactDOMFizzServer', () => {
expect(Scheduler).toFlushAndYield([]);
expectErrors(
errors,
[['This Suspense boundary was aborted by the server', expectedHash]],
[['This Suspense boundary was aborted by the server.', expectedDigest]],
[
[
'The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.',
expectedHash,
expectedDigest,
],
],
);
Expand Down Expand Up @@ -1777,7 +1777,7 @@ describe('ReactDOMFizzServer', () => {
loggedErrors.push(x);
return `hash of (${x.message})`;
}
const expectedHash = onError(theError);
const expectedDigest = onError(theError);
loggedErrors.length = 0;

let controls;
Expand Down Expand Up @@ -1834,7 +1834,7 @@ describe('ReactDOMFizzServer', () => {
[
[
theError.message,
expectedHash,
expectedDigest,
componentStack([
'AsyncText',
'h1',
Expand All @@ -1848,7 +1848,7 @@ describe('ReactDOMFizzServer', () => {
[
[
'The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.',
expectedHash,
expectedDigest,
],
],
);
Expand Down Expand Up @@ -3180,7 +3180,7 @@ describe('ReactDOMFizzServer', () => {
loggedErrors.push(x);
return x.message.replace('bad message', 'bad hash');
}
const expectedHash = onError(theError);
const expectedDigest = onError(theError);
loggedErrors.length = 0;

await act(async () => {
Expand All @@ -3206,14 +3206,14 @@ describe('ReactDOMFizzServer', () => {
[
[
theError.message,
expectedHash,
expectedDigest,
componentStack(['Erroring', 'Suspense', 'div', 'App']),
],
],
[
[
'The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.',
expectedHash,
expectedDigest,
],
],
);
Expand Down
4 changes: 2 additions & 2 deletions packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js
View file
Expand Up @@ -215,7 +215,7 @@ describe('ReactDOMFizzServer', () => {
expect(result).toContain('Loading');

expect(errors).toEqual([
'This Suspense boundary was aborted by the server',
'This Suspense boundary was aborted by the server.',
]);
});

Expand Down Expand Up @@ -256,7 +256,7 @@ describe('ReactDOMFizzServer', () => {
reader.cancel();

expect(errors).toEqual([
'This Suspense boundary was aborted by the server',
'This Suspense boundary was aborted by the server.',
]);

hasLoaded = true;
Expand Down
10 changes: 5 additions & 5 deletions packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js
View file
Expand Up @@ -226,7 +226,7 @@ describe('ReactDOMFizzServer', () => {
expect(output.result).toBe('');
expect(reportedErrors).toEqual([
theError.message,
'This Suspense boundary was aborted by the server',
'This Suspense boundary was aborted by the server.',
]);
expect(reportedShellErrors).toEqual([theError]);
});
Expand Down Expand Up @@ -322,7 +322,7 @@ describe('ReactDOMFizzServer', () => {
await completed;

expect(errors).toEqual([
'This Suspense boundary was aborted by the server',
'This Suspense boundary was aborted by the server.',
]);
expect(output.error).toBe(undefined);
expect(output.result).toContain('Loading');
Expand Down Expand Up @@ -365,8 +365,8 @@ describe('ReactDOMFizzServer', () => {

expect(errors).toEqual([
// There are two boundaries that abort
'This Suspense boundary was aborted by the server',
'This Suspense boundary was aborted by the server',
'This Suspense boundary was aborted by the server.',
'This Suspense boundary was aborted by the server.',
]);
expect(output.error).toBe(undefined);
expect(output.result).toContain('Loading');
Expand Down Expand Up @@ -603,7 +603,7 @@ describe('ReactDOMFizzServer', () => {
await completed;

expect(errors).toEqual([
'This Suspense boundary was aborted by the server',
'This Suspense boundary was aborted by the server.',
]);
expect(rendered).toBe(false);
expect(isComplete).toBe(true);
Expand Down
4 changes: 2 additions & 2 deletions packages/react-dom/src/__tests__/ReactDOMHydrationDiff-test.js
View file
Expand Up @@ -815,7 +815,7 @@ describe('ReactDOMServerHydration', () => {
}
expect(testMismatch(Mismatch)).toMatchInlineSnapshot(`
Array [
"Caught [The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.]",
"Caught [This Suspense boundary was aborted by the server.]",
]
`);
});
Expand All @@ -840,7 +840,7 @@ describe('ReactDOMServerHydration', () => {
}
expect(testMismatch(Mismatch)).toMatchInlineSnapshot(`
Array [
"Caught [The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.]",
"Caught [This Suspense boundary was aborted by the server.]",
]
`);
});
Expand Down
15 changes: 5 additions & 10 deletions packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js
View file
Expand Up @@ -1669,8 +1669,7 @@ describe('ReactDOMServerPartialHydration', () => {
},
});
expect(Scheduler).toFlushAndYield([
'The server could not finish this Suspense boundary, likely due to ' +
'an error during server rendering. Switched to client rendering.',
'This Suspense boundary was aborted by the server.',
]);
jest.runAllTimers();

Expand Down Expand Up @@ -1731,8 +1730,7 @@ describe('ReactDOMServerPartialHydration', () => {
},
});
expect(Scheduler).toFlushAndYield([
'The server could not finish this Suspense boundary, likely due to ' +
'an error during server rendering. Switched to client rendering.',
'This Suspense boundary was aborted by the server.',
]);
// This will have exceeded the suspended time so we should timeout.
jest.advanceTimersByTime(500);
Expand Down Expand Up @@ -1798,8 +1796,7 @@ describe('ReactDOMServerPartialHydration', () => {
},
});
expect(Scheduler).toFlushAndYield([
'The server could not finish this Suspense boundary, likely due to ' +
'an error during server rendering. Switched to client rendering.',
'This Suspense boundary was aborted by the server.',
]);
// This will have exceeded the suspended time so we should timeout.
jest.advanceTimersByTime(500);
Expand Down Expand Up @@ -2116,8 +2113,7 @@ describe('ReactDOMServerPartialHydration', () => {

suspend = true;
expect(Scheduler).toFlushAndYield([
'The server could not finish this Suspense boundary, likely due to ' +
'an error during server rendering. Switched to client rendering.',
'This Suspense boundary was aborted by the server.',
]);

// We haven't hydrated the second child but the placeholder is still in the list.
Expand Down Expand Up @@ -2179,8 +2175,7 @@ describe('ReactDOMServerPartialHydration', () => {
},
});
expect(Scheduler).toFlushAndYield([
'The server could not finish this Suspense boundary, likely due to ' +
'an error during server rendering. Switched to client rendering.',
'This Suspense boundary was aborted by the server.',
]);
jest.runAllTimers();

Expand Down
7 changes: 5 additions & 2 deletions packages/react-dom/src/server/ReactDOMServerLegacyFormatConfig.js
View file
Expand Up @@ -150,8 +150,8 @@ export function writeStartClientRenderedSuspenseBoundary(
responseState: ResponseState,
// flushing these error arguments are not currently supported in this legacy streaming format.
errorDigest: ?string,
errorMessage?: string,
errorComponentStack?: string,
errorMessage: ?string,
errorComponentStack: ?string,
): boolean {
if (responseState.generateStaticMarkup) {
// A client rendered boundary is done and doesn't need a representation in the HTML
Expand All @@ -161,6 +161,9 @@ export function writeStartClientRenderedSuspenseBoundary(
return writeStartClientRenderedSuspenseBoundaryImpl(
destination,
responseState,
errorDigest,
errorMessage,
errorComponentStack,
);
}
export function writeEndCompletedSuspenseBoundary(
Expand Down
2 changes: 1 addition & 1 deletion packages/react-server-dom-relay/src/__tests__/ReactDOMServerFB-test.internal.js
View file
Expand Up @@ -192,7 +192,7 @@ describe('ReactDOMServerFB', () => {
expect(remaining).toEqual('');

expect(errors).toEqual([
'This Suspense boundary was aborted by the server',
'This Suspense boundary was aborted by the server.',
]);
});
});
2 changes: 1 addition & 1 deletion packages/react-server/src/ReactFizzServer.js
View file
Expand Up @@ -1554,7 +1554,7 @@ function abortTask(task: Task): void {
if (!boundary.forceClientRender) {
boundary.forceClientRender = true;
const error = new Error(
'This Suspense boundary was aborted by the server',
'This Suspense boundary was aborted by the server.',
);
boundary.errorDigest = request.onError(error);
if (__DEV__) {
Expand Down
2 changes: 1 addition & 1 deletion scripts/error-codes/codes.json
View file
Expand Up @@ -417,5 +417,5 @@
"429": "ServerContext: %s already defined",
"430": "ServerContext can only have a value prop and children. Found: %s",
"431": "React elements are not allowed in ServerContext",
"432": "This Suspense boundary was aborted by the server"
"432": "This Suspense boundary was aborted by the server."
}

0 comments on commit 9214ca9

Please sign in to comment.