better error message per review

facebook · Apr 12, 2022 · e9aef87 · e9aef87
1 parent 9613f9a
commit e9aef87
Show file tree

Hide file tree

Showing 5 changed files with 34 additions and 79 deletions.
diff --git a/packages/react-devtools-shared/src/backend/renderer.js b/packages/react-devtools-shared/src/backend/renderer.js
@@ -3614,7 +3614,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) {
@@ -3624,7 +3626,8 @@ export function attach(
         }
 
         return {
-          type: 'user-error',
+          type: 'error',
+          errorType: 'user',
           id,
           responseID: requestID,
           message,
@@ -3635,7 +3638,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,
@@ -3647,6 +3651,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
@@ -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
@@ -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<Props, State> {
@@ -66,7 +66,7 @@ export default class ErrorBoundary extends Component<Props, State> {
 
     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<Props, State> {
       errorMessage,
       hasError: true,
       isUnsupportedBridgeOperationError,
-      isUnsupportedFeatureError,
+      isUnknownHookError,
       isTimeout,
       isUserError,
     };
@@ -123,7 +123,7 @@ export default class ErrorBoundary extends Component<Props, State> {
       isUnsupportedBridgeOperationError,
       isTimeout,
       isUserError,
-      isUnsupportedFeatureError,
+      isUnknownHookError,
     } = this.state;
 
     if (hasError) {
@@ -160,20 +160,17 @@ export default class ErrorBoundary extends Component<Props, State> {
             }
           />
         );
-      } else if (isUnsupportedFeatureError) {
+      } else if (isUnknownHookError) {
         return (
           <CaughtErrorView
             callStack={callStack}
             componentStack={componentStack}
-            errorMessage={
-              errorMessage ||
-              'Current DevTools version does not support a feature used in the inspected element.'
-            }
+            errorMessage={errorMessage || 'Encountered an unknown hook'}
             info={
               <>
-                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
diff --git 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;
       }