@samitouri / QOS-React-2 / commits / c89a15c716

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

* [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 & nits * try catch instead of mocking error * fix test for older node.js version * avoid false positive from try-catch in tests

Mengdi "Monday" Chen committed Apr 1, 2022 at 14:38 UTC c89a15c716febe71b7d857f839829cd1dc74918f
4 files changed +63 -13
packages/react-debug-tools/src/ReactDebugHooks.js
+29 -1
@@ -366,7 +366,7 @@ const DispatcherProxyHandler = {
366 const error = new Error('Missing method in Dispatcher: ' + prop);
367 // Note: This error name needs to stay in sync with react-devtools-shared
368 // TODO: refactor this if we ever combine the devtools and debug tools packages
369 - error.name = 'UnsupportedFeatureError';
369 + error.name = 'ReactDebugToolsUnsupportedHookError';
370 throw error;
371 },
372 };
@@ -667,6 +667,30 @@ function processDebugValues(
667 }
668 }
669
670 +function handleRenderFunctionError(error: any): void {
671 + // original error might be any type.
672 + if (
673 + error instanceof Error &&
674 + error.name === 'ReactDebugToolsUnsupportedHookError'
675 + ) {
676 + throw error;
677 + }
678 + // If the error is not caused by an unsupported feature, it means
679 + // that the error is caused by user's code in renderFunction.
680 + // In this case, we should wrap the original error inside a custom error
681 + // so that devtools can give a clear message about it.
682 + // $FlowFixMe: Flow doesn't know about 2nd argument of Error constructor
683 + const wrapperError = new Error('Error rendering inspected component', {
684 + cause: error,
685 + });
686 + // Note: This error name needs to stay in sync with react-devtools-shared
687 + // TODO: refactor this if we ever combine the devtools and debug tools packages
688 + wrapperError.name = 'ReactDebugToolsRenderError';
689 + // this stage-4 proposal is not supported by all environments yet.
690 + wrapperError.cause = error;
691 + throw wrapperError;
692 +}
693 +
694 export function inspectHooks<Props>(
695 renderFunction: Props => React$Node,
696 props: Props,
@@ -686,6 +710,8 @@ export function inspectHooks<Props>(
710 try {
711 ancestorStackError = new Error();
712 renderFunction(props);
713 + } catch (error) {
714 + handleRenderFunctionError(error);
715 } finally {
716 readHookLog = hookLog;
717 hookLog = [];
@@ -730,6 +756,8 @@ function inspectHooksOfForwardRef<Props, Ref>(
756 try {
757 ancestorStackError = new Error();
758 renderFunction(props, ref);
759 + } catch (error) {
760 + handleRenderFunctionError(error);
761 } finally {
762 readHookLog = hookLog;
763 hookLog = [];
packages/react-debug-tools/src/__tests__/ReactHooksInspection-test.js
+14 -2
@@ -276,10 +276,20 @@ describe('ReactHooksInspection', () => {
276 },
277 };
278
279 + let didCatch = false;
280 expect(() => {
280 - expect(() => {
281 + // mock the Error constructor to check the internal of the error instance
282 + try {
283 ReactDebugTools.inspectHooks(Foo, {}, FakeDispatcherRef);
282 - }).toThrow("Cannot read property 'useState' of null");
284 + } catch (error) {
285 + expect(error.message).toBe('Error rendering inspected component');
286 + // error.cause is the original error
287 + expect(error.cause).toBeInstanceOf(Error);
288 + expect(error.cause.message).toBe(
289 + "Cannot read property 'useState' of null",
290 + );
291 + }
292 + didCatch = true;
293 }).toErrorDev(
294 'Invalid hook call. Hooks can only be called inside of the body of a function component. This could happen for' +
295 ' one of the following reasons:\n' +
@@ -289,6 +299,8 @@ describe('ReactHooksInspection', () => {
299 'See https://reactjs.org/link/invalid-hook-call for tips about how to debug and fix this problem.',
300 {withoutStack: true},
301 );
302 + // avoid false positive if no error was thrown at all
303 + expect(didCatch).toBe(true);
304
305 expect(getterCalls).toBe(1);
306 expect(setterCalls).toHaveLength(2);
packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.js
+19 -9
@@ -920,16 +920,26 @@ describe('ReactHooksInspectionIntegration', () => {
920
921 const renderer = ReactTestRenderer.create(<Foo />);
922 const childFiber = renderer.root._currentFiber();
923 - expect(() => {
923 +
924 + let didCatch = false;
925 +
926 + try {
927 ReactDebugTools.inspectHooksOfFiber(childFiber, FakeDispatcherRef);
925 - }).toThrow(
926 - 'Invalid hook call. Hooks can only be called inside of the body of a function component. This could happen for' +
927 - ' one of the following reasons:\n' +
928 - '1. You might have mismatching versions of React and the renderer (such as React DOM)\n' +
929 - '2. You might be breaking the Rules of Hooks\n' +
930 - '3. You might have more than one copy of React in the same app\n' +
931 - 'See https://reactjs.org/link/invalid-hook-call for tips about how to debug and fix this problem.',
932 - );
928 + } catch (error) {
929 + expect(error.message).toBe('Error rendering inspected component');
930 + expect(error.cause).toBeInstanceOf(Error);
931 + expect(error.cause.message).toBe(
932 + 'Invalid hook call. Hooks can only be called inside of the body of a function component. This could happen for' +
933 + ' one of the following reasons:\n' +
934 + '1. You might have mismatching versions of React and the renderer (such as React DOM)\n' +
935 + '2. You might be breaking the Rules of Hooks\n' +
936 + '3. You might have more than one copy of React in the same app\n' +
937 + 'See https://reactjs.org/link/invalid-hook-call for tips about how to debug and fix this problem.',
938 + );
939 + didCatch = true;
940 + }
941 + // avoid false positive if no error was thrown at all
942 + expect(didCatch).toBe(true);
943
944 expect(getterCalls).toBe(1);
945 expect(setterCalls).toHaveLength(2);
packages/react-devtools-shared/src/__tests__/inspectedElement-test.js
+1 -1
@@ -2145,7 +2145,7 @@ describe('InspectedElement', () => {
2145 expect(value).toBe(null);
2146
2147 const error = errorBoundaryInstance.state.error;
2148 - expect(error.message).toBe('Expected');
2148 + expect(error.message).toBe('Error rendering inspected component');
2149 expect(error.stack).toContain('inspectHooksOfFiber');
2150 });
2151