Move error logging to update callback (#21737)
* Move error logging to update callback This prevents double logging for gDSFE boundaries with createRoot. * Add an explanation for the rest of duplicates
Dan Abramov committed
Jun 24, 2021 at 20:57 UTC
534c9c52ec3e33ed241f7a20e264bb4571d452b3
3 files changed
+23
-33
packages/react-dom/src/__tests__/ReactDOMConsoleErrorReporting-test.js
+9
-17
@@ -166,7 +166,8 @@ describe('ReactDOMConsoleErrorReporting', () => {
166
}),
167
],
168
[
169
- // TODO: This is duplicated only with createRoot. Why?
169
+ // This is only duplicated with createRoot
170
+ // because it retries once with a sync render.
171
expect.objectContaining({
172
message: 'Boom',
173
}),
@@ -181,7 +182,8 @@ describe('ReactDOMConsoleErrorReporting', () => {
182
}),
183
],
184
[
184
- // TODO: This is duplicated only with createRoot. Why?
185
+ // This is only duplicated with createRoot
186
+ // because it retries once with a sync render.
187
expect.stringContaining('Error: Uncaught [Error: Boom]'),
188
expect.objectContaining({
189
message: 'Boom',
@@ -246,7 +248,8 @@ describe('ReactDOMConsoleErrorReporting', () => {
248
}),
249
],
250
[
249
- // TODO: This is duplicated only with createRoot. Why?
251
+ // This is only duplicated with createRoot
252
+ // because it retries once with a sync render.
253
expect.objectContaining({
254
message: 'Boom',
255
}),
@@ -261,20 +264,15 @@ describe('ReactDOMConsoleErrorReporting', () => {
264
}),
265
],
266
[
264
- // Addendum by React:
265
- expect.stringContaining(
266
- 'The above error occurred in the <Foo> component',
267
- ),
268
- ],
269
- [
270
- // TODO: This is duplicated only with createRoot. Why?
267
+ // This is only duplicated with createRoot
268
+ // because it retries once with a sync render.
269
expect.stringContaining('Error: Uncaught [Error: Boom]'),
270
expect.objectContaining({
271
message: 'Boom',
272
}),
273
],
274
[
277
- // TODO: This is duplicated only with createRoot. Why?
275
+ // Addendum by React:
276
expect.stringContaining(
277
'The above error occurred in the <Foo> component',
278
),
@@ -291,12 +289,6 @@ describe('ReactDOMConsoleErrorReporting', () => {
289
message: 'Boom',
290
}),
291
],
294
- [
295
- // TODO: This is duplicated only with createRoot. Why?
296
- expect.objectContaining({
297
- message: 'Boom',
298
- }),
299
- ],
292
]);
293
}
294
packages/react-reconciler/src/ReactFiberThrow.new.js
+7
-8
@@ -108,9 +108,14 @@ function createClassErrorUpdate(
108
if (typeof getDerivedStateFromError === 'function') {
109
const error = errorInfo.value;
110
update.payload = () => {
111
- logCapturedError(fiber, errorInfo);
111
return getDerivedStateFromError(error);
112
};
113
+ update.callback = () => {
114
+ if (__DEV__) {
115
+ markFailedErrorBoundaryForHotReloading(fiber);
116
+ }
117
+ logCapturedError(fiber, errorInfo);
118
+ };
119
}
120
121
const inst = fiber.stateNode;
@@ -119,6 +124,7 @@ function createClassErrorUpdate(
124
if (__DEV__) {
125
markFailedErrorBoundaryForHotReloading(fiber);
126
}
127
+ logCapturedError(fiber, errorInfo);
128
if (typeof getDerivedStateFromError !== 'function') {
129
// To preserve the preexisting retry behavior of error boundaries,
130
// we keep track of which ones already failed during this batch.
@@ -126,9 +132,6 @@ function createClassErrorUpdate(
132
// TODO: Warn in strict mode if getDerivedStateFromError is
133
// not defined.
134
markLegacyErrorBoundaryAsFailed(this);
129
-
130
- // Only log here if componentDidCatch is the only error boundary method defined
131
- logCapturedError(fiber, errorInfo);
135
}
136
const error = errorInfo.value;
137
const stack = errorInfo.stack;
@@ -150,10 +153,6 @@ function createClassErrorUpdate(
153
}
154
}
155
};
153
- } else if (__DEV__) {
154
- update.callback = () => {
155
- markFailedErrorBoundaryForHotReloading(fiber);
156
- };
156
}
157
return update;
158
}
packages/react-reconciler/src/ReactFiberThrow.old.js
+7
-8
@@ -108,9 +108,14 @@ function createClassErrorUpdate(
108
if (typeof getDerivedStateFromError === 'function') {
109
const error = errorInfo.value;
110
update.payload = () => {
111
- logCapturedError(fiber, errorInfo);
111
return getDerivedStateFromError(error);
112
};
113
+ update.callback = () => {
114
+ if (__DEV__) {
115
+ markFailedErrorBoundaryForHotReloading(fiber);
116
+ }
117
+ logCapturedError(fiber, errorInfo);
118
+ };
119
}
120
121
const inst = fiber.stateNode;
@@ -119,6 +124,7 @@ function createClassErrorUpdate(
124
if (__DEV__) {
125
markFailedErrorBoundaryForHotReloading(fiber);
126
}
127
+ logCapturedError(fiber, errorInfo);
128
if (typeof getDerivedStateFromError !== 'function') {
129
// To preserve the preexisting retry behavior of error boundaries,
130
// we keep track of which ones already failed during this batch.
@@ -126,9 +132,6 @@ function createClassErrorUpdate(
132
// TODO: Warn in strict mode if getDerivedStateFromError is
133
// not defined.
134
markLegacyErrorBoundaryAsFailed(this);
129
-
130
- // Only log here if componentDidCatch is the only error boundary method defined
131
- logCapturedError(fiber, errorInfo);
135
}
136
const error = errorInfo.value;
137
const stack = errorInfo.stack;
@@ -150,10 +153,6 @@ function createClassErrorUpdate(
153
}
154
}
155
};
153
- } else if (__DEV__) {
154
- update.callback = () => {
155
- markFailedErrorBoundaryForHotReloading(fiber);
156
- };
156
}
157
return update;
158
}