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

[Fizz] Send errors down to client #24551

Merged
merged 7 commits into from
May 30, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Next Next commit
use return from onError
  • Loading branch information
salazarm authored and gnoff committed May 30, 2022
commit 1fe9dc6265bfba24bb6c7cefa39e5d8fdcf67fca
113 changes: 90 additions & 23 deletions packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,34 @@ describe('ReactDOMFizzServer', () => {
});
});

function expectErrors(errorsArr, toBeDevArr, toBeProdArr) {
if (__DEV__) {
expect(errorsArr.map(error => normalizeCodeLocInfo(error))).toEqual(
toBeDevArr.map(error => {
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(errorsArr).toEqual(toBeProdArr);
}
}

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.
Expand Down Expand Up @@ -426,12 +454,13 @@ describe('ReactDOMFizzServer', () => {
}

let bootstrapped = false;
const errors = [];
window.__INIT__ = function() {
bootstrapped = true;
// Attempt to hydrate the content.
ReactDOMClient.hydrateRoot(container, <App isClient={true} />, {
onRecoverableError(error) {
Scheduler.unstable_yieldValue(error.message);
errors.push(error.message);
},
});
};
Expand All @@ -443,6 +472,7 @@ describe('ReactDOMFizzServer', () => {
bootstrapScriptContent: '__INIT__();',
onError(x) {
loggedErrors.push(x);
return 'Hash';
},
},
);
Expand All @@ -469,10 +499,17 @@ describe('ReactDOMFizzServer', () => {
expect(getVisibleChildren(container)).toEqual(<div>Loading...</div>);

// 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,
[
{
error: theError.message,
componentStack: componentStack(['Lazy', 'Suspense', 'div', 'App']),
},
],
['Hash'],
);

// The client rendered HTML is now in place.
expect(getVisibleChildren(container)).toEqual(<div>Hello</div>);
Expand Down Expand Up @@ -539,17 +576,19 @@ describe('ReactDOMFizzServer', () => {
{
onError(x) {
loggedErrors.push(x);
return 'hash';
},
},
);
pipe(writable);
});
expect(loggedErrors).toEqual([]);

const errors = [];
// Attempt to hydrate the content.
ReactDOMClient.hydrateRoot(container, <App isClient={true} />, {
onRecoverableError(error) {
Scheduler.unstable_yieldValue(error.message);
errors.push(error.message);
},
});
Scheduler.unstable_flushAll();
Expand All @@ -570,10 +609,18 @@ describe('ReactDOMFizzServer', () => {
expect(getVisibleChildren(container)).toEqual(<div>Loading...</div>);

// 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,
[
{
error: theError.message,
componentStack: componentStack(['div', 'App']),
},
],
['hash'],
);

// The client rendered HTML is now in place.
expect(getVisibleChildren(container)).toEqual(<div>Hello</div>);
Expand Down Expand Up @@ -857,10 +904,11 @@ describe('ReactDOMFizzServer', () => {

// We're still showing a fallback.

const errors = [];
// Attempt to hydrate the content.
ReactDOMClient.hydrateRoot(container, <App />, {
onRecoverableError(error) {
Scheduler.unstable_yieldValue(error.message);
errors.push(error.message);
},
});
Scheduler.unstable_flushAll();
Expand All @@ -874,10 +922,12 @@ 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'],
['This Suspense boundary was aborted by the server'],
);
expect(getVisibleChildren(container)).toEqual(<div>Loading...</div>);

// We now resolve it on the client.
Expand Down Expand Up @@ -1544,6 +1594,7 @@ describe('ReactDOMFizzServer', () => {
{
onError(x) {
loggedErrors.push(x);
return 'error hash';
},
},
);
Expand All @@ -1552,10 +1603,11 @@ describe('ReactDOMFizzServer', () => {

// We're still showing a fallback.

const errors = [];
// Attempt to hydrate the content.
ReactDOMClient.hydrateRoot(container, <App isClient={true} />, {
onRecoverableError(error) {
Scheduler.unstable_yieldValue(error.message);
errors.push(error.message);
},
});
Scheduler.unstable_flushAll();
Expand Down Expand Up @@ -1586,10 +1638,24 @@ describe('ReactDOMFizzServer', () => {
expect(getVisibleChildren(container)).toEqual(<div>Loading...</div>);

// 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,
[
{
error: theError.message,
componentStack: componentStack([
'AsyncText',
'h1',
'Suspense',
'div',
'Suspense',
'App',
]),
},
],
['error hash'],
);

// The client rendered HTML is now in place.
expect(getVisibleChildren(container)).toEqual(
Expand Down Expand Up @@ -2178,18 +2244,19 @@ describe('ReactDOMFizzServer', () => {

// Hydrate the tree. Child will throw during render.
isClient = true;
const errors = [];
ReactDOMClient.hydrateRoot(container, <App />, {
onRecoverableError(error) {
Scheduler.unstable_yieldValue(
'Log recoverable error: ' + error.message,
);
errors.push(error.message);
},
});

// Because we failed to recover from the error, onRecoverableError
// shouldn't be called.
expect(Scheduler).toFlushAndYield([]);
expect(getVisibleChildren(container)).toEqual('Oops!');

expectErrors(errors, [], []);
},
);

Expand Down
5 changes: 5 additions & 0 deletions packages/react-dom/src/client/ReactDOMHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -729,6 +729,11 @@ export function isSuspenseInstancePending(instance: SuspenseInstance) {
export function isSuspenseInstanceFallback(instance: SuspenseInstance) {
return instance.data === SUSPENSE_FALLBACK_START_DATA;
}
export function getSuspenseInstanceFallbackError(
instance: SuspenseInstance,
): string {
return (instance: any).data2;
}

export function registerSuspenseInstanceRetry(
instance: SuspenseInstance,
Expand Down
2 changes: 1 addition & 1 deletion packages/react-dom/src/server/ReactDOMFizzServerNode.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ type Options = {|
onShellReady?: () => void,
onShellError?: (error: mixed) => void,
onAllReady?: () => void,
onError?: (error: mixed) => void,
onError?: (error: mixed) => string,
|};

type PipeableStream = {|
Expand Down
17 changes: 12 additions & 5 deletions packages/react-dom/src/server/ReactDOMServerFormatConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -1724,7 +1724,7 @@ export function writeEndSegment(
// const SUSPENSE_PENDING_START_DATA = '$?';
// const SUSPENSE_FALLBACK_START_DATA = '$!';
//
// function clientRenderBoundary(suspenseBoundaryID) {
// function clientRenderBoundary(suspenseBoundaryID, errorMsg) {
// // Find the fallback's first element.
// const suspenseIdNode = document.getElementById(suspenseBoundaryID);
// if (!suspenseIdNode) {
Expand All @@ -1736,6 +1736,7 @@ export function writeEndSegment(
// const suspenseNode = suspenseIdNode.previousSibling;
// // Tag it to be client rendered.
// suspenseNode.data = SUSPENSE_FALLBACK_START_DATA;
// suspenseNode.data2 = errorMsg;
// // Tell React to retry it if the parent already hydrated.
// if (suspenseNode._reactRetry) {
// suspenseNode._reactRetry();
Expand Down Expand Up @@ -1823,7 +1824,7 @@ const completeSegmentFunction =
const completeBoundaryFunction =
'function $RC(a,b){a=document.getElementById(a);b=document.getElementById(b);b.parentNode.removeChild(b);if(a){a=a.previousSibling;var f=a.parentNode,c=a.nextSibling,e=0;do{if(c&&8===c.nodeType){var d=c.data;if("/$"===d)if(0===e)break;else e--;else"$"!==d&&"$?"!==d&&"$!"!==d||e++}d=c.nextSibling;f.removeChild(c);c=d}while(c);for(;b.firstChild;)f.insertBefore(b.firstChild,c);a.data="$";a._reactRetry&&a._reactRetry()}}';
const clientRenderFunction =
'function $RX(a){if(a=document.getElementById(a))a=a.previousSibling,a.data="$!",a._reactRetry&&a._reactRetry()}';
'function $RX(a,b){if(a=document.getElementById(a))a=a.previousSibling,a.data="$!",a.data2=b,a._reactRetry&&a._reactRetry()}';

const completeSegmentScript1Full = stringToPrecomputedChunk(
completeSegmentFunction + ';$RS("',
Expand Down Expand Up @@ -1893,15 +1894,17 @@ export function writeCompletedBoundaryInstruction(
}

const clientRenderScript1Full = stringToPrecomputedChunk(
clientRenderFunction + ';$RX("',
clientRenderFunction + ";$RX('",
);
const clientRenderScript1Partial = stringToPrecomputedChunk('$RX("');
const clientRenderScript2 = stringToPrecomputedChunk('")</script>');
const clientRenderScript1Partial = stringToPrecomputedChunk("$RX('");
const clientRenderScript2 = stringToPrecomputedChunk("')</script>");
const clientRenderErrorScript1 = stringToPrecomputedChunk("','");

export function writeClientRenderBoundaryInstruction(
destination: Destination,
responseState: ResponseState,
boundaryID: SuspenseBoundaryID,
error: ?string,
): boolean {
writeChunk(destination, responseState.startInlineScript);
if (!responseState.sentClientRenderFunction) {
Expand All @@ -1920,5 +1923,9 @@ export function writeClientRenderBoundaryInstruction(
}

writeChunk(destination, boundaryID);
if (error) {
writeChunk(destination, clientRenderErrorScript1);
writeChunk(destination, error);
}
return writeChunkAndReturn(destination, clientRenderScript2);
}
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,8 @@ export function writeClientRenderBoundaryInstruction(
destination: Destination,
responseState: ResponseState,
boundaryID: SuspenseBoundaryID,
// TODO: encode error for native
error: ?string,
): boolean {
writeChunk(destination, SUSPENSE_UPDATE_TO_CLIENT_RENDER);
return writeChunkAndReturn(destination, formatID(boundaryID));
Expand Down
15 changes: 10 additions & 5 deletions packages/react-reconciler/src/ReactFiberBeginWork.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ import {
shouldSetTextContent,
isSuspenseInstancePending,
isSuspenseInstanceFallback,
getSuspenseInstanceFallbackError,
registerSuspenseInstanceRetry,
supportsHydration,
isPrimaryRenderer,
Expand Down Expand Up @@ -2572,18 +2573,22 @@ function updateDehydratedSuspenseComponent(
// This boundary is in a permanent fallback state. In this case, we'll never
// get an update and we'll never be able to hydrate the final content. Let's just try the
// client side render instead.
const errorMsg = getSuspenseInstanceFallbackError(suspenseInstance);
return retrySuspenseComponentWithoutHydrating(
current,
workInProgress,
renderLanes,
// TODO: The server should serialize the error message so we can log it
// here on the client. Or, in production, a hash/id that corresponds to
// the error.
new Error(
'The server could not finish this Suspense boundary, likely ' +
'due to an error during server rendering. Switched to ' +
'client rendering.',
),
errorMsg
? // eslint-disable-next-line react-internal/prod-error-codes
new Error(errorMsg)
: new Error(
'The server could not finish this Suspense boundary, likely ' +
'due to an error during server rendering. Switched to ' +
'client rendering.',
),
);
}

Expand Down
15 changes: 10 additions & 5 deletions packages/react-reconciler/src/ReactFiberBeginWork.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ import {
shouldSetTextContent,
isSuspenseInstancePending,
isSuspenseInstanceFallback,
getSuspenseInstanceFallbackError,
registerSuspenseInstanceRetry,
supportsHydration,
isPrimaryRenderer,
Expand Down Expand Up @@ -2572,18 +2573,22 @@ function updateDehydratedSuspenseComponent(
// This boundary is in a permanent fallback state. In this case, we'll never
// get an update and we'll never be able to hydrate the final content. Let's just try the
// client side render instead.
const errorMsg = getSuspenseInstanceFallbackError(suspenseInstance);
return retrySuspenseComponentWithoutHydrating(
current,
workInProgress,
renderLanes,
// TODO: The server should serialize the error message so we can log it
// here on the client. Or, in production, a hash/id that corresponds to
// the error.
new Error(
'The server could not finish this Suspense boundary, likely ' +
'due to an error during server rendering. Switched to ' +
'client rendering.',
),
errorMsg
? // eslint-disable-next-line react-internal/prod-error-codes
new Error(errorMsg)
: new Error(
'The server could not finish this Suspense boundary, likely ' +
'due to an error during server rendering. Switched to ' +
'client rendering.',
),
);
}

Expand Down
Loading