Skip to content

Commit

Permalink
Scheduling Profiler marks should include thrown Errors
Browse files Browse the repository at this point in the history
  • Loading branch information
Brian Vaughn committed Sep 24, 2021
1 parent 1c58cfa commit f4ab4e0
Show file tree
Hide file tree
Showing 6 changed files with 218 additions and 20 deletions.
10 changes: 0 additions & 10 deletions packages/react-reconciler/src/ReactFiberThrow.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ import {shouldCaptureSuspense} from './ReactFiberSuspenseComponent.new';
import {NoMode, ConcurrentMode, DebugTracingMode} from './ReactTypeOfMode';
import {
enableDebugTracing,
enableSchedulingProfiler,
enableLazyContextPropagation,
enableUpdaterTracking,
enablePersistentOffscreenHostContainer,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down
10 changes: 0 additions & 10 deletions packages/react-reconciler/src/ReactFiberThrow.old.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ import {shouldCaptureSuspense} from './ReactFiberSuspenseComponent.old';
import {NoMode, ConcurrentMode, DebugTracingMode} from './ReactTypeOfMode';
import {
enableDebugTracing,
enableSchedulingProfiler,
enableLazyContextPropagation,
enableUpdaterTracking,
enablePersistentOffscreenHostContainer,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down
26 changes: 26 additions & 0 deletions packages/react-reconciler/src/ReactFiberWorkLoop.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ import {
import {
markCommitStarted,
markCommitStopped,
markComponentRenderStopped,
markComponentSuspended,
markComponentErrored,
markLayoutEffectsStarted,
markLayoutEffectsStopped,
markPassiveEffectsStarted,
Expand Down Expand Up @@ -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,
Expand Down
26 changes: 26 additions & 0 deletions packages/react-reconciler/src/ReactFiberWorkLoop.old.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ import {
import {
markCommitStarted,
markCommitStopped,
markComponentRenderStopped,
markComponentSuspended,
markComponentErrored,
markLayoutEffectsStarted,
markLayoutEffectsStopped,
markPassiveEffectsStarted,
Expand Down Expand Up @@ -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,
Expand Down
27 changes: 27 additions & 0 deletions packages/react-reconciler/src/SchedulingProfiler.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
139 changes: 139 additions & 0 deletions packages/react-reconciler/src/__tests__/SchedulingProfiler-test.internal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
]
`);
}
});
});

0 comments on commit f4ab4e0

Please sign in to comment.