Implement onError signature for renderToMarkup (#30170)
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.
Sebastian Markbåge committed
Jul 2, 2024 at 16:31 UTC
309e146193c7b84d1c7a60d1a2ab2d6c836ba515
4 files changed
+180
-5
packages/react-html/src/ReactHTMLClient.js
+9
-2
@@ -8,6 +8,7 @@
8
*/
9
10
import type {ReactNodeList} from 'shared/ReactTypes';
11
+import type {ErrorInfo} from 'react-server/src/ReactFizzServer';
12
13
import ReactVersion from 'shared/ReactVersion';
14
@@ -27,6 +28,7 @@ import {
28
type MarkupOptions = {
29
identifierPrefix?: string,
30
signal?: AbortSignal,
31
+ onError?: (error: mixed, errorInfo: ErrorInfo) => ?string,
32
};
33
34
export function renderToMarkup(
@@ -49,11 +51,16 @@ export function renderToMarkup(
51
reject(error);
52
},
53
};
52
- function onError(error: mixed) {
54
+ function handleError(error: mixed, errorInfo: ErrorInfo) {
55
// Any error rejects the promise, regardless of where it happened.
56
// Unlike other React SSR we don't want to put Suspense boundaries into
57
// client rendering mode because there's no client rendering here.
58
reject(error);
59
+
60
+ const onError = options && options.onError;
61
+ if (onError) {
62
+ onError(error, errorInfo);
63
+ }
64
}
65
const resumableState = createResumableState(
66
options ? options.identifierPrefix : undefined,
@@ -72,7 +79,7 @@ export function renderToMarkup(
79
),
80
createRootFormatContext(),
81
Infinity,
75
- onError,
82
+ handleError,
83
undefined,
84
undefined,
85
undefined,
packages/react-html/src/ReactHTMLServer.js
+51
-3
@@ -9,9 +9,13 @@
9
10
import type {ReactNodeList} from 'shared/ReactTypes';
11
import type {LazyComponent} from 'react/src/ReactLazy';
12
+import type {ErrorInfo} from 'react-server/src/ReactFizzServer';
13
14
import ReactVersion from 'shared/ReactVersion';
15
16
+import ReactSharedInternalsServer from 'react-server/src/ReactSharedInternalsServer';
17
+import ReactSharedInternalsClient from 'shared/ReactSharedInternals';
18
+
19
import {
20
createRequest as createFlightRequest,
21
startWork as startFlightWork,
@@ -62,6 +66,7 @@ type ReactMarkupNodeList =
66
type MarkupOptions = {
67
identifierPrefix?: string,
68
signal?: AbortSignal,
69
+ onError?: (error: mixed, errorInfo: ErrorInfo) => ?string,
70
};
71
72
function noServerCallOrFormAction() {
@@ -109,17 +114,60 @@ export function renderToMarkup(
114
reject(error);
115
},
116
};
112
- function onError(error: mixed) {
117
+
118
+ let stashErrorIdx = 1;
119
+ const stashedErrors: Map<string, mixed> = new Map();
120
+
121
+ function handleFlightError(error: mixed): string {
122
+ // For Flight errors we don't immediately reject, because they might not matter
123
+ // to the output of the HTML. We stash the error with a digest in case we need
124
+ // to get to the original error from the Fizz render.
125
+ const id = '' + stashErrorIdx++;
126
+ stashedErrors.set(id, error);
127
+ return id;
128
+ }
129
+
130
+ function handleError(error: mixed, errorInfo: ErrorInfo) {
131
+ if (typeof error === 'object' && error !== null) {
132
+ const id = error.digest;
133
+ // Note that the original error might be `undefined` so we need a has check.
134
+ if (typeof id === 'string' && stashedErrors.has(id)) {
135
+ // Get the original error thrown inside Flight.
136
+ error = stashedErrors.get(id);
137
+ }
138
+ }
139
+
140
// Any error rejects the promise, regardless of where it happened.
141
// Unlike other React SSR we don't want to put Suspense boundaries into
142
// client rendering mode because there's no client rendering here.
143
reject(error);
144
+
145
+ const onError = options && options.onError;
146
+ if (onError) {
147
+ if (__DEV__) {
148
+ const prevGetCurrentStackImpl =
149
+ ReactSharedInternalsServer.getCurrentStack;
150
+ // We're inside a "client" callback from Fizz but we only have access to the
151
+ // "server" runtime so to get access to a stack trace within this callback we
152
+ // need to override it to get it from the client runtime.
153
+ ReactSharedInternalsServer.getCurrentStack =
154
+ ReactSharedInternalsClient.getCurrentStack;
155
+ try {
156
+ onError(error, errorInfo);
157
+ } finally {
158
+ ReactSharedInternalsServer.getCurrentStack =
159
+ prevGetCurrentStackImpl;
160
+ }
161
+ } else {
162
+ onError(error, errorInfo);
163
+ }
164
+ }
165
}
166
const flightRequest = createFlightRequest(
167
// $FlowFixMe: This should be a subtype but not everything is typed covariant.
168
children,
169
null,
122
- onError,
170
+ handleFlightError,
171
options ? options.identifierPrefix : undefined,
172
undefined,
173
'Markup',
@@ -153,7 +201,7 @@ export function renderToMarkup(
201
),
202
createRootFormatContext(),
203
Infinity,
156
- onError,
204
+ handleError,
205
undefined,
206
undefined,
207
undefined,
packages/react-html/src/__tests__/ReactHTMLClient-test.js
+62
@@ -12,6 +12,15 @@
12
let React;
13
let ReactHTML;
14
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
+
24
if (!__EXPERIMENTAL__) {
25
it('should not be built in stable', () => {
26
try {
@@ -170,5 +179,58 @@ if (!__EXPERIMENTAL__) {
179
const html = await ReactHTML.renderToMarkup(<Component />);
180
expect(html).toBe('<div>01</div>');
181
});
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
+ });
235
});
236
}
packages/react-html/src/__tests__/ReactHTMLServer-test.js
+58
@@ -15,6 +15,15 @@ global.TextEncoder = require('util').TextEncoder;
15
let React;
16
let ReactHTML;
17
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
+
27
if (!__EXPERIMENTAL__) {
28
it('should not be built in stable', () => {
29
try {
@@ -200,5 +209,54 @@ if (!__EXPERIMENTAL__) {
209
);
210
expect(html).toBe('<div>00</div>');
211
});
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
+ });
261
});
262
}