Scheduling Profiler marks should include thrown Errors (#22417)
The scheduling profiler markComponentRenderStopped method is supposed to be called when rendering finishes or when a value is thrown (Suspense or Error). Previously we were calling this in a Suspense-only path of `throwException`. This PR updates the code to handle errors (or non-Thenables) thrown as well. It also moves the mark logic the work loop `handleError` method, with Suspense/Error agnostic cleanup.
Brian Vaughn committed
Sep 24, 2021 at 12:32 UTC
af87f5a83d49bde495eb7fe760b2052ba8a76caf
6 files changed
+218
-20
packages/react-reconciler/src/ReactFiberThrow.new.js
-10
@@ -41,7 +41,6 @@ import {shouldCaptureSuspense} from './ReactFiberSuspenseComponent.new';
41
import {NoMode, ConcurrentMode, DebugTracingMode} from './ReactTypeOfMode';
42
import {
43
enableDebugTracing,
44
- enableSchedulingProfiler,
44
enableLazyContextPropagation,
45
enableUpdaterTracking,
46
enablePersistentOffscreenHostContainer,
@@ -71,10 +70,6 @@ import {
70
import {propagateParentContextChangesToDeferredTree} from './ReactFiberNewContext.new';
71
import {logCapturedError} from './ReactFiberErrorLogger';
72
import {logComponentSuspended} from './DebugTracing';
74
-import {
75
- markComponentRenderStopped,
76
- markComponentSuspended,
77
-} from './SchedulingProfiler';
73
import {isDevToolsPresent} from './ReactFiberDevToolsHook.new';
74
import {
75
SyncLane,
@@ -247,11 +242,6 @@ function throwException(
242
}
243
}
244
250
- if (enableSchedulingProfiler) {
251
- markComponentRenderStopped();
252
- markComponentSuspended(sourceFiber, wakeable, rootRenderLanes);
253
- }
254
-
245
// Reset the memoizedState to what it was before we attempted to render it.
246
// A legacy mode Suspense quirk, only relevant to hook components.
247
const tag = sourceFiber.tag;
packages/react-reconciler/src/ReactFiberThrow.old.js
-10
@@ -41,7 +41,6 @@ import {shouldCaptureSuspense} from './ReactFiberSuspenseComponent.old';
41
import {NoMode, ConcurrentMode, DebugTracingMode} from './ReactTypeOfMode';
42
import {
43
enableDebugTracing,
44
- enableSchedulingProfiler,
44
enableLazyContextPropagation,
45
enableUpdaterTracking,
46
enablePersistentOffscreenHostContainer,
@@ -71,10 +70,6 @@ import {
70
import {propagateParentContextChangesToDeferredTree} from './ReactFiberNewContext.old';
71
import {logCapturedError} from './ReactFiberErrorLogger';
72
import {logComponentSuspended} from './DebugTracing';
74
-import {
75
- markComponentRenderStopped,
76
- markComponentSuspended,
77
-} from './SchedulingProfiler';
73
import {isDevToolsPresent} from './ReactFiberDevToolsHook.old';
74
import {
75
SyncLane,
@@ -247,11 +242,6 @@ function throwException(
242
}
243
}
244
250
- if (enableSchedulingProfiler) {
251
- markComponentRenderStopped();
252
- markComponentSuspended(sourceFiber, wakeable, rootRenderLanes);
253
- }
254
-
245
// Reset the memoizedState to what it was before we attempted to render it.
246
// A legacy mode Suspense quirk, only relevant to hook components.
247
const tag = sourceFiber.tag;
packages/react-reconciler/src/ReactFiberWorkLoop.new.js
+26
@@ -68,6 +68,9 @@ import {
68
import {
69
markCommitStarted,
70
markCommitStopped,
71
+ markComponentRenderStopped,
72
+ markComponentSuspended,
73
+ markComponentErrored,
74
markLayoutEffectsStarted,
75
markLayoutEffectsStopped,
76
markPassiveEffectsStarted,
@@ -1356,6 +1359,29 @@ function handleError(root, thrownValue): void {
1359
stopProfilerTimerIfRunningAndRecordDelta(erroredWork, true);
1360
}
1361
1362
+ if (enableSchedulingProfiler) {
1363
+ markComponentRenderStopped();
1364
+
1365
+ if (
1366
+ thrownValue !== null &&
1367
+ typeof thrownValue === 'object' &&
1368
+ typeof thrownValue.then === 'function'
1369
+ ) {
1370
+ const wakeable: Wakeable = (thrownValue: any);
1371
+ markComponentSuspended(
1372
+ erroredWork,
1373
+ wakeable,
1374
+ workInProgressRootRenderLanes,
1375
+ );
1376
+ } else {
1377
+ markComponentErrored(
1378
+ erroredWork,
1379
+ thrownValue,
1380
+ workInProgressRootRenderLanes,
1381
+ );
1382
+ }
1383
+ }
1384
+
1385
throwException(
1386
root,
1387
erroredWork.return,
packages/react-reconciler/src/ReactFiberWorkLoop.old.js
+26
@@ -68,6 +68,9 @@ import {
68
import {
69
markCommitStarted,
70
markCommitStopped,
71
+ markComponentRenderStopped,
72
+ markComponentSuspended,
73
+ markComponentErrored,
74
markLayoutEffectsStarted,
75
markLayoutEffectsStopped,
76
markPassiveEffectsStarted,
@@ -1356,6 +1359,29 @@ function handleError(root, thrownValue): void {
1359
stopProfilerTimerIfRunningAndRecordDelta(erroredWork, true);
1360
}
1361
1362
+ if (enableSchedulingProfiler) {
1363
+ markComponentRenderStopped();
1364
+
1365
+ if (
1366
+ thrownValue !== null &&
1367
+ typeof thrownValue === 'object' &&
1368
+ typeof thrownValue.then === 'function'
1369
+ ) {
1370
+ const wakeable: Wakeable = (thrownValue: any);
1371
+ markComponentSuspended(
1372
+ erroredWork,
1373
+ wakeable,
1374
+ workInProgressRootRenderLanes,
1375
+ );
1376
+ } else {
1377
+ markComponentErrored(
1378
+ erroredWork,
1379
+ thrownValue,
1380
+ workInProgressRootRenderLanes,
1381
+ );
1382
+ }
1383
+ }
1384
+
1385
throwException(
1386
root,
1387
erroredWork.return,
packages/react-reconciler/src/SchedulingProfiler.js
+27
@@ -144,6 +144,33 @@ export function markComponentRenderStopped(): void {
144
}
145
}
146
147
+export function markComponentErrored(
148
+ fiber: Fiber,
149
+ thrownValue: mixed,
150
+ lanes: Lanes,
151
+): void {
152
+ if (enableSchedulingProfiler) {
153
+ if (supportsUserTimingV3) {
154
+ const componentName = getComponentNameFromFiber(fiber) || 'Unknown';
155
+ const phase = fiber.alternate === null ? 'mount' : 'update';
156
+
157
+ let message = '';
158
+ if (
159
+ thrownValue !== null &&
160
+ typeof thrownValue === 'object' &&
161
+ typeof thrownValue.message === 'string'
162
+ ) {
163
+ message = thrownValue.message;
164
+ } else if (typeof thrownValue === 'string') {
165
+ message = thrownValue;
166
+ }
167
+
168
+ // TODO (scheduling profiler) Add component stack id
169
+ markAndClear(`--error-${componentName}-${phase}-${message}`);
170
+ }
171
+ }
172
+}
173
+
174
const PossiblyWeakMap = typeof WeakMap === 'function' ? WeakMap : Map;
175
176
// $FlowFixMe: Flow cannot handle polymorphic WeakMaps
packages/react-reconciler/src/__tests__/SchedulingProfiler-test.internal.js
+139
@@ -715,4 +715,143 @@ describe('SchedulingProfiler', () => {
715
`);
716
}
717
});
718
+
719
+ it('should mark sync render that throws', async () => {
720
+ class ErrorBoundary extends React.Component {
721
+ state = {error: null};
722
+ componentDidCatch(error) {
723
+ this.setState({error});
724
+ }
725
+ render() {
726
+ if (this.state.error) {
727
+ return null;
728
+ }
729
+ return this.props.children;
730
+ }
731
+ }
732
+
733
+ function ExampleThatThrows() {
734
+ throw Error('Expected error');
735
+ }
736
+
737
+ ReactTestRenderer.create(
738
+ <ErrorBoundary>
739
+ <ExampleThatThrows />
740
+ </ErrorBoundary>,
741
+ );
742
+
743
+ if (gate(flags => flags.enableSchedulingProfiler)) {
744
+ expect(getMarks()).toMatchInlineSnapshot(`
745
+ Array [
746
+ "--schedule-render-1",
747
+ "--render-start-1",
748
+ "--component-render-start-ErrorBoundary",
749
+ "--component-render-stop",
750
+ "--component-render-start-ExampleThatThrows",
751
+ "--component-render-start-ExampleThatThrows",
752
+ "--component-render-stop",
753
+ "--error-ExampleThatThrows-mount-Expected error",
754
+ "--render-stop",
755
+ "--commit-start-1",
756
+ "--react-version-17.0.3",
757
+ "--profiler-version-1",
758
+ "--react-lane-labels-Sync,InputContinuousHydration,InputContinuous,DefaultHydration,Default,TransitionHydration,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Retry,Retry,Retry,Retry,Retry,SelectiveHydration,IdleHydration,Idle,Offscreen",
759
+ "--layout-effects-start-1",
760
+ "--schedule-state-update-1-ErrorBoundary",
761
+ "--layout-effects-stop",
762
+ "--commit-stop",
763
+ "--render-start-1",
764
+ "--component-render-start-ErrorBoundary",
765
+ "--component-render-stop",
766
+ "--render-stop",
767
+ "--commit-start-1",
768
+ "--react-version-17.0.3",
769
+ "--profiler-version-1",
770
+ "--react-lane-labels-Sync,InputContinuousHydration,InputContinuous,DefaultHydration,Default,TransitionHydration,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Retry,Retry,Retry,Retry,Retry,SelectiveHydration,IdleHydration,Idle,Offscreen",
771
+ "--commit-stop",
772
+ ]
773
+ `);
774
+ }
775
+ });
776
+
777
+ it('should mark concurrent render that throws', async () => {
778
+ spyOnProd(console, 'error');
779
+
780
+ class ErrorBoundary extends React.Component {
781
+ state = {error: null};
782
+ componentDidCatch(error) {
783
+ this.setState({error});
784
+ }
785
+ render() {
786
+ if (this.state.error) {
787
+ return null;
788
+ }
789
+ return this.props.children;
790
+ }
791
+ }
792
+
793
+ function ExampleThatThrows() {
794
+ // eslint-disable-next-line no-throw-literal
795
+ throw 'Expected error';
796
+ }
797
+
798
+ ReactTestRenderer.create(
799
+ <ErrorBoundary>
800
+ <ExampleThatThrows />
801
+ </ErrorBoundary>,
802
+ {unstable_isConcurrent: true},
803
+ );
804
+
805
+ if (gate(flags => flags.enableSchedulingProfiler)) {
806
+ expect(getMarks()).toMatchInlineSnapshot(`
807
+ Array [
808
+ "--schedule-render-16",
809
+ ]
810
+ `);
811
+ }
812
+
813
+ clearPendingMarks();
814
+
815
+ expect(Scheduler).toFlushUntilNextPaint([]);
816
+
817
+ if (gate(flags => flags.enableSchedulingProfiler)) {
818
+ expect(getMarks()).toMatchInlineSnapshot(`
819
+ Array [
820
+ "--render-start-16",
821
+ "--component-render-start-ErrorBoundary",
822
+ "--component-render-stop",
823
+ "--component-render-start-ExampleThatThrows",
824
+ "--component-render-start-ExampleThatThrows",
825
+ "--component-render-stop",
826
+ "--error-ExampleThatThrows-mount-Expected error",
827
+ "--render-stop",
828
+ "--render-start-16",
829
+ "--component-render-start-ErrorBoundary",
830
+ "--component-render-stop",
831
+ "--component-render-start-ExampleThatThrows",
832
+ "--component-render-start-ExampleThatThrows",
833
+ "--component-render-stop",
834
+ "--error-ExampleThatThrows-mount-Expected error",
835
+ "--render-stop",
836
+ "--commit-start-16",
837
+ "--react-version-17.0.3",
838
+ "--profiler-version-1",
839
+ "--react-lane-labels-Sync,InputContinuousHydration,InputContinuous,DefaultHydration,Default,TransitionHydration,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Retry,Retry,Retry,Retry,Retry,SelectiveHydration,IdleHydration,Idle,Offscreen",
840
+ "--layout-effects-start-16",
841
+ "--schedule-state-update-1-ErrorBoundary",
842
+ "--layout-effects-stop",
843
+ "--render-start-1",
844
+ "--component-render-start-ErrorBoundary",
845
+ "--component-render-stop",
846
+ "--render-stop",
847
+ "--commit-start-1",
848
+ "--react-version-17.0.3",
849
+ "--profiler-version-1",
850
+ "--react-lane-labels-Sync,InputContinuousHydration,InputContinuous,DefaultHydration,Default,TransitionHydration,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Retry,Retry,Retry,Retry,Retry,SelectiveHydration,IdleHydration,Idle,Offscreen",
851
+ "--commit-stop",
852
+ "--commit-stop",
853
+ ]
854
+ `);
855
+ }
856
+ });
857
});