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

mondaychen · web-flow · commit e531a4a62d0b · 2022-05-05T20:17:23.000-04:00
* [ReactDevTools] custom view for errors occur in user's code

* [ReactDevTools] show message for unsupported feature

* fix bad import

* fix typo

* fix issues from rebasing

* prettier

* sync error names

* sync error name with upstream

* fix lint &amp; better comment

* fix error message for test

* better error message per review

* add missing file

* remove dead enum &amp; provide component name in error message

* better error message

* better user facing error message
diff --git 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');
   });
 
diff --git a/packages/react-devtools-shared/src/backend/renderer.js b/packages/react-devtools-shared/src/backend/renderer.js
@@ -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.';
+        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,
diff --git a/packages/react-devtools-shared/src/backend/types.js b/packages/react-devtools-shared/src/backend/types.js
@@ -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 = {|
diff --git 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/ErrorBoundary/CaughtErrorView.js 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 (
+    <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>
+  );
+}
diff --git 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,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 = {|
@@ -34,6 +37,8 @@ type State = {|
   hasError: boolean,
   isUnsupportedBridgeOperationError: boolean,
   isTimeout: boolean,
+  isUserError: boolean,
+  isUnknownHookError: boolean,
 |};
 
 const InitialState: State = {
@@ -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> {
@@ -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;
 
@@ -76,7 +85,9 @@ export default class ErrorBoundary extends Component<Props, State> {
       errorMessage,
       hasError: true,
       isUnsupportedBridgeOperationError,
+      isUnknownHookError,
       isTimeout,
+      isUserError,
     };
   }
 
@@ -111,6 +122,8 @@ export default class ErrorBoundary extends Component<Props, State> {
       hasError,
       isUnsupportedBridgeOperationError,
       isTimeout,
+      isUserError,
+      isUnknownHookError,
     } = this.state;
 
     if (hasError) {
@@ -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={
+              <>
+                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
@@ -141,10 +185,7 @@ export default class ErrorBoundary extends Component<Props, State> {
             dismissError={
               canDismissProp || canDismissState ? this._dismissError : null
             }
-            errorMessage={errorMessage}
-            isUnsupportedBridgeOperationError={
-              isUnsupportedBridgeOperationError
-            }>
+            errorMessage={errorMessage}>
             <Suspense fallback={<SearchingGitHubIssues />}>
               <SuspendingErrorView
                 callStack={callStack}
diff --git a/packages/react-devtools-shared/src/errors/TimeoutError.js b/packages/react-devtools-shared/src/errors/TimeoutError.js
diff --git a/packages/react-devtools-shared/src/errors/UnknownHookError.js 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';
+  }
+}
diff --git a/packages/react-devtools-shared/src/errors/UserError.js 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
@@ -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.
@@ -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.
