Skip to content
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.

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

[React DevTools] Improve DevTools UI when Inspecting a user Component that Throws an Error #24248

Merged
merged 15 commits into from
May 6, 2022
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -2145,7 +2145,7 @@ describe('InspectedElement', () => {
expect(value).toBe(null);

const error = errorBoundaryInstance.state.error;
expect(error.message).toBe('Error rendering inspected component');
expect(error.message).toBe('Expected');
expect(error.stack).toContain('inspectHooksOfFiber');
});

Expand Down
48 changes: 48 additions & 0 deletions packages/react-devtools-shared/src/backend/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -3607,10 +3607,58 @@ export function attach(
try {
mostRecentlyInspectedElement = inspectElementRaw(id);
} catch (error) {
// the error name is synced with ReactDebugHooks
if (error.name === 'ReactDebugToolsRenderError') {
let message = 'Error rendering inspected element.';
mondaychen marked this conversation as resolved.
Show resolved Hide resolved
let stack;
// Log error & cause for user to debug
console.error(message + '\n\n', error);
if (error.cause != null) {
const fiber = findCurrentFiberUsingSlowPathById(id);
const componentName =
fiber != null ? getDisplayNameForFiber(fiber) : null;
console.error(
'React DevTools encountered an error while trying to inspect hooks. ' +
'This is most likely caused by an error in current inspected component' +
(componentName != null ? `: "${componentName}".` : '.') +
'\nThe error thrown in the component is: \n\n',
error.cause,
);
if (error.cause instanceof Error) {
message = error.cause.message || message;
stack = error.cause.stack;
}
}

return {
type: 'error',
errorType: 'user',
id,
responseID: requestID,
message,
stack,
};
}

// the error name is synced with ReactDebugHooks
if (error.name === 'ReactDebugToolsUnsupportedHookError') {
return {
type: 'error',
errorType: 'unknown-hook',
id,
responseID: requestID,
message:
'Unsupported hook in the react-debug-tools package: ' +
error.message,
};
}

// Log Uncaught Error
console.error('Error inspecting element.\n\n', error);

return {
type: 'error',
errorType: 'uncaught',
id,
responseID: requestID,
message: error.message,
Expand Down
3 changes: 2 additions & 1 deletion packages/react-devtools-shared/src/backend/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -289,8 +289,9 @@ export type InspectElementError = {|
id: number,
responseID: number,
type: 'error',
errorType: 'user' | 'unknown-hook' | 'uncaught',
message: string,
stack: string,
stack?: string,
|};

export type InspectElementFullData = {|
Expand Down
2 changes: 1 addition & 1 deletion packages/react-devtools-shared/src/backendAPI.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import {hydrate, fillInPath} from 'react-devtools-shared/src/hydration';
import {separateDisplayNameAndHOCs} from 'react-devtools-shared/src/utils';
import Store from 'react-devtools-shared/src/devtools/store';
import TimeoutError from 'react-devtools-shared/src/TimeoutError';
import TimeoutError from 'react-devtools-shared/src/errors/TimeoutError';

import type {
InspectedElement as InspectedElementBackend,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow
*/

import * as React from 'react';
import styles from './shared.css';

type Props = {|
callStack: string | null,
children: React$Node,
info: React$Node | null,
componentStack: string | null,
errorMessage: string,
|};

export default function CaughtErrorView({
callStack,
children,
info,
componentStack,
errorMessage,
}: Props) {
return (
<div className={styles.ErrorBoundary}>
{children}
<div className={styles.ErrorInfo}>
<div className={styles.HeaderRow}>
<div className={styles.ErrorHeader}>{errorMessage}</div>
</div>
{!!info && <div className={styles.InfoBox}>{info}</div>}
{!!callStack && (
<div className={styles.ErrorStack}>
The error was thrown {callStack.trim()}
</div>
)}
</div>
</div>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,11 @@ import ErrorView from './ErrorView';
import SearchingGitHubIssues from './SearchingGitHubIssues';
import SuspendingErrorView from './SuspendingErrorView';
import TimeoutView from './TimeoutView';
import CaughtErrorView from './CaughtErrorView';
import UnsupportedBridgeOperationError from 'react-devtools-shared/src/UnsupportedBridgeOperationError';
import TimeoutError from 'react-devtools-shared/src/TimeoutError';
import TimeoutError from 'react-devtools-shared/src/errors/TimeoutError';
import UserError from 'react-devtools-shared/src/errors/UserError';
import UnknownHookError from 'react-devtools-shared/src/errors/UnknownHookError';
import {logEvent} from 'react-devtools-shared/src/Logger';

type Props = {|
Expand All @@ -34,6 +37,8 @@ type State = {|
hasError: boolean,
isUnsupportedBridgeOperationError: boolean,
isTimeout: boolean,
isUserError: boolean,
isUnknownHookError: boolean,
|};

const InitialState: State = {
Expand All @@ -44,6 +49,8 @@ const InitialState: State = {
hasError: false,
isUnsupportedBridgeOperationError: false,
isTimeout: false,
isUserError: false,
isUnknownHookError: false,
};

export default class ErrorBoundary extends Component<Props, State> {
Expand All @@ -58,6 +65,8 @@ export default class ErrorBoundary extends Component<Props, State> {
: null;

const isTimeout = error instanceof TimeoutError;
const isUserError = error instanceof UserError;
const isUnknownHookError = error instanceof UnknownHookError;
const isUnsupportedBridgeOperationError =
error instanceof UnsupportedBridgeOperationError;

Expand All @@ -76,7 +85,9 @@ export default class ErrorBoundary extends Component<Props, State> {
errorMessage,
hasError: true,
isUnsupportedBridgeOperationError,
isUnknownHookError,
isTimeout,
isUserError,
};
}

Expand Down Expand Up @@ -111,6 +122,8 @@ export default class ErrorBoundary extends Component<Props, State> {
hasError,
isUnsupportedBridgeOperationError,
isTimeout,
isUserError,
isUnknownHookError,
} = this.state;

if (hasError) {
Expand All @@ -133,6 +146,37 @@ export default class ErrorBoundary extends Component<Props, State> {
errorMessage={errorMessage}
/>
);
} else if (isUserError) {
return (
<CaughtErrorView
callStack={callStack}
componentStack={componentStack}
errorMessage={errorMessage || 'Error occured in inspected element'}
info={
<>
React DevTools encountered an error while trying to inspect the
hooks. This is most likely caused by a developer error in the
currently inspected element. Please see your console for logged
error.
</>
}
/>
);
} else if (isUnknownHookError) {
return (
<CaughtErrorView
callStack={callStack}
componentStack={componentStack}
errorMessage={errorMessage || 'Encountered an unknown hook'}
info={
mondaychen marked this conversation as resolved.
Show resolved Hide resolved
<>
React DevTools encountered an unknown hook. This is probably
because the react-debug-tools package is out of date. To fix,
upgrade the React DevTools to the most recent version.
</>
}
/>
);
} else {
return (
<ErrorView
Expand All @@ -141,10 +185,7 @@ export default class ErrorBoundary extends Component<Props, State> {
dismissError={
canDismissProp || canDismissState ? this._dismissError : null
}
errorMessage={errorMessage}
isUnsupportedBridgeOperationError={
mondaychen marked this conversation as resolved.
Show resolved Hide resolved
isUnsupportedBridgeOperationError
}>
errorMessage={errorMessage}>
<Suspense fallback={<SearchingGitHubIssues />}>
<SuspendingErrorView
callStack={callStack}
Expand Down
21 changes: 21 additions & 0 deletions packages/react-devtools-shared/src/errors/UnknownHookError.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow
*/

export default class UnknownHookError extends Error {
constructor(message: string) {
super(message);

// Maintains proper stack trace for where our error was thrown (only available on V8)
if (Error.captureStackTrace) {
Error.captureStackTrace(this, UnknownHookError);
}

this.name = 'UnknownHookError';
}
}
21 changes: 21 additions & 0 deletions packages/react-devtools-shared/src/errors/UserError.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow
*/

export default class UserError extends Error {
constructor(message: string) {
super(message);

// Maintains proper stack trace for where our error was thrown (only available on V8)
if (Error.captureStackTrace) {
Error.captureStackTrace(this, UserError);
}

this.name = 'UserError';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ import type {
InspectedElement as InspectedElementFrontend,
InspectedElementResponseType,
} from 'react-devtools-shared/src/devtools/views/Components/types';
import UserError from 'react-devtools-shared/src/errors/UserError';
import UnknownHookError from 'react-devtools-shared/src/errors/UnknownHookError';

// Maps element ID to inspected data.
// We use an LRU for this rather than a WeakMap because of how the "no-change" optimization works.
Expand Down Expand Up @@ -80,14 +82,24 @@ export function inspectElement({

let inspectedElement;
switch (type) {
case 'error':
const {message, stack} = ((data: any): InspectElementError);

case 'error': {
const {message, stack, errorType} = ((data: any): InspectElementError);

// create a different error class for each error type
// and keep useful information from backend.
let error;
if (errorType === 'user') {
error = new UserError(message);
} else if (errorType === 'unknown-hook') {
error = new UnknownHookError(message);
} else {
error = new Error(message);
}
// The backend's stack (where the error originated) is more meaningful than this stack.
const error = new Error(message);
error.stack = stack;
error.stack = stack || error.stack;

throw error;
}

case 'no-change':
// This is a no-op for the purposes of our cache.
Expand Down