Skip to content

Commit 4a3b5eb

Browse files
committed
Implement onError signature for renderToMarkup
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.
1 parent 476f6d0 commit 4a3b5eb

File tree

4 files changed

+161
-5
lines changed

4 files changed

+161
-5
lines changed

packages/react-html/src/ReactHTMLClient.js

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
*/
99

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

1213
import ReactVersion from 'shared/ReactVersion';
1314

@@ -27,6 +28,7 @@ import {
2728
type MarkupOptions = {
2829
identifierPrefix?: string,
2930
signal?: AbortSignal,
31+
onError?: (error: mixed, errorInfo: ErrorInfo) => ?string,
3032
};
3133

3234
export function renderToMarkup(
@@ -49,11 +51,16 @@ export function renderToMarkup(
4951
reject(error);
5052
},
5153
};
52-
function onError(error: mixed) {
54+
function handleError(error: mixed, errorInfo: ErrorInfo) {
5355
// Any error rejects the promise, regardless of where it happened.
5456
// Unlike other React SSR we don't want to put Suspense boundaries into
5557
// client rendering mode because there's no client rendering here.
5658
reject(error);
59+
60+
const onError = options && options.onError;
61+
if (onError) {
62+
onError(error, errorInfo);
63+
}
5764
}
5865
const resumableState = createResumableState(
5966
options ? options.identifierPrefix : undefined,
@@ -72,7 +79,7 @@ export function renderToMarkup(
7279
),
7380
createRootFormatContext(),
7481
Infinity,
75-
onError,
82+
handleError,
7683
undefined,
7784
undefined,
7885
undefined,

packages/react-html/src/ReactHTMLServer.js

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
import type {ReactNodeList} from 'shared/ReactTypes';
1111
import type {LazyComponent} from 'react/src/ReactLazy';
12+
import type {ErrorInfo} from 'react-server/src/ReactFizzServer';
1213

1314
import ReactVersion from 'shared/ReactVersion';
1415

@@ -62,6 +63,7 @@ type ReactMarkupNodeList =
6263
type MarkupOptions = {
6364
identifierPrefix?: string,
6465
signal?: AbortSignal,
66+
onError?: (error: mixed, errorInfo: ErrorInfo) => ?string,
6567
};
6668

6769
function noServerCallOrFormAction() {
@@ -109,17 +111,44 @@ export function renderToMarkup(
109111
reject(error);
110112
},
111113
};
112-
function onError(error: mixed) {
114+
115+
let stashErrorIdx = 1;
116+
const stashedErrors: Map<string, mixed> = new Map();
117+
118+
function handleFlightError(error: mixed): string {
119+
// For Flight errors we don't immediately reject, because they might not matter
120+
// to the output of the HTML. We stash the error with a digest in case we need
121+
// to get to the original error from the Fizz render.
122+
const id = '' + stashErrorIdx++;
123+
stashedErrors.set(id, error);
124+
return id;
125+
}
126+
127+
function handleError(error: mixed, errorInfo: ErrorInfo) {
128+
if (typeof error === 'object' && error !== null) {
129+
const id = error.digest;
130+
// Note that the original error might be `undefined` so we need a has check.
131+
if (typeof id === 'string' && stashedErrors.has(id)) {
132+
// Get the original error thrown inside Flight.
133+
error = stashedErrors.get(id);
134+
}
135+
}
136+
113137
// Any error rejects the promise, regardless of where it happened.
114138
// Unlike other React SSR we don't want to put Suspense boundaries into
115139
// client rendering mode because there's no client rendering here.
116140
reject(error);
141+
142+
const onError = options && options.onError;
143+
if (onError) {
144+
onError(error, errorInfo);
145+
}
117146
}
118147
const flightRequest = createFlightRequest(
119148
// $FlowFixMe: This should be a subtype but not everything is typed covariant.
120149
children,
121150
null,
122-
onError,
151+
handleFlightError,
123152
options ? options.identifierPrefix : undefined,
124153
undefined,
125154
'Markup',
@@ -153,7 +182,7 @@ export function renderToMarkup(
153182
),
154183
createRootFormatContext(),
155184
Infinity,
156-
onError,
185+
handleError,
157186
undefined,
158187
undefined,
159188
undefined,

packages/react-html/src/__tests__/ReactHTMLClient-test.js

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,15 @@
1212
let React;
1313
let ReactHTML;
1414

15+
function normalizeCodeLocInfo(str) {
16+
return (
17+
str &&
18+
String(str).replace(/\n +(?:at|in) ([\S]+)[^\n]*/g, function (m, name) {
19+
return '\n in ' + name + ' (at **)';
20+
})
21+
);
22+
}
23+
1524
if (!__EXPERIMENTAL__) {
1625
it('should not be built in stable', () => {
1726
try {
@@ -170,5 +179,58 @@ if (!__EXPERIMENTAL__) {
170179
const html = await ReactHTML.renderToMarkup(<Component />);
171180
expect(html).toBe('<div>01</div>');
172181
});
182+
183+
it('can get the component owner stacks for onError in dev', async () => {
184+
const thrownError = new Error('hi');
185+
const caughtErrors = [];
186+
187+
function Foo() {
188+
return <Bar />;
189+
}
190+
function Bar() {
191+
return (
192+
<div>
193+
<Baz />
194+
</div>
195+
);
196+
}
197+
function Baz({unused}) {
198+
throw thrownError;
199+
}
200+
201+
await expect(async () => {
202+
await ReactHTML.renderToMarkup(
203+
<div>
204+
<Foo />
205+
</div>,
206+
{
207+
onError(error, errorInfo) {
208+
caughtErrors.push({
209+
error: error,
210+
parentStack: errorInfo.componentStack,
211+
ownerStack: React.captureOwnerStack
212+
? React.captureOwnerStack()
213+
: null,
214+
});
215+
},
216+
},
217+
);
218+
}).rejects.toThrow(thrownError);
219+
220+
expect(caughtErrors.length).toBe(1);
221+
expect(caughtErrors[0].error).toBe(thrownError);
222+
expect(normalizeCodeLocInfo(caughtErrors[0].parentStack)).toBe(
223+
'\n in Baz (at **)' +
224+
'\n in div (at **)' +
225+
'\n in Bar (at **)' +
226+
'\n in Foo (at **)' +
227+
'\n in div (at **)',
228+
);
229+
expect(normalizeCodeLocInfo(caughtErrors[0].ownerStack)).toBe(
230+
__DEV__ && gate(flags => flags.enableOwnerStacks)
231+
? '\n in Bar (at **)' + '\n in Foo (at **)'
232+
: null,
233+
);
234+
});
173235
});
174236
}

packages/react-html/src/__tests__/ReactHTMLServer-test.js

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,15 @@ global.TextEncoder = require('util').TextEncoder;
1515
let React;
1616
let ReactHTML;
1717

18+
function normalizeCodeLocInfo(str) {
19+
return (
20+
str &&
21+
String(str).replace(/\n +(?:at|in) ([\S]+)[^\n]*/g, function (m, name) {
22+
return '\n in ' + name + ' (at **)';
23+
})
24+
);
25+
}
26+
1827
if (!__EXPERIMENTAL__) {
1928
it('should not be built in stable', () => {
2029
try {
@@ -200,5 +209,54 @@ if (!__EXPERIMENTAL__) {
200209
);
201210
expect(html).toBe('<div>00</div>');
202211
});
212+
213+
it('can get the component owner stacks for onError in dev', async () => {
214+
const thrownError = new Error('hi');
215+
const caughtErrors = [];
216+
217+
function Foo() {
218+
return React.createElement(Bar);
219+
}
220+
function Bar() {
221+
return React.createElement('div', null, React.createElement(Baz));
222+
}
223+
function Baz({unused}) {
224+
throw thrownError;
225+
}
226+
227+
await expect(async () => {
228+
await ReactHTML.renderToMarkup(
229+
React.createElement('div', null, React.createElement(Foo)),
230+
{
231+
onError(error, errorInfo) {
232+
caughtErrors.push({
233+
error: error,
234+
parentStack: errorInfo.componentStack,
235+
ownerStack: React.captureOwnerStack
236+
? React.captureOwnerStack()
237+
: null,
238+
});
239+
},
240+
},
241+
);
242+
}).rejects.toThrow(thrownError);
243+
244+
expect(caughtErrors.length).toBe(1);
245+
expect(caughtErrors[0].error).toBe(thrownError);
246+
expect(normalizeCodeLocInfo(caughtErrors[0].parentStack)).toBe(
247+
// TODO: Because Fizz doesn't yet implement debugInfo for parent stacks
248+
// it doesn't have the Server Components in the parent stacks.
249+
'\n in Lazy (at **)' +
250+
'\n in div (at **)' +
251+
'\n in div (at **)',
252+
);
253+
expect(normalizeCodeLocInfo(caughtErrors[0].ownerStack)).toBe(
254+
__DEV__ && gate(flags => flags.enableOwnerStacks)
255+
? // TODO: Because Fizz doesn't yet implement debugInfo for parent stacks
256+
// it doesn't have the Server Components in the parent stacks.
257+
'\n in Lazy (at **)'
258+
: null,
259+
);
260+
});
203261
});
204262
}

0 commit comments

Comments
 (0)