Skip to content

Commit

Permalink
Implement onError signature for renderToMarkup (#30170)
Browse files Browse the repository at this point in the history
Stacked on #30132.

This way we can get parent and owner stacks from the error.

This forces us to confront multiple errors and whether or not a Flight
error that ends up being unobservable needs to really reject the render.

This implements stashing of Flight errors with a digest and only errors
if they end up erroring the Fizz render too. At this point they'll have
parent stacks so we can surface those.
  • Loading branch information
sebmarkbage authored Jul 2, 2024
1 parent e60063d commit 309e146
Show file tree
Hide file tree
Showing 4 changed files with 180 additions and 5 deletions.
11 changes: 9 additions & 2 deletions packages/react-html/src/ReactHTMLClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
*/

import type {ReactNodeList} from 'shared/ReactTypes';
import type {ErrorInfo} from 'react-server/src/ReactFizzServer';

import ReactVersion from 'shared/ReactVersion';

Expand All @@ -27,6 +28,7 @@ import {
type MarkupOptions = {
identifierPrefix?: string,
signal?: AbortSignal,
onError?: (error: mixed, errorInfo: ErrorInfo) => ?string,
};

export function renderToMarkup(
Expand All @@ -49,11 +51,16 @@ export function renderToMarkup(
reject(error);
},
};
function onError(error: mixed) {
function handleError(error: mixed, errorInfo: ErrorInfo) {
// Any error rejects the promise, regardless of where it happened.
// Unlike other React SSR we don't want to put Suspense boundaries into
// client rendering mode because there's no client rendering here.
reject(error);

const onError = options && options.onError;
if (onError) {
onError(error, errorInfo);
}
}
const resumableState = createResumableState(
options ? options.identifierPrefix : undefined,
Expand All @@ -72,7 +79,7 @@ export function renderToMarkup(
),
createRootFormatContext(),
Infinity,
onError,
handleError,
undefined,
undefined,
undefined,
Expand Down
54 changes: 51 additions & 3 deletions packages/react-html/src/ReactHTMLServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,13 @@

import type {ReactNodeList} from 'shared/ReactTypes';
import type {LazyComponent} from 'react/src/ReactLazy';
import type {ErrorInfo} from 'react-server/src/ReactFizzServer';

import ReactVersion from 'shared/ReactVersion';

import ReactSharedInternalsServer from 'react-server/src/ReactSharedInternalsServer';
import ReactSharedInternalsClient from 'shared/ReactSharedInternals';

import {
createRequest as createFlightRequest,
startWork as startFlightWork,
Expand Down Expand Up @@ -62,6 +66,7 @@ type ReactMarkupNodeList =
type MarkupOptions = {
identifierPrefix?: string,
signal?: AbortSignal,
onError?: (error: mixed, errorInfo: ErrorInfo) => ?string,
};

function noServerCallOrFormAction() {
Expand Down Expand Up @@ -109,17 +114,60 @@ export function renderToMarkup(
reject(error);
},
};
function onError(error: mixed) {

let stashErrorIdx = 1;
const stashedErrors: Map<string, mixed> = new Map();

function handleFlightError(error: mixed): string {
// For Flight errors we don't immediately reject, because they might not matter
// to the output of the HTML. We stash the error with a digest in case we need
// to get to the original error from the Fizz render.
const id = '' + stashErrorIdx++;
stashedErrors.set(id, error);
return id;
}

function handleError(error: mixed, errorInfo: ErrorInfo) {
if (typeof error === 'object' && error !== null) {
const id = error.digest;
// Note that the original error might be `undefined` so we need a has check.
if (typeof id === 'string' && stashedErrors.has(id)) {
// Get the original error thrown inside Flight.
error = stashedErrors.get(id);
}
}

// Any error rejects the promise, regardless of where it happened.
// Unlike other React SSR we don't want to put Suspense boundaries into
// client rendering mode because there's no client rendering here.
reject(error);

const onError = options && options.onError;
if (onError) {
if (__DEV__) {
const prevGetCurrentStackImpl =
ReactSharedInternalsServer.getCurrentStack;
// We're inside a "client" callback from Fizz but we only have access to the
// "server" runtime so to get access to a stack trace within this callback we
// need to override it to get it from the client runtime.
ReactSharedInternalsServer.getCurrentStack =
ReactSharedInternalsClient.getCurrentStack;
try {
onError(error, errorInfo);
} finally {
ReactSharedInternalsServer.getCurrentStack =
prevGetCurrentStackImpl;
}
} else {
onError(error, errorInfo);
}
}
}
const flightRequest = createFlightRequest(
// $FlowFixMe: This should be a subtype but not everything is typed covariant.
children,
null,
onError,
handleFlightError,
options ? options.identifierPrefix : undefined,
undefined,
'Markup',
Expand Down Expand Up @@ -153,7 +201,7 @@ export function renderToMarkup(
),
createRootFormatContext(),
Infinity,
onError,
handleError,
undefined,
undefined,
undefined,
Expand Down
62 changes: 62 additions & 0 deletions packages/react-html/src/__tests__/ReactHTMLClient-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,15 @@
let React;
let ReactHTML;

function normalizeCodeLocInfo(str) {
return (
str &&
String(str).replace(/\n +(?:at|in) ([\S]+)[^\n]*/g, function (m, name) {
return '\n in ' + name + ' (at **)';
})
);
}

if (!__EXPERIMENTAL__) {
it('should not be built in stable', () => {
try {
Expand Down Expand Up @@ -170,5 +179,58 @@ if (!__EXPERIMENTAL__) {
const html = await ReactHTML.renderToMarkup(<Component />);
expect(html).toBe('<div>01</div>');
});

it('can get the component owner stacks for onError in dev', async () => {
const thrownError = new Error('hi');
const caughtErrors = [];

function Foo() {
return <Bar />;
}
function Bar() {
return (
<div>
<Baz />
</div>
);
}
function Baz({unused}) {
throw thrownError;
}

await expect(async () => {
await ReactHTML.renderToMarkup(
<div>
<Foo />
</div>,
{
onError(error, errorInfo) {
caughtErrors.push({
error: error,
parentStack: errorInfo.componentStack,
ownerStack: React.captureOwnerStack
? React.captureOwnerStack()
: null,
});
},
},
);
}).rejects.toThrow(thrownError);

expect(caughtErrors.length).toBe(1);
expect(caughtErrors[0].error).toBe(thrownError);
expect(normalizeCodeLocInfo(caughtErrors[0].parentStack)).toBe(
'\n in Baz (at **)' +
'\n in div (at **)' +
'\n in Bar (at **)' +
'\n in Foo (at **)' +
'\n in div (at **)',
);
expect(normalizeCodeLocInfo(caughtErrors[0].ownerStack)).toBe(
__DEV__ && gate(flags => flags.enableOwnerStacks)
? '\n in Bar (at **)' + '\n in Foo (at **)'
: null,
);
});
});
}
58 changes: 58 additions & 0 deletions packages/react-html/src/__tests__/ReactHTMLServer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,15 @@ global.TextEncoder = require('util').TextEncoder;
let React;
let ReactHTML;

function normalizeCodeLocInfo(str) {
return (
str &&
String(str).replace(/\n +(?:at|in) ([\S]+)[^\n]*/g, function (m, name) {
return '\n in ' + name + ' (at **)';
})
);
}

if (!__EXPERIMENTAL__) {
it('should not be built in stable', () => {
try {
Expand Down Expand Up @@ -200,5 +209,54 @@ if (!__EXPERIMENTAL__) {
);
expect(html).toBe('<div>00</div>');
});

it('can get the component owner stacks for onError in dev', async () => {
const thrownError = new Error('hi');
const caughtErrors = [];

function Foo() {
return React.createElement(Bar);
}
function Bar() {
return React.createElement('div', null, React.createElement(Baz));
}
function Baz({unused}) {
throw thrownError;
}

await expect(async () => {
await ReactHTML.renderToMarkup(
React.createElement('div', null, React.createElement(Foo)),
{
onError(error, errorInfo) {
caughtErrors.push({
error: error,
parentStack: errorInfo.componentStack,
ownerStack: React.captureOwnerStack
? React.captureOwnerStack()
: null,
});
},
},
);
}).rejects.toThrow(thrownError);

expect(caughtErrors.length).toBe(1);
expect(caughtErrors[0].error).toBe(thrownError);
expect(normalizeCodeLocInfo(caughtErrors[0].parentStack)).toBe(
// TODO: Because Fizz doesn't yet implement debugInfo for parent stacks
// it doesn't have the Server Components in the parent stacks.
'\n in Lazy (at **)' +
'\n in div (at **)' +
'\n in div (at **)',
);
expect(normalizeCodeLocInfo(caughtErrors[0].ownerStack)).toBe(
__DEV__ && gate(flags => flags.enableOwnerStacks)
? // TODO: Because Fizz doesn't yet implement debugInfo for parent stacks
// it doesn't have the Server Components in the parent stacks.
'\n in Lazy (at **)'
: null,
);
});
});
}

0 comments on commit 309e146

Please sign in to comment.