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

Implement onError signature for renderToMarkup #30170

Merged
merged 3 commits into from
Jul 2, 2024
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
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,
);
});
});
}
Loading