From 9214ca924b19510157c377417703c7ecfa822c2a Mon Sep 17 00:00:00 2001 From: Josh Story Date: Mon, 6 Jun 2022 09:08:25 -0700 Subject: [PATCH] fix bug in legacy dom server preventing some errors from propagating to client --- .../src/__tests__/ReactDOMFizzServer-test.js | 44 +++++++++---------- .../ReactDOMFizzServerBrowser-test.js | 4 +- .../__tests__/ReactDOMFizzServerNode-test.js | 10 ++--- .../__tests__/ReactDOMHydrationDiff-test.js | 4 +- ...DOMServerPartialHydration-test.internal.js | 15 +++---- .../ReactDOMServerLegacyFormatConfig.js | 7 ++- .../ReactDOMServerFB-test.internal.js | 2 +- packages/react-server/src/ReactFizzServer.js | 2 +- scripts/error-codes/codes.json | 2 +- 9 files changed, 44 insertions(+), 46 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index f78e69ae7c945..7104350fde00e 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -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 () => { @@ -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, ], ], ); @@ -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}) { @@ -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, ], ], ); @@ -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 () => { @@ -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, ], ], ); @@ -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 () => { @@ -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, ], ], ); @@ -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; @@ -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, ], ], ); @@ -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; @@ -1834,7 +1834,7 @@ describe('ReactDOMFizzServer', () => { [ [ theError.message, - expectedHash, + expectedDigest, componentStack([ 'AsyncText', 'h1', @@ -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, ], ], ); @@ -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 () => { @@ -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, ], ], ); diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js index 3aacf68cc3b26..a1429b0d2a173 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js @@ -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.', ]); }); @@ -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; diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js index cb5ec892bd2c8..a625a8df0e2f0 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js @@ -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]); }); @@ -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'); @@ -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'); @@ -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); diff --git a/packages/react-dom/src/__tests__/ReactDOMHydrationDiff-test.js b/packages/react-dom/src/__tests__/ReactDOMHydrationDiff-test.js index 1b6a412406582..6de0a03df393c 100644 --- a/packages/react-dom/src/__tests__/ReactDOMHydrationDiff-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMHydrationDiff-test.js @@ -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.]", ] `); }); @@ -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.]", ] `); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js index e4d33645820e9..2a1aad18e4c67 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js @@ -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(); @@ -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); @@ -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); @@ -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. @@ -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(); diff --git a/packages/react-dom/src/server/ReactDOMServerLegacyFormatConfig.js b/packages/react-dom/src/server/ReactDOMServerLegacyFormatConfig.js index 4dff9651858b7..375562e80b56d 100644 --- a/packages/react-dom/src/server/ReactDOMServerLegacyFormatConfig.js +++ b/packages/react-dom/src/server/ReactDOMServerLegacyFormatConfig.js @@ -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 @@ -161,6 +161,9 @@ export function writeStartClientRenderedSuspenseBoundary( return writeStartClientRenderedSuspenseBoundaryImpl( destination, responseState, + errorDigest, + errorMessage, + errorComponentStack, ); } export function writeEndCompletedSuspenseBoundary( diff --git a/packages/react-server-dom-relay/src/__tests__/ReactDOMServerFB-test.internal.js b/packages/react-server-dom-relay/src/__tests__/ReactDOMServerFB-test.internal.js index 22ae1e6d71ec2..857a374f3e81a 100644 --- a/packages/react-server-dom-relay/src/__tests__/ReactDOMServerFB-test.internal.js +++ b/packages/react-server-dom-relay/src/__tests__/ReactDOMServerFB-test.internal.js @@ -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.', ]); }); }); diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index 31a22f132ecaf..2b0a3d82e0600 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -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__) { diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json index 7a93b9a0ac641..840b4dba57dd5 100644 --- a/scripts/error-codes/codes.json +++ b/scripts/error-codes/codes.json @@ -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." }