Scheduling Profiler marks should include thrown Errors (facebook#22417)

Brian Vaughn · zhengjitf · commit e6697299b58a · 2022-04-15T14:00:46.000+08:00
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.
diff --git a/packages/react-reconciler/src/ReactFiberThrow.new.js b/packages/react-reconciler/src/ReactFiberThrow.new.js
@@ -41,7 +41,6 @@ import {shouldCaptureSuspense} from './ReactFiberSuspenseComponent.new';
 import {NoMode, ConcurrentMode, DebugTracingMode} from './ReactTypeOfMode';
 import {
   enableDebugTracing,
-  enableSchedulingProfiler,
   enableLazyContextPropagation,
   enableUpdaterTracking,
   enablePersistentOffscreenHostContainer,
@@ -71,10 +70,6 @@ import {
 import {propagateParentContextChangesToDeferredTree} from './ReactFiberNewContext.new';
 import {logCapturedError} from './ReactFiberErrorLogger';
 import {logComponentSuspended} from './DebugTracing';
-import {
-  markComponentRenderStopped,
-  markComponentSuspended,
-} from './SchedulingProfiler';
 import {isDevToolsPresent} from './ReactFiberDevToolsHook.new';
 import {
   SyncLane,
@@ -247,11 +242,6 @@ function throwException(
       }
     }
 
-    if (enableSchedulingProfiler) {
-      markComponentRenderStopped();
-      markComponentSuspended(sourceFiber, wakeable, rootRenderLanes);
-    }
-
     // Reset the memoizedState to what it was before we attempted to render it.
     // A legacy mode Suspense quirk, only relevant to hook components.
     const tag = sourceFiber.tag;
diff --git a/packages/react-reconciler/src/ReactFiberThrow.old.js b/packages/react-reconciler/src/ReactFiberThrow.old.js
@@ -41,7 +41,6 @@ import {shouldCaptureSuspense} from './ReactFiberSuspenseComponent.old';
 import {NoMode, ConcurrentMode, DebugTracingMode} from './ReactTypeOfMode';
 import {
   enableDebugTracing,
-  enableSchedulingProfiler,
   enableLazyContextPropagation,
   enableUpdaterTracking,
   enablePersistentOffscreenHostContainer,
@@ -71,10 +70,6 @@ import {
 import {propagateParentContextChangesToDeferredTree} from './ReactFiberNewContext.old';
 import {logCapturedError} from './ReactFiberErrorLogger';
 import {logComponentSuspended} from './DebugTracing';
-import {
-  markComponentRenderStopped,
-  markComponentSuspended,
-} from './SchedulingProfiler';
 import {isDevToolsPresent} from './ReactFiberDevToolsHook.old';
 import {
   SyncLane,
@@ -247,11 +242,6 @@ function throwException(
       }
     }
 
-    if (enableSchedulingProfiler) {
-      markComponentRenderStopped();
-      markComponentSuspended(sourceFiber, wakeable, rootRenderLanes);
-    }
-
     // Reset the memoizedState to what it was before we attempted to render it.
     // A legacy mode Suspense quirk, only relevant to hook components.
     const tag = sourceFiber.tag;
diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
@@ -68,6 +68,9 @@ import {
 import {
   markCommitStarted,
   markCommitStopped,
+  markComponentRenderStopped,
+  markComponentSuspended,
+  markComponentErrored,
   markLayoutEffectsStarted,
   markLayoutEffectsStopped,
   markPassiveEffectsStarted,
@@ -1356,6 +1359,29 @@ function handleError(root, thrownValue): void {
         stopProfilerTimerIfRunningAndRecordDelta(erroredWork, true);
       }
 
+      if (enableSchedulingProfiler) {
+        markComponentRenderStopped();
+
+        if (
+          thrownValue !== null &&
+          typeof thrownValue === 'object' &&
+          typeof thrownValue.then === 'function'
+        ) {
+          const wakeable: Wakeable = (thrownValue: any);
+          markComponentSuspended(
+            erroredWork,
+            wakeable,
+            workInProgressRootRenderLanes,
+          );
+        } else {
+          markComponentErrored(
+            erroredWork,
+            thrownValue,
+            workInProgressRootRenderLanes,
+          );
+        }
+      }
+
       throwException(
         root,
         erroredWork.return,
diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js
@@ -68,6 +68,9 @@ import {
 import {
   markCommitStarted,
   markCommitStopped,
+  markComponentRenderStopped,
+  markComponentSuspended,
+  markComponentErrored,
   markLayoutEffectsStarted,
   markLayoutEffectsStopped,
   markPassiveEffectsStarted,
@@ -1356,6 +1359,29 @@ function handleError(root, thrownValue): void {
         stopProfilerTimerIfRunningAndRecordDelta(erroredWork, true);
       }
 
+      if (enableSchedulingProfiler) {
+        markComponentRenderStopped();
+
+        if (
+          thrownValue !== null &&
+          typeof thrownValue === 'object' &&
+          typeof thrownValue.then === 'function'
+        ) {
+          const wakeable: Wakeable = (thrownValue: any);
+          markComponentSuspended(
+            erroredWork,
+            wakeable,
+            workInProgressRootRenderLanes,
+          );
+        } else {
+          markComponentErrored(
+            erroredWork,
+            thrownValue,
+            workInProgressRootRenderLanes,
+          );
+        }
+      }
+
       throwException(
         root,
         erroredWork.return,
diff --git a/packages/react-reconciler/src/SchedulingProfiler.js b/packages/react-reconciler/src/SchedulingProfiler.js
@@ -144,6 +144,33 @@ export function markComponentRenderStopped(): void {
   }
 }
 
+export function markComponentErrored(
+  fiber: Fiber,
+  thrownValue: mixed,
+  lanes: Lanes,
+): void {
+  if (enableSchedulingProfiler) {
+    if (supportsUserTimingV3) {
+      const componentName = getComponentNameFromFiber(fiber) || 'Unknown';
+      const phase = fiber.alternate === null ? 'mount' : 'update';
+
+      let message = '';
+      if (
+        thrownValue !== null &&
+        typeof thrownValue === 'object' &&
+        typeof thrownValue.message === 'string'
+      ) {
+        message = thrownValue.message;
+      } else if (typeof thrownValue === 'string') {
+        message = thrownValue;
+      }
+
+      // TODO (scheduling profiler) Add component stack id
+      markAndClear(`--error-${componentName}-${phase}-${message}`);
+    }
+  }
+}
+
 const PossiblyWeakMap = typeof WeakMap === 'function' ? WeakMap : Map;
 
 // $FlowFixMe: Flow cannot handle polymorphic WeakMaps
diff --git a/packages/react-reconciler/src/__tests__/SchedulingProfiler-test.internal.js b/packages/react-reconciler/src/__tests__/SchedulingProfiler-test.internal.js
@@ -715,4 +715,143 @@ describe('SchedulingProfiler', () => {
     `);
     }
   });
+
+  it('should mark sync render that throws', async () => {
+    class ErrorBoundary extends React.Component {
+      state = {error: null};
+      componentDidCatch(error) {
+        this.setState({error});
+      }
+      render() {
+        if (this.state.error) {
+          return null;
+        }
+        return this.props.children;
+      }
+    }
+
+    function ExampleThatThrows() {
+      throw Error('Expected error');
+    }
+
+    ReactTestRenderer.create(
+      <ErrorBoundary>
+        <ExampleThatThrows />
+      </ErrorBoundary>,
+    );
+
+    if (gate(flags => flags.enableSchedulingProfiler)) {
+      expect(getMarks()).toMatchInlineSnapshot(`
+      Array [
+        "--schedule-render-1",
+        "--render-start-1",
+        "--component-render-start-ErrorBoundary",
+        "--component-render-stop",
+        "--component-render-start-ExampleThatThrows",
+        "--component-render-start-ExampleThatThrows",
+        "--component-render-stop",
+        "--error-ExampleThatThrows-mount-Expected error",
+        "--render-stop",
+        "--commit-start-1",
+        "--react-version-17.0.3",
+        "--profiler-version-1",
+        "--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",
+        "--layout-effects-start-1",
+        "--schedule-state-update-1-ErrorBoundary",
+        "--layout-effects-stop",
+        "--commit-stop",
+        "--render-start-1",
+        "--component-render-start-ErrorBoundary",
+        "--component-render-stop",
+        "--render-stop",
+        "--commit-start-1",
+        "--react-version-17.0.3",
+        "--profiler-version-1",
+        "--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",
+        "--commit-stop",
+      ]
+    `);
+    }
+  });
+
+  it('should mark concurrent render that throws', async () => {
+    spyOnProd(console, 'error');
+
+    class ErrorBoundary extends React.Component {
+      state = {error: null};
+      componentDidCatch(error) {
+        this.setState({error});
+      }
+      render() {
+        if (this.state.error) {
+          return null;
+        }
+        return this.props.children;
+      }
+    }
+
+    function ExampleThatThrows() {
+      // eslint-disable-next-line no-throw-literal
+      throw 'Expected error';
+    }
+
+    ReactTestRenderer.create(
+      <ErrorBoundary>
+        <ExampleThatThrows />
+      </ErrorBoundary>,
+      {unstable_isConcurrent: true},
+    );
+
+    if (gate(flags => flags.enableSchedulingProfiler)) {
+      expect(getMarks()).toMatchInlineSnapshot(`
+      Array [
+        "--schedule-render-16",
+      ]
+    `);
+    }
+
+    clearPendingMarks();
+
+    expect(Scheduler).toFlushUntilNextPaint([]);
+
+    if (gate(flags => flags.enableSchedulingProfiler)) {
+      expect(getMarks()).toMatchInlineSnapshot(`
+        Array [
+          "--render-start-16",
+          "--component-render-start-ErrorBoundary",
+          "--component-render-stop",
+          "--component-render-start-ExampleThatThrows",
+          "--component-render-start-ExampleThatThrows",
+          "--component-render-stop",
+          "--error-ExampleThatThrows-mount-Expected error",
+          "--render-stop",
+          "--render-start-16",
+          "--component-render-start-ErrorBoundary",
+          "--component-render-stop",
+          "--component-render-start-ExampleThatThrows",
+          "--component-render-start-ExampleThatThrows",
+          "--component-render-stop",
+          "--error-ExampleThatThrows-mount-Expected error",
+          "--render-stop",
+          "--commit-start-16",
+          "--react-version-17.0.3",
+          "--profiler-version-1",
+          "--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",
+          "--layout-effects-start-16",
+          "--schedule-state-update-1-ErrorBoundary",
+          "--layout-effects-stop",
+          "--render-start-1",
+          "--component-render-start-ErrorBoundary",
+          "--component-render-stop",
+          "--render-stop",
+          "--commit-start-1",
+          "--react-version-17.0.3",
+          "--profiler-version-1",
+          "--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",
+          "--commit-stop",
+          "--commit-stop",
+        ]
+      `);
+    }
+  });
 });
