From aec575914a0fd76c9db8998ea62c3ee975de70d7 Mon Sep 17 00:00:00 2001 From: Josh Story Date: Sun, 29 May 2022 23:07:10 -0700 Subject: [PATCH] [Fizz] Send errors down to client (#24551) * use return from onError * export getSuspenseInstanceFallbackError * stringToChunk * return string from onError in downstream type signatures * 1 more type * support encoding errors in html stream and escape user input This commit adds another way to get errors to the suspense instance by encoding them as dataset properties of a template element at the head of the boundary. Previously if there was an error before the boundary flushed there was no way to stream the error to the client because there would never be a client render instruction. Additionally the error is sent in 3 parts 1) error hash - this is always sent (dev or prod) if one is provided 2) error message - Dev only 3) error component stack - Dev only, this now captures the stack at the point of error Another item addressed in this commit is the escaping of potentially unsafe data. all error components are escaped as test for browers when written into the html and as javascript strings when written into a client render instruction. * nits Co-authored-by: Marco Salazar --- .../src/__tests__/ReactDOMFizzServer-test.js | 457 ++++++++++++++++-- .../ReactDOMFizzServerBrowser-test.js | 22 +- .../__tests__/ReactDOMFizzServerNode-test.js | 30 +- .../src/client/ReactDOMHostConfig.js | 24 + .../src/server/ReactDOMFizzServerBrowser.js | 2 +- .../src/server/ReactDOMFizzServerNode.js | 2 +- .../src/server/ReactDOMServerFormatConfig.js | 110 ++++- .../ReactDOMServerLegacyFormatConfig.js | 4 + .../server/ReactNativeServerFormatConfig.js | 8 + .../src/ReactNoopServer.js | 2 +- .../src/ReactFiberBeginWork.new.js | 21 +- .../src/ReactFiberBeginWork.old.js | 21 +- .../ReactFiberHostConfigWithNoHydration.js | 1 + .../src/forks/ReactFiberHostConfig.custom.js | 2 + .../ReactDOMServerFB-test.internal.js | 7 +- packages/react-server/src/ReactFizzServer.js | 129 ++++- scripts/error-codes/codes.json | 3 +- 17 files changed, 768 insertions(+), 77 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index 56ae7c8a09652..66ae2c25e6901 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -89,6 +89,48 @@ describe('ReactDOMFizzServer', () => { }); }); + function expectErrors(errorsArr, toBeDevArr, toBeProdArr) { + const mappedErrows = errorsArr.map(error => { + if (error.componentStack) { + return [ + error.message, + error.hash, + normalizeCodeLocInfo(error.componentStack), + ]; + } else if (error.hash) { + return [error.message, error.hash]; + } + return error.message; + }); + if (__DEV__) { + expect(mappedErrows).toEqual( + toBeDevArr, + // .map(([errorMessage, errorHash, errorComponentStack]) => { + // if (typeof error === 'string' || error instanceof String) { + // return error; + // } + // let str = JSON.stringify(error).replace(/\\n/g, '\n'); + // // this gets stripped away by normalizeCodeLocInfo... + // // Kind of hacky but lets strip it away here too just so they match... + // // easier than fixing the regex to account for this edge case + // if (str.endsWith('at **)"}')) { + // str = str.replace(/at \*\*\)\"}$/, 'at **)'); + // } + // return str; + // }), + ); + } else { + expect(mappedErrows).toEqual(toBeProdArr); + } + } + + // @TODO we will use this in a followup change once we start exposing componentStacks from server errors + // function componentStack(components) { + // return components + // .map(component => `\n in ${component} (at **)`) + // .join(''); + // } + async function act(callback) { await callback(); // Await one turn around the event loop. @@ -413,8 +455,6 @@ describe('ReactDOMFizzServer', () => { }); }); - const loggedErrors = []; - function App({isClient}) { return (
@@ -426,24 +466,32 @@ describe('ReactDOMFizzServer', () => { } let bootstrapped = false; + const errors = []; window.__INIT__ = function() { bootstrapped = true; // Attempt to hydrate the content. ReactDOMClient.hydrateRoot(container, , { onRecoverableError(error) { - Scheduler.unstable_yieldValue(error.message); + errors.push(error); }, }); }; + const theError = new Error('Test'); + const loggedErrors = []; + function onError(x) { + loggedErrors.push(x); + return 'Hash of (' + x.message + ')'; + } + // const expectedHash = onError(theError); + // loggedErrors.length = 0; + await act(async () => { const {pipe} = ReactDOMFizzServer.renderToPipeableStream( , { bootstrapScriptContent: '__INIT__();', - onError(x) { - loggedErrors.push(x); - }, + onError, }, ); pipe(writable); @@ -458,7 +506,6 @@ describe('ReactDOMFizzServer', () => { expect(loggedErrors).toEqual([]); - const theError = new Error('Test'); await act(async () => { rejectComponent(theError); }); @@ -469,10 +516,14 @@ describe('ReactDOMFizzServer', () => { expect(getVisibleChildren(container)).toEqual(
Loading...
); // Now we can client render it instead. - expect(Scheduler).toFlushAndYield([ - 'The server could not finish this Suspense boundary, likely due to ' + - 'an error during server rendering. Switched to client rendering.', - ]); + expect(Scheduler).toFlushAndYield([]); + expectErrors( + errors, + [theError.message], + [ + 'The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.', + ], + ); // The client rendered HTML is now in place. expect(getVisibleChildren(container)).toEqual(
Hello
); @@ -520,7 +571,14 @@ describe('ReactDOMFizzServer', () => { }); }); + const theError = new Error('Test'); const loggedErrors = []; + function onError(x) { + loggedErrors.push(x); + return 'hash of (' + x.message + ')'; + } + // const expectedHash = onError(theError); + // loggedErrors.length = 0; function App({isClient}) { return ( @@ -537,19 +595,18 @@ describe('ReactDOMFizzServer', () => { , { - onError(x) { - loggedErrors.push(x); - }, + onError, }, ); pipe(writable); }); expect(loggedErrors).toEqual([]); + const errors = []; // Attempt to hydrate the content. ReactDOMClient.hydrateRoot(container, , { onRecoverableError(error) { - Scheduler.unstable_yieldValue(error.message); + errors.push(error); }, }); Scheduler.unstable_flushAll(); @@ -559,7 +616,6 @@ describe('ReactDOMFizzServer', () => { expect(loggedErrors).toEqual([]); - const theError = new Error('Test'); await act(async () => { rejectElement(theError); }); @@ -570,14 +626,161 @@ describe('ReactDOMFizzServer', () => { expect(getVisibleChildren(container)).toEqual(
Loading...
); // Now we can client render it instead. - expect(Scheduler).toFlushAndYield([ - 'The server could not finish this Suspense boundary, likely due to ' + - 'an error during server rendering. Switched to client rendering.', - ]); + expect(Scheduler).toFlushAndYield([]); + + expectErrors( + errors, + [theError.message], + [ + 'The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.', + ], + ); // The client rendered HTML is now in place. - expect(getVisibleChildren(container)).toEqual(
Hello
); + // expect(getVisibleChildren(container)).toEqual(
Hello
); + + expect(loggedErrors).toEqual([theError]); + }); + + // @gate experimental + it('Errors in boundaries should be sent to the client and reported on client render - Error before flushing', async () => { + function Indirection({level, children}) { + if (level > 0) { + return {children}; + } + return children; + } + + const theError = new Error('uh oh'); + + function Erroring({isClient}) { + if (isClient) { + return 'Hello World'; + } + throw theError; + } + + function App({isClient}) { + return ( +
+ loading...}> + + +
+ ); + } + + const loggedErrors = []; + function onError(x) { + loggedErrors.push(x); + return 'hash(' + x.message + ')'; + } + // const expectedHash = onError(theError); + // loggedErrors.length = 0; + + await act(async () => { + const {pipe} = ReactDOMFizzServer.renderToPipeableStream( + , + { + onError, + }, + ); + pipe(writable); + }); + expect(loggedErrors).toEqual([theError]); + + const errors = []; + // Attempt to hydrate the content. + ReactDOMClient.hydrateRoot(container, , { + onRecoverableError(error) { + errors.push(error); + }, + }); + Scheduler.unstable_flushAll(); + + expect(getVisibleChildren(container)).toEqual(
Hello World
); + + expectErrors( + errors, + [theError.message], + [ + 'The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.', + ], + ); + }); + + // @gate experimental + it('Errors in boundaries should be sent to the client and reported on client render - Error after flushing', async () => { + let rejectComponent; + const LazyComponent = React.lazy(() => { + return new Promise((resolve, reject) => { + rejectComponent = reject; + }); + }); + + function App({isClient}) { + return ( +
+ }> + {isClient ? : } + +
+ ); + } + + const loggedErrors = []; + const theError = new Error('uh oh'); + function onError(x) { + loggedErrors.push(x); + return 'hash(' + x.message + ')'; + } + // const expectedHash = onError(theError); + // loggedErrors.length = 0; + + await act(async () => { + const {pipe} = ReactDOMFizzServer.renderToPipeableStream( + , + + { + onError, + }, + ); + pipe(writable); + }); + expect(loggedErrors).toEqual([]); + + const errors = []; + // Attempt to hydrate the content. + ReactDOMClient.hydrateRoot(container, , { + onRecoverableError(error) { + errors.push(error); + }, + }); + Scheduler.unstable_flushAll(); + + expect(getVisibleChildren(container)).toEqual(
Loading...
); + + await act(async () => { + rejectComponent(theError); + }); + + expect(loggedErrors).toEqual([theError]); + expect(getVisibleChildren(container)).toEqual(
Loading...
); + + // Now we can client render it instead. + expect(Scheduler).toFlushAndYield([]); + + expectErrors( + errors, + [theError.message], + [ + 'The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.', + ], + ); + + // The client rendered HTML is now in place. + expect(getVisibleChildren(container)).toEqual(
Hello
); expect(loggedErrors).toEqual([theError]); }); @@ -849,18 +1052,25 @@ describe('ReactDOMFizzServer', () => { ); } + const loggedErrors = []; + function onError(error) { + loggedErrors.push(error); + return `Hash of (${error.message})`; + } + let controls; await act(async () => { - controls = ReactDOMFizzServer.renderToPipeableStream(); + controls = ReactDOMFizzServer.renderToPipeableStream(, {onError}); controls.pipe(writable); }); // We're still showing a fallback. + const errors = []; // Attempt to hydrate the content. ReactDOMClient.hydrateRoot(container, , { onRecoverableError(error) { - Scheduler.unstable_yieldValue(error.message); + errors.push(error); }, }); Scheduler.unstable_flushAll(); @@ -874,10 +1084,14 @@ describe('ReactDOMFizzServer', () => { }); // We still can't render it on the client. - expect(Scheduler).toFlushAndYield([ - 'The server could not finish this Suspense boundary, likely due to an ' + - 'error during server rendering. Switched to client rendering.', - ]); + expect(Scheduler).toFlushAndYield([]); + expectErrors( + errors, + ['This Suspense boundary was aborted by the server'], + [ + 'The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.', + ], + ); expect(getVisibleChildren(container)).toEqual(
Loading...
); // We now resolve it on the client. @@ -1535,16 +1749,22 @@ describe('ReactDOMFizzServer', () => { ); } + const theError = new Error('Test'); const loggedErrors = []; + function onError(x) { + loggedErrors.push(x); + return `hash of (${x.message})`; + } + // const expectedHash = onError(theError); + // loggedErrors.length = 0; + let controls; await act(async () => { controls = ReactDOMFizzServer.renderToPipeableStream( , { - onError(x) { - loggedErrors.push(x); - }, + onError, }, ); controls.pipe(writable); @@ -1552,10 +1772,11 @@ describe('ReactDOMFizzServer', () => { // We're still showing a fallback. + const errors = []; // Attempt to hydrate the content. ReactDOMClient.hydrateRoot(container, , { onRecoverableError(error) { - Scheduler.unstable_yieldValue(error.message); + errors.push(error); }, }); Scheduler.unstable_flushAll(); @@ -1565,7 +1786,6 @@ describe('ReactDOMFizzServer', () => { expect(loggedErrors).toEqual([]); - const theError = new Error('Test'); // Error the content, but we don't have a fallback yet. await act(async () => { rejectText('Hello', theError); @@ -1586,10 +1806,14 @@ describe('ReactDOMFizzServer', () => { expect(getVisibleChildren(container)).toEqual(
Loading...
); // That will let us client render it instead. - expect(Scheduler).toFlushAndYield([ - 'The server could not finish this Suspense boundary, likely due to ' + - 'an error during server rendering. Switched to client rendering.', - ]); + expect(Scheduler).toFlushAndYield([]); + expectErrors( + errors, + [theError.message], + [ + 'The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.', + ], + ); // The client rendered HTML is now in place. expect(getVisibleChildren(container)).toEqual( @@ -2178,11 +2402,10 @@ describe('ReactDOMFizzServer', () => { // Hydrate the tree. Child will throw during render. isClient = true; + const errors = []; ReactDOMClient.hydrateRoot(container, , { onRecoverableError(error) { - Scheduler.unstable_yieldValue( - 'Log recoverable error: ' + error.message, - ); + errors.push(error.message); }, }); @@ -2190,6 +2413,8 @@ describe('ReactDOMFizzServer', () => { // shouldn't be called. expect(Scheduler).toFlushAndYield([]); expect(getVisibleChildren(container)).toEqual('Oops!'); + + expectErrors(errors, [], []); }, ); @@ -2794,6 +3019,160 @@ describe('ReactDOMFizzServer', () => { ); }); + describe('error escaping', () => { + //@gate experimental + it('escapes error hash, message, and component stack values in directly flushed errors (html escaping)', async () => { + window.__outlet = {}; + + const dangerousErrorString = + '">