Skip to content

Commit

Permalink
Prod can now send error messages along with a hash, however this is c…
Browse files Browse the repository at this point in the history 
…urrently only used when the server aborts

On the client, the error messages received is used or a generic fallback error message is used. In all environments the hash is attached to the error if found. In Dev, the component stack will be included as well. This gives us more options to inspect the error in onRecoverableError on the client while unifying the code paths in dev/prod
  • Loading branch information
@gnoff
gnoff committed May 18, 2022
1 parent 036afda commit f88fff1
Show file tree
Hide file tree
Showing 6 changed files with 173 additions and 162 deletions.
143 changes: 95 additions & 48 deletions packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
View file
Expand Up @@ -90,24 +90,37 @@ 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(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;
}),
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(errorsArr).toEqual(toBeProdArr);
expect(mappedErrows).toEqual(toBeProdArr);
}
}

Expand Down Expand Up @@ -453,7 +466,7 @@ describe('ReactDOMFizzServer', () => {
// Attempt to hydrate the content.
ReactDOMClient.hydrateRoot(container, <App isClient={true} />, {
onRecoverableError(error) {
errors.push(error.message);
errors.push(error);
},
});
};
Expand Down Expand Up @@ -501,12 +514,18 @@ describe('ReactDOMFizzServer', () => {
expectErrors(
errors,
[
theError.message +
'\nServer Error Hash: ' +
expectedHash +
[
theError.message,
expectedHash,
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,
],
],
[expectedHash],
);

// The client rendered HTML is now in place.
Expand Down Expand Up @@ -590,7 +609,7 @@ describe('ReactDOMFizzServer', () => {
// Attempt to hydrate the content.
ReactDOMClient.hydrateRoot(container, <App isClient={true} />, {
onRecoverableError(error) {
errors.push(error.message);
errors.push(error);
},
});
Scheduler.unstable_flushAll();
Expand All @@ -615,12 +634,18 @@ describe('ReactDOMFizzServer', () => {
expectErrors(
errors,
[
theError.message +
'\nServer Error Hash: ' +
expectedHash +
[
theError.message,
expectedHash,
componentStack(['~lazy-element~', 'Suspense', 'div', 'App']),
],
],
[
[
'The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.',
expectedHash,
],
],
[expectedHash],
);

// The client rendered HTML is now in place.
Expand Down Expand Up @@ -681,7 +706,7 @@ describe('ReactDOMFizzServer', () => {
// Attempt to hydrate the content.
ReactDOMClient.hydrateRoot(container, <App isClient={true} />, {
onRecoverableError(error) {
errors.push(error.message);
errors.push(error);
},
});
Scheduler.unstable_flushAll();
Expand All @@ -691,12 +716,18 @@ describe('ReactDOMFizzServer', () => {
expectErrors(
errors,
[
theError.message +
'\nServer Error Hash: ' +
expectedHash +
[
theError.message,
expectedHash,
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,
],
],
[expectedHash],
);
});

Expand Down Expand Up @@ -744,7 +775,7 @@ describe('ReactDOMFizzServer', () => {
// Attempt to hydrate the content.
ReactDOMClient.hydrateRoot(container, <App isClient={true} />, {
onRecoverableError(error) {
errors.push(error.message);
errors.push(error);
},
});
Scheduler.unstable_flushAll();
Expand All @@ -764,12 +795,18 @@ describe('ReactDOMFizzServer', () => {
expectErrors(
errors,
[
theError.message +
'\nServer Error Hash: ' +
expectedHash +
[
theError.message,
expectedHash,
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,
],
],
[expectedHash],
);

// The client rendered HTML is now in place.
Expand Down Expand Up @@ -1057,7 +1094,7 @@ describe('ReactDOMFizzServer', () => {
// Attempt to hydrate the content.
ReactDOMClient.hydrateRoot(container, <App />, {
onRecoverableError(error) {
errors.push(error.message);
errors.push(error);
},
});
Scheduler.unstable_flushAll();
Expand Down Expand Up @@ -1761,7 +1798,7 @@ describe('ReactDOMFizzServer', () => {
// Attempt to hydrate the content.
ReactDOMClient.hydrateRoot(container, <App isClient={true} />, {
onRecoverableError(error) {
errors.push(error.message);
errors.push(error);
},
});
Scheduler.unstable_flushAll();
Expand Down Expand Up @@ -1795,9 +1832,9 @@ describe('ReactDOMFizzServer', () => {
expectErrors(
errors,
[
theError.message +
'\nServer Error Hash: ' +
expectedHash +
[
theError.message,
expectedHash,
componentStack([
'AsyncText',
'h1',
Expand All @@ -1806,8 +1843,14 @@ describe('ReactDOMFizzServer', () => {
'Suspense',
'App',
]),
],
],
[
[
'The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.',
expectedHash,
],
],
[expectedHash],
);

// The client rendered HTML is now in place.
Expand Down Expand Up @@ -3114,8 +3157,6 @@ describe('ReactDOMFizzServer', () => {
});
//@gate experimental
it('escapes such that attributes cannot be masked', async () => {
window.__outlet = {};

const dangerousErrorString = '" data-msg="bad message" data-foo="';
const theError = new Error(dangerousErrorString);

Expand Down Expand Up @@ -3154,7 +3195,7 @@ describe('ReactDOMFizzServer', () => {
const errors = [];
ReactDOMClient.hydrateRoot(container, <App isClient={true} />, {
onRecoverableError(error) {
errors.push(error.message);
errors.push(error);
},
});
expect(Scheduler).toFlushAndYield([]);
Expand All @@ -3163,12 +3204,18 @@ describe('ReactDOMFizzServer', () => {
expectErrors(
errors,
[
theError.message +
'\nServer Error Hash: ' +
expectedHash +
[
theError.message,
expectedHash,
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,
],
],
[expectedHash],
);
});
});
Expand Down
74 changes: 28 additions & 46 deletions packages/react-dom/src/server/ReactDOMServerFormatConfig.js
View file
Expand Up @@ -1506,10 +1506,10 @@ const endSuspenseBoundary = stringToPrecomputedChunk('<!--/$-->');
const clientRenderedSuspenseBoundaryError1 = stringToPrecomputedChunk(
'<template data-hash="',
);
const clientRenderedSuspenseBoundaryErrorDevA = stringToPrecomputedChunk(
const clientRenderedSuspenseBoundaryError1A = stringToPrecomputedChunk(
'" data-msg="',
);
const clientRenderedSuspenseBoundaryErrorDevB = stringToPrecomputedChunk(
const clientRenderedSuspenseBoundaryError1B = stringToPrecomputedChunk(
'" data-stack="',
);
const clientRenderedSuspenseBoundaryError2 = stringToPrecomputedChunk(
Expand Down Expand Up @@ -1565,17 +1565,19 @@ export function writeStartClientRenderedSuspenseBoundary(
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__) {
// in dev send message and component stack if they exist
if (errorMesssage) {
writeChunk(destination, clientRenderedSuspenseBoundaryErrorDevA);
writeChunk(
destination,
stringToChunk(escapeTextForBrowser(errorMesssage)),
);
}
// Component stacks are currently only captured in dev
if (errorComponentStack) {
writeChunk(destination, clientRenderedSuspenseBoundaryErrorDevB);
writeChunk(destination, clientRenderedSuspenseBoundaryError1B);
writeChunk(
destination,
stringToChunk(escapeTextForBrowser(errorComponentStack)),
Expand Down Expand Up @@ -1924,8 +1926,9 @@ const clientRenderScript1Full = stringToPrecomputedChunk(
clientRenderFunction + ';$RX("',
);
const clientRenderScript1Partial = stringToPrecomputedChunk('$RX("');
const clientRenderScript2 = stringToPrecomputedChunk('")</script>');
const clientRenderErrorScriptArgInterstitial = stringToPrecomputedChunk('","');
const clientRenderScript1A = stringToPrecomputedChunk('"');
const clientRenderScript2 = stringToPrecomputedChunk(')</script>');
const clientRenderErrorScriptArgInterstitial = stringToPrecomputedChunk(',');

export function writeClientRenderBoundaryInstruction(
destination: Destination,
Expand All @@ -1952,21 +1955,20 @@ export function writeClientRenderBoundaryInstruction(
}

writeChunk(destination, boundaryID);
writeChunk(destination, clientRenderScript1A);
if (errorHash || errorMessage || errorComponentStack) {
writeChunk(destination, clientRenderErrorScriptArgInterstitial);
if (errorHash)
writeChunk(
destination,
stringToChunk(escapeJSStringsForInstructionScripts(errorHash)),
);
writeChunk(
destination,
stringToChunk(escapeJSStringsForInstructionScripts(errorHash || '')),
);
}
if (errorMessage || errorComponentStack) {
writeChunk(destination, clientRenderErrorScriptArgInterstitial);
if (errorMessage)
writeChunk(
destination,
stringToChunk(escapeJSStringsForInstructionScripts(errorMessage)),
);
writeChunk(
destination,
stringToChunk(escapeJSStringsForInstructionScripts(errorMessage || '')),
);
}
if (errorComponentStack) {
writeChunk(destination, clientRenderErrorScriptArgInterstitial);
Expand All @@ -1978,28 +1980,14 @@ export function writeClientRenderBoundaryInstruction(
return writeChunkAndReturn(destination, clientRenderScript2);
}

const regexForJSStringsInScripts = /[<\'\`\u2028\u2029]/g;
const regexForJSStringsInScripts = /[<\u2028\u2029]/g;
function escapeJSStringsForInstructionScripts(input: string): string {
if (typeof input !== 'string') {
// eslint-disable-next-line react-internal/prod-error-codes
throw new Error(
`regexForJSStringsInScripts must be passed a string but was passed something of type "${
input === null ? 'null' : typeof input
}" instead`,
);
}
let escaped = JSON.stringify(input);
escaped = escaped.replace(regexForJSStringsInScripts, match => {
const escaped = JSON.stringify(input);
return escaped.replace(regexForJSStringsInScripts, match => {
switch (match) {
// santizing breaking out of strings and script tags
case '<':
return '\\u003c';
case "'":
return "\\'";
// case '"': // JSON.stringify already escaped these
// return '\\"';
case '`':
return '\\`';
case '\u2028':
return '\\u2028';
case '\u2029':
Expand All @@ -2012,10 +2000,4 @@ function escapeJSStringsForInstructionScripts(input: string): string {
}
}
});
if (escaped[0] === '"' && escaped[escaped.length - 1] === '"') {
// This should always be true since JSON.stringify of a string produces a value wrapped in double quotes.
return escaped.slice(1, -1);
} else {
return escaped;
}
}

0 comments on commit f88fff1

Please sign in to comment.