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 all commits
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
457 changes: 418 additions & 39 deletions packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -193,20 +193,30 @@ describe('ReactDOMFizzServer', () => {

// @gate experimental
it('should be able to complete by aborting even if the promise never resolves', async () => {
const errors = [];
const controller = new AbortController();
const stream = await ReactDOMFizzServer.renderToReadableStream(
<div>
<Suspense fallback={<div>Loading</div>}>
<InfiniteSuspend />
</Suspense>
</div>,
{signal: controller.signal},
{
signal: controller.signal,
onError(x) {
errors.push(x.message);
},
},
);

controller.abort();

const result = await readResult(stream);
expect(result).toContain('Loading');

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

// @gate experimental
Expand All @@ -223,12 +233,18 @@ describe('ReactDOMFizzServer', () => {
rendered = true;
return 'Done';
}
const errors = [];
const stream = await ReactDOMFizzServer.renderToReadableStream(
<div>
<Suspense fallback={<div>Loading</div>}>
<Wait /> />
</Suspense>
</div>,
{
onError(x) {
errors.push(x.message);
},
},
);

stream.allReady.then(() => (isComplete = true));
Expand All @@ -239,6 +255,10 @@ describe('ReactDOMFizzServer', () => {
const reader = stream.getReader();
reader.cancel();

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

hasLoaded = true;
resolve();

Expand Down
30 changes: 28 additions & 2 deletions packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ describe('ReactDOMFizzServer', () => {

{
onError(x) {
reportedErrors.push(x);
reportedErrors.push(x.message);
},
onShellError(x) {
reportedShellErrors.push(x);
Expand All @@ -224,7 +224,10 @@ describe('ReactDOMFizzServer', () => {

expect(output.error).toBe(theError);
expect(output.result).toBe('');
expect(reportedErrors).toEqual([theError]);
expect(reportedErrors).toEqual([
theError.message,
'This Suspense boundary was aborted by the server',
]);
expect(reportedShellErrors).toEqual([theError]);
});

Expand Down Expand Up @@ -289,6 +292,7 @@ describe('ReactDOMFizzServer', () => {
// @gate experimental
it('should be able to complete by aborting even if the promise never resolves', async () => {
let isCompleteCalls = 0;
const errors = [];
const {writable, output, completed} = getTestWritable();
const {pipe, abort} = ReactDOMFizzServer.renderToPipeableStream(
<div>
Expand All @@ -298,6 +302,9 @@ describe('ReactDOMFizzServer', () => {
</div>,

{
onError(x) {
errors.push(x.message);
},
onAllReady() {
isCompleteCalls++;
},
Expand All @@ -314,6 +321,9 @@ describe('ReactDOMFizzServer', () => {

await completed;

expect(errors).toEqual([
'This Suspense boundary was aborted by the server',
]);
expect(output.error).toBe(undefined);
expect(output.result).toContain('Loading');
expect(isCompleteCalls).toBe(1);
Expand All @@ -322,6 +332,7 @@ describe('ReactDOMFizzServer', () => {
// @gate experimental
it('should be able to complete by abort when the fallback is also suspended', async () => {
let isCompleteCalls = 0;
const errors = [];
const {writable, output, completed} = getTestWritable();
const {pipe, abort} = ReactDOMFizzServer.renderToPipeableStream(
<div>
Expand All @@ -333,6 +344,9 @@ describe('ReactDOMFizzServer', () => {
</div>,

{
onError(x) {
errors.push(x.message);
},
onAllReady() {
isCompleteCalls++;
},
Expand All @@ -349,6 +363,11 @@ describe('ReactDOMFizzServer', () => {

await completed;

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',
]);
expect(output.error).toBe(undefined);
expect(output.result).toContain('Loading');
expect(isCompleteCalls).toBe(1);
Expand Down Expand Up @@ -552,6 +571,7 @@ describe('ReactDOMFizzServer', () => {
rendered = true;
return 'Done';
}
const errors = [];
const {writable, completed} = getTestWritable();
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(
<div>
Expand All @@ -560,6 +580,9 @@ describe('ReactDOMFizzServer', () => {
</Suspense>
</div>,
{
onError(x) {
errors.push(x.message);
},
onAllReady() {
isComplete = true;
},
Expand All @@ -579,6 +602,9 @@ describe('ReactDOMFizzServer', () => {

await completed;

expect(errors).toEqual([
'This Suspense boundary was aborted by the server',
]);
expect(rendered).toBe(false);
expect(isComplete).toBe(true);
});
Expand Down
24 changes: 24 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,30 @@ export function isSuspenseInstancePending(instance: SuspenseInstance) {
export function isSuspenseInstanceFallback(instance: SuspenseInstance) {
return instance.data === SUSPENSE_FALLBACK_START_DATA;
}
export function getSuspenseInstanceFallbackErrorDetails(
instance: SuspenseInstance,
) {
const nextSibling = instance.nextSibling;
let errorMessage /*, errorComponentStack, errorHash*/;
if (
nextSibling &&
nextSibling.nodeType === ELEMENT_NODE &&
nextSibling.nodeName.toLowerCase() === 'template'
) {
const msg = ((nextSibling: any): HTMLTemplateElement).dataset.msg;
if (msg !== null) errorMessage = msg;

// @TODO read and return hash and componentStack once we know how we are goign to
// expose this extra errorInfo to onRecoverableError

// const hash = ((nextSibling: any): HTMLTemplateElement).dataset.hash;
// if (hash !== null) errorHash = hash;

// const stack = ((nextSibling: any): HTMLTemplateElement).dataset.stack;
// if (stack !== null) errorComponentStack = stack;
}
return {errorMessage /*, errorComponentStack, errorHash*/};
}

export function registerSuspenseInstanceRetry(
instance: SuspenseInstance,
Expand Down
2 changes: 1 addition & 1 deletion packages/react-dom/src/server/ReactDOMFizzServerBrowser.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ type Options = {|
bootstrapModules?: Array<string>,
progressiveChunkSize?: number,
signal?: AbortSignal,
onError?: (error: mixed) => void,
onError?: (error: mixed) => ?string,
|};

// TODO: Move to sub-classing ReadableStream.
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
110 changes: 106 additions & 4 deletions packages/react-dom/src/server/ReactDOMServerFormatConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -1526,6 +1526,19 @@ const startClientRenderedSuspenseBoundary = stringToPrecomputedChunk(
);
const endSuspenseBoundary = stringToPrecomputedChunk('<!--/$-->');

const clientRenderedSuspenseBoundaryError1 = stringToPrecomputedChunk(
'<template data-hash="',
);
const clientRenderedSuspenseBoundaryError1A = stringToPrecomputedChunk(
'" data-msg="',
);
const clientRenderedSuspenseBoundaryError1B = stringToPrecomputedChunk(
'" data-stack="',
);
const clientRenderedSuspenseBoundaryError2 = stringToPrecomputedChunk(
'"></template>',
);

export function pushStartCompletedSuspenseBoundary(
target: Array<Chunk | PrecomputedChunk>,
) {
Expand Down Expand Up @@ -1563,8 +1576,43 @@ export function writeStartPendingSuspenseBoundary(
export function writeStartClientRenderedSuspenseBoundary(
destination: Destination,
responseState: ResponseState,
errorHash: ?string,
errorMesssage: ?string,
errorComponentStack: ?string,
): boolean {
return writeChunkAndReturn(destination, startClientRenderedSuspenseBoundary);
let result;
result = writeChunkAndReturn(
destination,
startClientRenderedSuspenseBoundary,
);
if (errorHash) {
writeChunk(destination, clientRenderedSuspenseBoundaryError1);
writeChunk(destination, stringToChunk(escapeTextForBrowser(errorHash)));
// In prod errorMessage will usually be nullish but there is one case where
// it is used (currently when the server aborts the task) so we leave it ungated.
if (errorMesssage) {
writeChunk(destination, clientRenderedSuspenseBoundaryError1A);
writeChunk(
destination,
stringToChunk(escapeTextForBrowser(errorMesssage)),
);
}
if (__DEV__) {
// Component stacks are currently only captured in dev
if (errorComponentStack) {
writeChunk(destination, clientRenderedSuspenseBoundaryError1B);
writeChunk(
destination,
stringToChunk(escapeTextForBrowser(errorComponentStack)),
);
}
}
result = writeChunkAndReturn(
destination,
clientRenderedSuspenseBoundaryError2,
);
}
return result;
}
export function writeEndCompletedSuspenseBoundary(
destination: Destination,
Expand Down Expand Up @@ -1724,7 +1772,7 @@ export function writeEndSegment(
// const SUSPENSE_PENDING_START_DATA = '$?';
// const SUSPENSE_FALLBACK_START_DATA = '$!';
//
// function clientRenderBoundary(suspenseBoundaryID) {
// function clientRenderBoundary(suspenseBoundaryID, errorHash, errorMsg, errorComponentStack) {
// // Find the fallback's first element.
// const suspenseIdNode = document.getElementById(suspenseBoundaryID);
// if (!suspenseIdNode) {
Expand All @@ -1736,6 +1784,11 @@ export function writeEndSegment(
// const suspenseNode = suspenseIdNode.previousSibling;
// // Tag it to be client rendered.
// suspenseNode.data = SUSPENSE_FALLBACK_START_DATA;
// // assign error metadata to first sibling
// let dataset = suspenseIdNode.dataset;
// if (errorHash) dataset.hash = errorHash;
// if (errorMsg) dataset.msg = errorMsg;
// if (errorComponentStack) dataset.stack = errorComponentStack;
// // Tell React to retry it if the parent already hydrated.
// if (suspenseNode._reactRetry) {
// suspenseNode._reactRetry();
Expand Down Expand Up @@ -1823,7 +1876,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(b,c,d,e){var a=document.getElementById(b);a&&(b=a.previousSibling,b.data="$!",a=a.dataset,c&&(a.hash=c),d&&(a.msg=d),e&&(a.stack=e),b._reactRetry&&b._reactRetry())}';

const completeSegmentScript1Full = stringToPrecomputedChunk(
completeSegmentFunction + ';$RS("',
Expand Down Expand Up @@ -1896,12 +1949,17 @@ const clientRenderScript1Full = stringToPrecomputedChunk(
clientRenderFunction + ';$RX("',
);
const clientRenderScript1Partial = stringToPrecomputedChunk('$RX("');
const clientRenderScript2 = stringToPrecomputedChunk('")</script>');
const clientRenderScript1A = stringToPrecomputedChunk('"');
const clientRenderScript2 = stringToPrecomputedChunk(')</script>');
const clientRenderErrorScriptArgInterstitial = stringToPrecomputedChunk(',');

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

writeChunk(destination, boundaryID);
writeChunk(destination, clientRenderScript1A);
if (errorHash || errorMessage || errorComponentStack) {
writeChunk(destination, clientRenderErrorScriptArgInterstitial);
writeChunk(
destination,
stringToChunk(escapeJSStringsForInstructionScripts(errorHash || '')),
);
}
if (errorMessage || errorComponentStack) {
writeChunk(destination, clientRenderErrorScriptArgInterstitial);
writeChunk(
destination,
stringToChunk(escapeJSStringsForInstructionScripts(errorMessage || '')),
);
}
if (errorComponentStack) {
writeChunk(destination, clientRenderErrorScriptArgInterstitial);
writeChunk(
destination,
stringToChunk(escapeJSStringsForInstructionScripts(errorComponentStack)),
);
}
return writeChunkAndReturn(destination, clientRenderScript2);
}

const regexForJSStringsInScripts = /[<\u2028\u2029]/g;
function escapeJSStringsForInstructionScripts(input: string): string {
const escaped = JSON.stringify(input);
return escaped.replace(regexForJSStringsInScripts, match => {
switch (match) {
// santizing breaking out of strings and script tags
case '<':
return '\\u003c';
case '\u2028':
return '\\u2028';
case '\u2029':
return '\\u2029';
default: {
// eslint-disable-next-line react-internal/prod-error-codes
throw new Error(
'escapeJSStringsForInstructionScripts encountered a match it does not know how to replace. this means the match regex and the replacement characters are no longer in sync. This is a bug in React',
);
}
}
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,10 @@ export function writeStartCompletedSuspenseBoundary(
export function writeStartClientRenderedSuspenseBoundary(
destination: Destination,
responseState: ResponseState,
// flushing these error arguments are not currently supported in this legacy streaming format.
errorHash: ?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 Down
Loading