[ReactDebugTools] wrap uncaught error from rendering user's component (#24216)

mondaychen · web-flow · commit c89a15c716fe · 2022-04-01T14:38:11.000-04:00
* [ReactDebugTools] wrap uncaught error from rendering user's component

* fix lint

* make error names more package specific

* update per review comments

* fix tests

* fix lint

* fix tests

* fix lint

* fix error name &amp; nits

* try catch instead of mocking error

* fix test for older node.js version

* avoid false positive from try-catch in tests
diff --git a/packages/react-debug-tools/src/ReactDebugHooks.js b/packages/react-debug-tools/src/ReactDebugHooks.js
@@ -366,7 +366,7 @@ const DispatcherProxyHandler = {
     const error = new Error('Missing method in Dispatcher: ' + prop);
     // Note: This error name needs to stay in sync with react-devtools-shared
     // TODO: refactor this if we ever combine the devtools and debug tools packages
-    error.name = 'UnsupportedFeatureError';
+    error.name = 'ReactDebugToolsUnsupportedHookError';
     throw error;
   },
 };
@@ -667,6 +667,30 @@ function processDebugValues(
   }
 }
 
+function handleRenderFunctionError(error: any): void {
+  // original error might be any type.
+  if (
+    error instanceof Error &&
+    error.name === 'ReactDebugToolsUnsupportedHookError'
+  ) {
+    throw error;
+  }
+  // If the error is not caused by an unsupported feature, it means
+  // that the error is caused by user's code in renderFunction.
+  // In this case, we should wrap the original error inside a custom error
+  // so that devtools can give a clear message about it.
+  // $FlowFixMe: Flow doesn't know about 2nd argument of Error constructor
+  const wrapperError = new Error('Error rendering inspected component', {
+    cause: error,
+  });
+  // Note: This error name needs to stay in sync with react-devtools-shared
+  // TODO: refactor this if we ever combine the devtools and debug tools packages
+  wrapperError.name = 'ReactDebugToolsRenderError';
+  // this stage-4 proposal is not supported by all environments yet.
+  wrapperError.cause = error;
+  throw wrapperError;
+}
+
 export function inspectHooks<Props>(
   renderFunction: Props => React$Node,
   props: Props,
@@ -686,6 +710,8 @@ export function inspectHooks<Props>(
   try {
     ancestorStackError = new Error();
     renderFunction(props);
+  } catch (error) {
+    handleRenderFunctionError(error);
   } finally {
     readHookLog = hookLog;
     hookLog = [];
@@ -730,6 +756,8 @@ function inspectHooksOfForwardRef<Props, Ref>(
   try {
     ancestorStackError = new Error();
     renderFunction(props, ref);
+  } catch (error) {
+    handleRenderFunctionError(error);
   } finally {
     readHookLog = hookLog;
     hookLog = [];
diff --git a/packages/react-debug-tools/src/__tests__/ReactHooksInspection-test.js b/packages/react-debug-tools/src/__tests__/ReactHooksInspection-test.js
@@ -276,10 +276,20 @@ describe('ReactHooksInspection', () => {
       },
     };
 
+    let didCatch = false;
     expect(() => {
-      expect(() => {
+      // mock the Error constructor to check the internal of the error instance
+      try {
         ReactDebugTools.inspectHooks(Foo, {}, FakeDispatcherRef);
-      }).toThrow("Cannot read property 'useState' of null");
+      } catch (error) {
+        expect(error.message).toBe('Error rendering inspected component');
+        // error.cause is the original error
+        expect(error.cause).toBeInstanceOf(Error);
+        expect(error.cause.message).toBe(
+          "Cannot read property 'useState' of null",
+        );
+      }
+      didCatch = true;
     }).toErrorDev(
       'Invalid hook call. Hooks can only be called inside of the body of a function component. This could happen for' +
         ' one of the following reasons:\n' +
@@ -289,6 +299,8 @@ describe('ReactHooksInspection', () => {
         'See https://reactjs.org/link/invalid-hook-call for tips about how to debug and fix this problem.',
       {withoutStack: true},
     );
+    // avoid false positive if no error was thrown at all
+    expect(didCatch).toBe(true);
 
     expect(getterCalls).toBe(1);
     expect(setterCalls).toHaveLength(2);
diff --git a/packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.js b/packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.js
@@ -920,16 +920,26 @@ describe('ReactHooksInspectionIntegration', () => {
 
     const renderer = ReactTestRenderer.create(<Foo />);
     const childFiber = renderer.root._currentFiber();
-    expect(() => {
+
+    let didCatch = false;
+
+    try {
       ReactDebugTools.inspectHooksOfFiber(childFiber, FakeDispatcherRef);
-    }).toThrow(
-      'Invalid hook call. Hooks can only be called inside of the body of a function component. This could happen for' +
-        ' one of the following reasons:\n' +
-        '1. You might have mismatching versions of React and the renderer (such as React DOM)\n' +
-        '2. You might be breaking the Rules of Hooks\n' +
-        '3. You might have more than one copy of React in the same app\n' +
-        'See https://reactjs.org/link/invalid-hook-call for tips about how to debug and fix this problem.',
-    );
+    } catch (error) {
+      expect(error.message).toBe('Error rendering inspected component');
+      expect(error.cause).toBeInstanceOf(Error);
+      expect(error.cause.message).toBe(
+        'Invalid hook call. Hooks can only be called inside of the body of a function component. This could happen for' +
+          ' one of the following reasons:\n' +
+          '1. You might have mismatching versions of React and the renderer (such as React DOM)\n' +
+          '2. You might be breaking the Rules of Hooks\n' +
+          '3. You might have more than one copy of React in the same app\n' +
+          'See https://reactjs.org/link/invalid-hook-call for tips about how to debug and fix this problem.',
+      );
+      didCatch = true;
+    }
+    // avoid false positive if no error was thrown at all
+    expect(didCatch).toBe(true);
 
     expect(getterCalls).toBe(1);
     expect(setterCalls).toHaveLength(2);
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('Expected');
+    expect(error.message).toBe('Error rendering inspected component');
     expect(error.stack).toContain('inspectHooksOfFiber');
   });
 
