Skip to content

Scheduling Profiler marks should include thrown Errors #22417

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 24, 2021
Merged

Scheduling Profiler marks should include thrown Errors #22417

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 0 additions & 10 deletions packages/react-reconciler/src/ReactFiberThrow.new.js
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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",
]
`);
}
});
});