From b4a8d91d6ad1e26f648e7f8d781a8c0b49d62e10 Mon Sep 17 00:00:00 2001 From: Mengdi Chen Date: Fri, 25 Mar 2022 15:47:07 -0400 Subject: [PATCH 01/15] [ReactDevTools] custom view for errors occur in user's code --- .../src/backend/renderer.js | 23 ++++++++- .../src/backend/types.js | 10 ++++ .../react-devtools-shared/src/backendAPI.js | 2 +- .../src/devtools/views/Components/types.js | 1 + .../views/ErrorBoundary/ErrorBoundary.js | 17 ++++++- .../views/ErrorBoundary/UserErrorView.js | 47 +++++++++++++++++++ .../src/{ => errors}/TimeoutError.js | 0 .../src/errors/UserError.js | 21 +++++++++ .../src/inspectedElementMutableSource.js | 14 +++++- 9 files changed, 131 insertions(+), 4 deletions(-) create mode 100644 packages/react-devtools-shared/src/devtools/views/ErrorBoundary/UserErrorView.js rename packages/react-devtools-shared/src/{ => errors}/TimeoutError.js (100%) create mode 100644 packages/react-devtools-shared/src/errors/UserError.js diff --git a/packages/react-devtools-shared/src/backend/renderer.js b/packages/react-devtools-shared/src/backend/renderer.js index c161902df711c..9cf7ba8ef0adb 100644 --- a/packages/react-devtools-shared/src/backend/renderer.js +++ b/packages/react-devtools-shared/src/backend/renderer.js @@ -60,7 +60,7 @@ import { TREE_OPERATION_UPDATE_ERRORS_OR_WARNINGS, TREE_OPERATION_UPDATE_TREE_BASE_DURATION, } from '../constants'; -import {inspectHooksOfFiber} from 'react-debug-tools'; +import {inspectHooksOfFiber, ErrorsNames as DebugToolsErrors} from 'react-debug-tools'; import { patch as patchConsole, registerRenderer as registerRendererWithConsole, @@ -3607,6 +3607,27 @@ export function attach( try { mostRecentlyInspectedElement = inspectElementRaw(id); } catch (error) { + if (error.name === DebugToolsErrors.RENDER_FUNCTION_ERROR) { + let message = 'Error rendering inspected element.'; + let stack; + console.error(message + '\n\n', error); + if (error.cause != null) { + console.error('Original error causing above error: \n\n', error.cause); + if (error.cause instanceof Error) { + message = error.cause.message || message; + stack = error.cause.stack; + } + } + + return { + type: 'user-error', + id, + responseID: requestID, + message, + stack, + }; + } + console.error('Error inspecting element.\n\n', error); return { diff --git a/packages/react-devtools-shared/src/backend/types.js b/packages/react-devtools-shared/src/backend/types.js index 4d975dbfec0d5..11019d3f0e4f2 100644 --- a/packages/react-devtools-shared/src/backend/types.js +++ b/packages/react-devtools-shared/src/backend/types.js @@ -281,6 +281,7 @@ export type InspectedElement = {| |}; export const InspectElementErrorType = 'error'; +export const InspectElementUserErrorType = 'user-error'; export const InspectElementFullDataType = 'full-data'; export const InspectElementNoChangeType = 'no-change'; export const InspectElementNotFoundType = 'not-found'; @@ -293,6 +294,14 @@ export type InspectElementError = {| stack: string, |}; +export type InspectElementUserError = {| + id: number, + responseID: number, + type: 'user-error', + message: string, + stack: ?string, +|}; + export type InspectElementFullData = {| id: number, responseID: number, @@ -322,6 +331,7 @@ export type InspectElementNotFound = {| export type InspectedElementPayload = | InspectElementError + | InspectElementUserError | InspectElementFullData | InspectElementHydratedPath | InspectElementNoChange diff --git a/packages/react-devtools-shared/src/backendAPI.js b/packages/react-devtools-shared/src/backendAPI.js index 3849899b7df02..adf0c5e8b0911 100644 --- a/packages/react-devtools-shared/src/backendAPI.js +++ b/packages/react-devtools-shared/src/backendAPI.js @@ -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, diff --git a/packages/react-devtools-shared/src/devtools/views/Components/types.js b/packages/react-devtools-shared/src/devtools/views/Components/types.js index c6789fa1e9ec1..d6918a0813606 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/types.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/types.js @@ -63,6 +63,7 @@ export type OwnersList = {| export type InspectedElementResponseType = | 'error' + | 'user-error' | 'full-data' | 'hydrated-path' | 'no-change' diff --git a/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ErrorBoundary.js b/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ErrorBoundary.js index f4994e23a9bf5..c770ce1b436ae 100644 --- a/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ErrorBoundary.js +++ b/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ErrorBoundary.js @@ -15,8 +15,10 @@ import ErrorView from './ErrorView'; import SearchingGitHubIssues from './SearchingGitHubIssues'; import SuspendingErrorView from './SuspendingErrorView'; import TimeoutView from './TimeoutView'; +import UserErrorView from './UserErrorView'; 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 {logEvent} from 'react-devtools-shared/src/Logger'; type Props = {| @@ -34,6 +36,7 @@ type State = {| hasError: boolean, isUnsupportedBridgeOperationError: boolean, isTimeout: boolean, + isUserError: boolean, |}; const InitialState: State = { @@ -44,6 +47,7 @@ const InitialState: State = { hasError: false, isUnsupportedBridgeOperationError: false, isTimeout: false, + isUserError: false, }; export default class ErrorBoundary extends Component { @@ -58,6 +62,7 @@ export default class ErrorBoundary extends Component { : null; const isTimeout = error instanceof TimeoutError; + const isUserError = error instanceof UserError; const isUnsupportedBridgeOperationError = error instanceof UnsupportedBridgeOperationError; @@ -77,6 +82,7 @@ export default class ErrorBoundary extends Component { hasError: true, isUnsupportedBridgeOperationError, isTimeout, + isUserError, }; } @@ -111,6 +117,7 @@ export default class ErrorBoundary extends Component { hasError, isUnsupportedBridgeOperationError, isTimeout, + isUserError, } = this.state; if (hasError) { @@ -133,6 +140,14 @@ export default class ErrorBoundary extends Component { errorMessage={errorMessage} /> ); + } else if (isUserError) { + return ( + + ); } else { return ( + {children} +
+
+
+ {errorMessage || 'Error occured in inspected element'} +
+
+
+ This is likely to be caused by implementation of current inspected element. +
+ {!!callStack && ( +
+ The error was thrown {callStack.trim()} +
+ )} +
+ + ); + } + \ No newline at end of file diff --git a/packages/react-devtools-shared/src/TimeoutError.js b/packages/react-devtools-shared/src/errors/TimeoutError.js similarity index 100% rename from packages/react-devtools-shared/src/TimeoutError.js rename to packages/react-devtools-shared/src/errors/TimeoutError.js diff --git a/packages/react-devtools-shared/src/errors/UserError.js b/packages/react-devtools-shared/src/errors/UserError.js new file mode 100644 index 0000000000000..752dbf9e88403 --- /dev/null +++ b/packages/react-devtools-shared/src/errors/UserError.js @@ -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'; + } +} diff --git a/packages/react-devtools-shared/src/inspectedElementMutableSource.js b/packages/react-devtools-shared/src/inspectedElementMutableSource.js index d91d8de8c1de4..905c4f7bc97fa 100644 --- a/packages/react-devtools-shared/src/inspectedElementMutableSource.js +++ b/packages/react-devtools-shared/src/inspectedElementMutableSource.js @@ -8,6 +8,7 @@ */ import LRU from 'lru-cache'; +import {UserHookError} from 'react-debug-tools'; import { convertInspectedElementBackendToFrontend, hydrateHelper, @@ -19,6 +20,7 @@ import type {LRUCache} from 'react-devtools-shared/src/types'; import type {FrontendBridge} from 'react-devtools-shared/src/bridge'; import type { InspectElementError, + InspectElementUserError, InspectElementFullData, InspectElementHydratedPath, } from 'react-devtools-shared/src/backend/types'; @@ -27,6 +29,7 @@ import type { InspectedElement as InspectedElementFrontend, InspectedElementResponseType, } from 'react-devtools-shared/src/devtools/views/Components/types'; +import UserError from 'react-devtools-shared/src/errors/UserError'; // Maps element ID to inspected data. // We use an LRU for this rather than a WeakMap because of how the "no-change" optimization works. @@ -80,7 +83,7 @@ export function inspectElement({ let inspectedElement; switch (type) { - case 'error': + case 'error': { const {message, stack} = ((data: any): InspectElementError); // The backend's stack (where the error originated) is more meaningful than this stack. @@ -88,6 +91,15 @@ export function inspectElement({ error.stack = stack; throw error; + } + + case 'user-error': { + const {message, stack} = (data: InspectElementUserError); + // Trying to keep useful information from user's side. + const error = new UserError(message); + error.stack = stack || error.stack; + throw error; + } case 'no-change': // This is a no-op for the purposes of our cache. From ce3cea5ef88cf0028297138dd752a7ed07e2f055 Mon Sep 17 00:00:00 2001 From: Mengdi Chen Date: Tue, 29 Mar 2022 10:37:38 -0400 Subject: [PATCH 02/15] [ReactDevTools] show message for unsupported feature --- .../src/backend/renderer.js | 21 ++++++++- .../src/backend/types.js | 9 ++++ .../views/ErrorBoundary/CaughtErrorView.js | 44 +++++++++++++++++ .../views/ErrorBoundary/ErrorBoundary.js | 42 ++++++++++++++--- .../views/ErrorBoundary/UserErrorView.js | 47 ------------------- .../src/errors/UnsupportedFeatureError.js | 21 +++++++++ .../src/errors/UserError.js | 2 +- .../src/inspectedElementMutableSource.js | 9 ++++ 8 files changed, 138 insertions(+), 57 deletions(-) create mode 100644 packages/react-devtools-shared/src/devtools/views/ErrorBoundary/CaughtErrorView.js delete mode 100644 packages/react-devtools-shared/src/devtools/views/ErrorBoundary/UserErrorView.js create mode 100644 packages/react-devtools-shared/src/errors/UnsupportedFeatureError.js diff --git a/packages/react-devtools-shared/src/backend/renderer.js b/packages/react-devtools-shared/src/backend/renderer.js index 9cf7ba8ef0adb..041b36811807d 100644 --- a/packages/react-devtools-shared/src/backend/renderer.js +++ b/packages/react-devtools-shared/src/backend/renderer.js @@ -60,7 +60,10 @@ import { TREE_OPERATION_UPDATE_ERRORS_OR_WARNINGS, TREE_OPERATION_UPDATE_TREE_BASE_DURATION, } from '../constants'; -import {inspectHooksOfFiber, ErrorsNames as DebugToolsErrors} from 'react-debug-tools'; +import { + inspectHooksOfFiber, + ErrorsNames as DebugToolsErrors, +} from 'react-debug-tools'; import { patch as patchConsole, registerRenderer as registerRendererWithConsole, @@ -3610,9 +3613,13 @@ export function attach( if (error.name === DebugToolsErrors.RENDER_FUNCTION_ERROR) { let message = 'Error rendering inspected element.'; let stack; + // Log error & cause for user to debug console.error(message + '\n\n', error); if (error.cause != null) { - console.error('Original error causing above error: \n\n', error.cause); + console.error( + 'Original error causing above error: \n\n', + error.cause, + ); if (error.cause instanceof Error) { message = error.cause.message || message; stack = error.cause.stack; @@ -3628,6 +3635,16 @@ export function attach( }; } + if (error.name === DebugToolsErrors.UNSUPPORTTED_FEATURE_ERROR) { + return { + type: 'unsupported-feature', + id, + responseID: requestID, + message: 'Unsupported feature: ' + error.message, + }; + } + + // Log Uncaught Error console.error('Error inspecting element.\n\n', error); return { diff --git a/packages/react-devtools-shared/src/backend/types.js b/packages/react-devtools-shared/src/backend/types.js index 11019d3f0e4f2..4a263baa5250e 100644 --- a/packages/react-devtools-shared/src/backend/types.js +++ b/packages/react-devtools-shared/src/backend/types.js @@ -282,6 +282,7 @@ export type InspectedElement = {| export const InspectElementErrorType = 'error'; export const InspectElementUserErrorType = 'user-error'; +export const InspectElementUnsupportedFeatureErrorType = 'unsupported-feature'; export const InspectElementFullDataType = 'full-data'; export const InspectElementNoChangeType = 'no-change'; export const InspectElementNotFoundType = 'not-found'; @@ -302,6 +303,13 @@ export type InspectElementUserError = {| stack: ?string, |}; +export type InspectElementUnsupportedFeatureError = {| + id: number, + responseID: number, + type: 'unsupported-feature', + message: string, +|}; + export type InspectElementFullData = {| id: number, responseID: number, @@ -332,6 +340,7 @@ export type InspectElementNotFound = {| export type InspectedElementPayload = | InspectElementError | InspectElementUserError + | InspectElementUnsupportedFeatureError | InspectElementFullData | InspectElementHydratedPath | InspectElementNoChange diff --git a/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/CaughtErrorView.js b/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/CaughtErrorView.js new file mode 100644 index 0000000000000..c4511ba0312bd --- /dev/null +++ b/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/CaughtErrorView.js @@ -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 ( +
+ {children} +
+
+
{errorMessage}
+
+ {!!info &&
{info}
} + {!!callStack && ( +
+ The error was thrown {callStack.trim()} +
+ )} +
+
+ ); +} diff --git a/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ErrorBoundary.js b/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ErrorBoundary.js index c770ce1b436ae..694e0ca20512a 100644 --- a/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ErrorBoundary.js +++ b/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ErrorBoundary.js @@ -15,11 +15,13 @@ import ErrorView from './ErrorView'; import SearchingGitHubIssues from './SearchingGitHubIssues'; import SuspendingErrorView from './SuspendingErrorView'; import TimeoutView from './TimeoutView'; -import UserErrorView from './UserErrorView'; +import CaughtErrorView from './CaughtErrorView'; import UnsupportedBridgeOperationError from 'react-devtools-shared/src/UnsupportedBridgeOperationError'; import TimeoutError from 'react-devtools-shared/src/errors/TimeoutError'; import UserError from 'react-devtools-shared/src/errors/UserError'; +import UnsupportedFeatureError from 'react-devtools-shared/src/errors/UnsupportedFeatureError'; import {logEvent} from 'react-devtools-shared/src/Logger'; +import {boolean} from 'yargs'; type Props = {| children: React$Node, @@ -37,6 +39,7 @@ type State = {| isUnsupportedBridgeOperationError: boolean, isTimeout: boolean, isUserError: boolean, + isUnsupportedFeatureError: boolean, |}; const InitialState: State = { @@ -48,6 +51,7 @@ const InitialState: State = { isUnsupportedBridgeOperationError: false, isTimeout: false, isUserError: false, + isUnsupportedFeatureError: false, }; export default class ErrorBoundary extends Component { @@ -63,6 +67,7 @@ export default class ErrorBoundary extends Component { const isTimeout = error instanceof TimeoutError; const isUserError = error instanceof UserError; + const isUnsupportedFeatureError = error instanceof UnsupportedFeatureError; const isUnsupportedBridgeOperationError = error instanceof UnsupportedBridgeOperationError; @@ -81,6 +86,7 @@ export default class ErrorBoundary extends Component { errorMessage, hasError: true, isUnsupportedBridgeOperationError, + isUnsupportedFeatureError, isTimeout, isUserError, }; @@ -118,6 +124,7 @@ export default class ErrorBoundary extends Component { isUnsupportedBridgeOperationError, isTimeout, isUserError, + isUnsupportedFeatureError, } = this.state; if (hasError) { @@ -142,10 +149,34 @@ export default class ErrorBoundary extends Component { ); } else if (isUserError) { return ( - + This is likely to be caused by implementation of current + inspected element. Please see your console for logged error. + + } + /> + ); + } else if (isUnsupportedFeatureError) { + return ( + + React DevTools is unable to handle a feature you are using in + this component (e.g. a new React build-in Hook). Please upgrade + to the latest version. + + } /> ); } else { @@ -156,10 +187,7 @@ export default class ErrorBoundary extends Component { dismissError={ canDismissProp || canDismissState ? this._dismissError : null } - errorMessage={errorMessage} - isUnsupportedBridgeOperationError={ - isUnsupportedBridgeOperationError - }> + errorMessage={errorMessage}> }> - {children} -
-
-
- {errorMessage || 'Error occured in inspected element'} -
-
-
- This is likely to be caused by implementation of current inspected element. -
- {!!callStack && ( -
- The error was thrown {callStack.trim()} -
- )} -
- - ); - } - \ No newline at end of file diff --git a/packages/react-devtools-shared/src/errors/UnsupportedFeatureError.js b/packages/react-devtools-shared/src/errors/UnsupportedFeatureError.js new file mode 100644 index 0000000000000..0f98b0b3760b8 --- /dev/null +++ b/packages/react-devtools-shared/src/errors/UnsupportedFeatureError.js @@ -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 UnsupportedFeatureError 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, UnsupportedFeatureError); + } + + this.name = 'UnsupportedFeatureError'; + } +} diff --git a/packages/react-devtools-shared/src/errors/UserError.js b/packages/react-devtools-shared/src/errors/UserError.js index 752dbf9e88403..81a9ee86e5e16 100644 --- a/packages/react-devtools-shared/src/errors/UserError.js +++ b/packages/react-devtools-shared/src/errors/UserError.js @@ -7,7 +7,7 @@ * @flow */ - export default class UserError extends Error { +export default class UserError extends Error { constructor(message: string) { super(message); diff --git a/packages/react-devtools-shared/src/inspectedElementMutableSource.js b/packages/react-devtools-shared/src/inspectedElementMutableSource.js index 905c4f7bc97fa..43f63381d19c4 100644 --- a/packages/react-devtools-shared/src/inspectedElementMutableSource.js +++ b/packages/react-devtools-shared/src/inspectedElementMutableSource.js @@ -21,6 +21,7 @@ import type {FrontendBridge} from 'react-devtools-shared/src/bridge'; import type { InspectElementError, InspectElementUserError, + InspectElementUnsupportedFeatureError, InspectElementFullData, InspectElementHydratedPath, } from 'react-devtools-shared/src/backend/types'; @@ -30,6 +31,7 @@ import type { InspectedElementResponseType, } from 'react-devtools-shared/src/devtools/views/Components/types'; import UserError from 'react-devtools-shared/src/errors/UserError'; +import UnsupportedFeatureError from 'react-devtools-shared/src/errors/UnsupportedFeatureError'; // Maps element ID to inspected data. // We use an LRU for this rather than a WeakMap because of how the "no-change" optimization works. @@ -101,6 +103,13 @@ export function inspectElement({ throw error; } + case 'unsupported-faeture': { + const {message} = (data: InspectElementUnsupportedFeatureError); + // Trying to keep useful information from user's side. + const error = new UnsupportedFeatureError(message); + throw error; + } + case 'no-change': // This is a no-op for the purposes of our cache. inspectedElement = inspectedElementCache.get(id); From 8fecbd4d9e8c3e726dd76bd126807e908b063ebb Mon Sep 17 00:00:00 2001 From: Mengdi Chen Date: Tue, 29 Mar 2022 11:08:01 -0400 Subject: [PATCH 03/15] fix bad import --- .../src/devtools/views/ErrorBoundary/ErrorBoundary.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ErrorBoundary.js b/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ErrorBoundary.js index 694e0ca20512a..6aa875153c2f8 100644 --- a/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ErrorBoundary.js +++ b/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ErrorBoundary.js @@ -21,7 +21,6 @@ import TimeoutError from 'react-devtools-shared/src/errors/TimeoutError'; import UserError from 'react-devtools-shared/src/errors/UserError'; import UnsupportedFeatureError from 'react-devtools-shared/src/errors/UnsupportedFeatureError'; import {logEvent} from 'react-devtools-shared/src/Logger'; -import {boolean} from 'yargs'; type Props = {| children: React$Node, From 7344311b85072628b3cf5c9ddf47b222199fbeac Mon Sep 17 00:00:00 2001 From: Mengdi Chen Date: Tue, 29 Mar 2022 11:40:11 -0400 Subject: [PATCH 04/15] fix typo --- .../react-devtools-shared/src/inspectedElementMutableSource.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-devtools-shared/src/inspectedElementMutableSource.js b/packages/react-devtools-shared/src/inspectedElementMutableSource.js index 43f63381d19c4..92771df0f76b1 100644 --- a/packages/react-devtools-shared/src/inspectedElementMutableSource.js +++ b/packages/react-devtools-shared/src/inspectedElementMutableSource.js @@ -103,7 +103,7 @@ export function inspectElement({ throw error; } - case 'unsupported-faeture': { + case 'unsupported-feature': { const {message} = (data: InspectElementUnsupportedFeatureError); // Trying to keep useful information from user's side. const error = new UnsupportedFeatureError(message); From 4c34e7e7294e44c7447a5354bff3249d13c31d68 Mon Sep 17 00:00:00 2001 From: Mengdi Chen Date: Tue, 29 Mar 2022 14:10:45 -0400 Subject: [PATCH 05/15] fix issues from rebasing --- packages/react-devtools-shared/src/backend/renderer.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/react-devtools-shared/src/backend/renderer.js b/packages/react-devtools-shared/src/backend/renderer.js index 041b36811807d..9621582fb8d27 100644 --- a/packages/react-devtools-shared/src/backend/renderer.js +++ b/packages/react-devtools-shared/src/backend/renderer.js @@ -62,7 +62,6 @@ import { } from '../constants'; import { inspectHooksOfFiber, - ErrorsNames as DebugToolsErrors, } from 'react-debug-tools'; import { patch as patchConsole, @@ -3610,7 +3609,8 @@ export function attach( try { mostRecentlyInspectedElement = inspectElementRaw(id); } catch (error) { - if (error.name === DebugToolsErrors.RENDER_FUNCTION_ERROR) { + // the error name is synced with ReactDebugHooks + if (error.name === 'RenderFunctionError') { let message = 'Error rendering inspected element.'; let stack; // Log error & cause for user to debug @@ -3635,7 +3635,8 @@ export function attach( }; } - if (error.name === DebugToolsErrors.UNSUPPORTTED_FEATURE_ERROR) { + // the error name is synced with ReactDebugHooks + if (error.name === 'UnsupportedFeatureError') { return { type: 'unsupported-feature', id, From 102ba983b11080614d60f42ce7628fb3bc4e6d5c Mon Sep 17 00:00:00 2001 From: Mengdi Chen Date: Tue, 29 Mar 2022 14:14:45 -0400 Subject: [PATCH 06/15] prettier --- packages/react-devtools-shared/src/backend/renderer.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/react-devtools-shared/src/backend/renderer.js b/packages/react-devtools-shared/src/backend/renderer.js index 9621582fb8d27..d72a97c26df18 100644 --- a/packages/react-devtools-shared/src/backend/renderer.js +++ b/packages/react-devtools-shared/src/backend/renderer.js @@ -60,9 +60,7 @@ import { TREE_OPERATION_UPDATE_ERRORS_OR_WARNINGS, TREE_OPERATION_UPDATE_TREE_BASE_DURATION, } from '../constants'; -import { - inspectHooksOfFiber, -} from 'react-debug-tools'; +import {inspectHooksOfFiber} from 'react-debug-tools'; import { patch as patchConsole, registerRenderer as registerRendererWithConsole, From 84a8557dc24172db94c0c03bea378346c291b56f Mon Sep 17 00:00:00 2001 From: Mengdi Chen Date: Wed, 30 Mar 2022 15:36:26 -0400 Subject: [PATCH 07/15] sync error names --- packages/react-devtools-shared/src/backend/renderer.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react-devtools-shared/src/backend/renderer.js b/packages/react-devtools-shared/src/backend/renderer.js index d72a97c26df18..dc8403722f1f4 100644 --- a/packages/react-devtools-shared/src/backend/renderer.js +++ b/packages/react-devtools-shared/src/backend/renderer.js @@ -3608,7 +3608,7 @@ export function attach( mostRecentlyInspectedElement = inspectElementRaw(id); } catch (error) { // the error name is synced with ReactDebugHooks - if (error.name === 'RenderFunctionError') { + if (error.name === 'ReactDebugToolsRenderFunctionError') { let message = 'Error rendering inspected element.'; let stack; // Log error & cause for user to debug @@ -3634,7 +3634,7 @@ export function attach( } // the error name is synced with ReactDebugHooks - if (error.name === 'UnsupportedFeatureError') { + if (error.name === 'ReactDebugToolsUnsupportedHookError') { return { type: 'unsupported-feature', id, From 15e57f408e5aff65b13983b475de68927c8c2095 Mon Sep 17 00:00:00 2001 From: Mengdi Chen Date: Fri, 1 Apr 2022 14:49:19 -0400 Subject: [PATCH 08/15] sync error name with upstream --- packages/react-devtools-shared/src/backend/renderer.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-devtools-shared/src/backend/renderer.js b/packages/react-devtools-shared/src/backend/renderer.js index dc8403722f1f4..5c27f50e4da55 100644 --- a/packages/react-devtools-shared/src/backend/renderer.js +++ b/packages/react-devtools-shared/src/backend/renderer.js @@ -3608,7 +3608,7 @@ export function attach( mostRecentlyInspectedElement = inspectElementRaw(id); } catch (error) { // the error name is synced with ReactDebugHooks - if (error.name === 'ReactDebugToolsRenderFunctionError') { + if (error.name === 'ReactDebugToolsRenderError') { let message = 'Error rendering inspected element.'; let stack; // Log error & cause for user to debug From 05505f88a7a784e7d9ace4ca821ab4069924ad9d Mon Sep 17 00:00:00 2001 From: Mengdi Chen Date: Fri, 1 Apr 2022 15:00:34 -0400 Subject: [PATCH 09/15] fix lint & better comment --- .../src/inspectedElementMutableSource.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/react-devtools-shared/src/inspectedElementMutableSource.js b/packages/react-devtools-shared/src/inspectedElementMutableSource.js index 92771df0f76b1..00fd0c3045ff4 100644 --- a/packages/react-devtools-shared/src/inspectedElementMutableSource.js +++ b/packages/react-devtools-shared/src/inspectedElementMutableSource.js @@ -8,7 +8,6 @@ */ import LRU from 'lru-cache'; -import {UserHookError} from 'react-debug-tools'; import { convertInspectedElementBackendToFrontend, hydrateHelper, @@ -97,7 +96,7 @@ export function inspectElement({ case 'user-error': { const {message, stack} = (data: InspectElementUserError); - // Trying to keep useful information from user's side. + // Trying to keep useful information from user's component. const error = new UserError(message); error.stack = stack || error.stack; throw error; @@ -105,7 +104,7 @@ export function inspectElement({ case 'unsupported-feature': { const {message} = (data: InspectElementUnsupportedFeatureError); - // Trying to keep useful information from user's side. + // Trying to keep useful information from backend. const error = new UnsupportedFeatureError(message); throw error; } From 1da5e176456f7b7713ba2d79f8434ae7220fdd30 Mon Sep 17 00:00:00 2001 From: Mengdi Chen Date: Fri, 1 Apr 2022 15:21:54 -0400 Subject: [PATCH 10/15] fix error message for test --- .../src/__tests__/inspectedElement-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js b/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js index 81c0a3a758212..919fbb059f76b 100644 --- a/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js +++ b/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js @@ -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'); }); From 3862f3e42c7f3a8459b5ea038f8c969000b02694 Mon Sep 17 00:00:00 2001 From: Mengdi Chen Date: Tue, 12 Apr 2022 11:41:50 -0400 Subject: [PATCH 11/15] better error message per review --- .../src/backend/renderer.js | 11 ++++-- .../src/backend/types.js | 22 ++---------- .../views/ErrorBoundary/ErrorBoundary.js | 25 ++++++-------- .../src/errors/UnsupportedFeatureError.js | 21 ------------ .../src/inspectedElementMutableSource.js | 34 +++++++------------ 5 files changed, 34 insertions(+), 79 deletions(-) delete mode 100644 packages/react-devtools-shared/src/errors/UnsupportedFeatureError.js diff --git a/packages/react-devtools-shared/src/backend/renderer.js b/packages/react-devtools-shared/src/backend/renderer.js index 5c27f50e4da55..5b468f9c2aa38 100644 --- a/packages/react-devtools-shared/src/backend/renderer.js +++ b/packages/react-devtools-shared/src/backend/renderer.js @@ -3615,7 +3615,9 @@ export function attach( console.error(message + '\n\n', error); if (error.cause != null) { console.error( - 'Original error causing above error: \n\n', + 'React DevTools encountered an error while trying to inspect hooks. ' + + 'This is most likely caused by an error in current inspected component.' + + 'The error thrown in the component is: \n\n', error.cause, ); if (error.cause instanceof Error) { @@ -3625,7 +3627,8 @@ export function attach( } return { - type: 'user-error', + type: 'error', + errorType: 'user', id, responseID: requestID, message, @@ -3636,7 +3639,8 @@ export function attach( // the error name is synced with ReactDebugHooks if (error.name === 'ReactDebugToolsUnsupportedHookError') { return { - type: 'unsupported-feature', + type: 'error', + errorType: 'unknown-hook', id, responseID: requestID, message: 'Unsupported feature: ' + error.message, @@ -3648,6 +3652,7 @@ export function attach( return { type: 'error', + errorType: 'uncaught', id, responseID: requestID, message: error.message, diff --git a/packages/react-devtools-shared/src/backend/types.js b/packages/react-devtools-shared/src/backend/types.js index 4a263baa5250e..92ad42c3010cf 100644 --- a/packages/react-devtools-shared/src/backend/types.js +++ b/packages/react-devtools-shared/src/backend/types.js @@ -281,8 +281,6 @@ export type InspectedElement = {| |}; export const InspectElementErrorType = 'error'; -export const InspectElementUserErrorType = 'user-error'; -export const InspectElementUnsupportedFeatureErrorType = 'unsupported-feature'; export const InspectElementFullDataType = 'full-data'; export const InspectElementNoChangeType = 'no-change'; export const InspectElementNotFoundType = 'not-found'; @@ -291,23 +289,9 @@ export type InspectElementError = {| id: number, responseID: number, type: 'error', + errorType: 'user' | 'unknown-hook' | 'uncaught', message: string, - stack: string, -|}; - -export type InspectElementUserError = {| - id: number, - responseID: number, - type: 'user-error', - message: string, - stack: ?string, -|}; - -export type InspectElementUnsupportedFeatureError = {| - id: number, - responseID: number, - type: 'unsupported-feature', - message: string, + stack?: string, |}; export type InspectElementFullData = {| @@ -339,8 +323,6 @@ export type InspectElementNotFound = {| export type InspectedElementPayload = | InspectElementError - | InspectElementUserError - | InspectElementUnsupportedFeatureError | InspectElementFullData | InspectElementHydratedPath | InspectElementNoChange diff --git a/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ErrorBoundary.js b/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ErrorBoundary.js index 6aa875153c2f8..5cf5a823d7589 100644 --- a/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ErrorBoundary.js +++ b/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ErrorBoundary.js @@ -19,7 +19,7 @@ import CaughtErrorView from './CaughtErrorView'; import UnsupportedBridgeOperationError from 'react-devtools-shared/src/UnsupportedBridgeOperationError'; import TimeoutError from 'react-devtools-shared/src/errors/TimeoutError'; import UserError from 'react-devtools-shared/src/errors/UserError'; -import UnsupportedFeatureError from 'react-devtools-shared/src/errors/UnsupportedFeatureError'; +import UnknownHookError from 'react-devtools-shared/src/errors/UnknownHookError'; import {logEvent} from 'react-devtools-shared/src/Logger'; type Props = {| @@ -38,7 +38,7 @@ type State = {| isUnsupportedBridgeOperationError: boolean, isTimeout: boolean, isUserError: boolean, - isUnsupportedFeatureError: boolean, + isUnknownHookError: boolean, |}; const InitialState: State = { @@ -50,7 +50,7 @@ const InitialState: State = { isUnsupportedBridgeOperationError: false, isTimeout: false, isUserError: false, - isUnsupportedFeatureError: false, + isUnknownHookError: false, }; export default class ErrorBoundary extends Component { @@ -66,7 +66,7 @@ export default class ErrorBoundary extends Component { const isTimeout = error instanceof TimeoutError; const isUserError = error instanceof UserError; - const isUnsupportedFeatureError = error instanceof UnsupportedFeatureError; + const isUnknownHookError = error instanceof UnknownHookError; const isUnsupportedBridgeOperationError = error instanceof UnsupportedBridgeOperationError; @@ -85,7 +85,7 @@ export default class ErrorBoundary extends Component { errorMessage, hasError: true, isUnsupportedBridgeOperationError, - isUnsupportedFeatureError, + isUnknownHookError, isTimeout, isUserError, }; @@ -123,7 +123,7 @@ export default class ErrorBoundary extends Component { isUnsupportedBridgeOperationError, isTimeout, isUserError, - isUnsupportedFeatureError, + isUnknownHookError, } = this.state; if (hasError) { @@ -160,20 +160,17 @@ export default class ErrorBoundary extends Component { } /> ); - } else if (isUnsupportedFeatureError) { + } else if (isUnknownHookError) { return ( - React DevTools is unable to handle a feature you are using in - this component (e.g. a new React build-in Hook). Please upgrade - to the latest version. + React DevTools encountered an unknown hook. This is probably + because the package is out of date. To fix, upgrade the package + to the most recent version. } /> diff --git a/packages/react-devtools-shared/src/errors/UnsupportedFeatureError.js b/packages/react-devtools-shared/src/errors/UnsupportedFeatureError.js deleted file mode 100644 index 0f98b0b3760b8..0000000000000 --- a/packages/react-devtools-shared/src/errors/UnsupportedFeatureError.js +++ /dev/null @@ -1,21 +0,0 @@ -/** - * 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 UnsupportedFeatureError 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, UnsupportedFeatureError); - } - - this.name = 'UnsupportedFeatureError'; - } -} diff --git a/packages/react-devtools-shared/src/inspectedElementMutableSource.js b/packages/react-devtools-shared/src/inspectedElementMutableSource.js index 00fd0c3045ff4..bb687bf98d7ac 100644 --- a/packages/react-devtools-shared/src/inspectedElementMutableSource.js +++ b/packages/react-devtools-shared/src/inspectedElementMutableSource.js @@ -19,8 +19,6 @@ import type {LRUCache} from 'react-devtools-shared/src/types'; import type {FrontendBridge} from 'react-devtools-shared/src/bridge'; import type { InspectElementError, - InspectElementUserError, - InspectElementUnsupportedFeatureError, InspectElementFullData, InspectElementHydratedPath, } from 'react-devtools-shared/src/backend/types'; @@ -30,7 +28,7 @@ import type { InspectedElementResponseType, } from 'react-devtools-shared/src/devtools/views/Components/types'; import UserError from 'react-devtools-shared/src/errors/UserError'; -import UnsupportedFeatureError from 'react-devtools-shared/src/errors/UnsupportedFeatureError'; +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. @@ -85,27 +83,21 @@ export function inspectElement({ let inspectedElement; switch (type) { case 'error': { - const {message, stack} = ((data: any): InspectElementError); - + 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; - - throw error; - } - - case 'user-error': { - const {message, stack} = (data: InspectElementUserError); - // Trying to keep useful information from user's component. - const error = new UserError(message); error.stack = stack || error.stack; - throw error; - } - case 'unsupported-feature': { - const {message} = (data: InspectElementUnsupportedFeatureError); - // Trying to keep useful information from backend. - const error = new UnsupportedFeatureError(message); throw error; } From b7ecfdf8b9b9cdd88b3375be8a79123f9a2b2ee7 Mon Sep 17 00:00:00 2001 From: Mengdi Chen Date: Tue, 12 Apr 2022 15:00:30 -0400 Subject: [PATCH 12/15] add missing file --- .../src/errors/UnknownHookError.js | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 packages/react-devtools-shared/src/errors/UnknownHookError.js diff --git a/packages/react-devtools-shared/src/errors/UnknownHookError.js b/packages/react-devtools-shared/src/errors/UnknownHookError.js new file mode 100644 index 0000000000000..c00be01bb4e8a --- /dev/null +++ b/packages/react-devtools-shared/src/errors/UnknownHookError.js @@ -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'; + } +} From 5edaca45604b78897306d9c18242809f2c7eb707 Mon Sep 17 00:00:00 2001 From: Mengdi Chen Date: Thu, 5 May 2022 15:55:27 -0400 Subject: [PATCH 13/15] remove dead enum & provide component name in error message --- .../react-devtools-shared/src/backend/renderer.js | 12 +++++++++--- .../src/devtools/views/Components/types.js | 1 - 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/packages/react-devtools-shared/src/backend/renderer.js b/packages/react-devtools-shared/src/backend/renderer.js index 5b468f9c2aa38..5e09a2d0f8043 100644 --- a/packages/react-devtools-shared/src/backend/renderer.js +++ b/packages/react-devtools-shared/src/backend/renderer.js @@ -3614,10 +3614,14 @@ export function attach( // 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.' + - 'The error thrown in the component is: \n\n', + '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) { @@ -3643,7 +3647,9 @@ export function attach( errorType: 'unknown-hook', id, responseID: requestID, - message: 'Unsupported feature: ' + error.message, + message: + 'Unsupported hook in the react-debug-tools package: ' + + error.message, }; } diff --git a/packages/react-devtools-shared/src/devtools/views/Components/types.js b/packages/react-devtools-shared/src/devtools/views/Components/types.js index d6918a0813606..c6789fa1e9ec1 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/types.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/types.js @@ -63,7 +63,6 @@ export type OwnersList = {| export type InspectedElementResponseType = | 'error' - | 'user-error' | 'full-data' | 'hydrated-path' | 'no-change' From 2eda8672b53dc572ad43bca14a61516a3ae27463 Mon Sep 17 00:00:00 2001 From: Mengdi Chen Date: Thu, 5 May 2022 16:12:35 -0400 Subject: [PATCH 14/15] better error message --- .../src/devtools/views/ErrorBoundary/ErrorBoundary.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ErrorBoundary.js b/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ErrorBoundary.js index 5cf5a823d7589..5e7ca9d166797 100644 --- a/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ErrorBoundary.js +++ b/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ErrorBoundary.js @@ -169,8 +169,8 @@ export default class ErrorBoundary extends Component { info={ <> React DevTools encountered an unknown hook. This is probably - because the package is out of date. To fix, upgrade the package - to the most recent version. + because the react-debug-tools package is out of date. To fix, + upgrade the React DevTools to the most recent version. } /> From 65a2c902b464897ad19c54f751caa8112be9dfd0 Mon Sep 17 00:00:00 2001 From: Mengdi Chen Date: Thu, 5 May 2022 17:38:10 -0400 Subject: [PATCH 15/15] better user facing error message --- .../src/devtools/views/ErrorBoundary/ErrorBoundary.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ErrorBoundary.js b/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ErrorBoundary.js index 5e7ca9d166797..d207fcfe4f736 100644 --- a/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ErrorBoundary.js +++ b/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ErrorBoundary.js @@ -154,8 +154,10 @@ export default class ErrorBoundary extends Component { errorMessage={errorMessage || 'Error occured in inspected element'} info={ <> - This is likely to be caused by implementation of current - inspected element. Please see your console for logged error. + 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. } />