Skip to content

Commit f98d035

Browse files
mondaychenrickhanlonii
authored andcommitted
[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
1 parent a96c3fd commit f98d035

File tree

4 files changed

+63
-13
lines changed

4 files changed

+63
-13
lines changed

packages/react-debug-tools/src/ReactDebugHooks.js

+29-1
Original file line numberDiff line numberDiff line change
@@ -366,7 +366,7 @@ const DispatcherProxyHandler = {
366366
const error = new Error('Missing method in Dispatcher: ' + prop);
367367
// Note: This error name needs to stay in sync with react-devtools-shared
368368
// TODO: refactor this if we ever combine the devtools and debug tools packages
369-
error.name = 'UnsupportedFeatureError';
369+
error.name = 'ReactDebugToolsUnsupportedHookError';
370370
throw error;
371371
},
372372
};
@@ -667,6 +667,30 @@ function processDebugValues(
667667
}
668668
}
669669

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+
670694
export function inspectHooks<Props>(
671695
renderFunction: Props => React$Node,
672696
props: Props,
@@ -686,6 +710,8 @@ export function inspectHooks<Props>(
686710
try {
687711
ancestorStackError = new Error();
688712
renderFunction(props);
713+
} catch (error) {
714+
handleRenderFunctionError(error);
689715
} finally {
690716
readHookLog = hookLog;
691717
hookLog = [];
@@ -730,6 +756,8 @@ function inspectHooksOfForwardRef<Props, Ref>(
730756
try {
731757
ancestorStackError = new Error();
732758
renderFunction(props, ref);
759+
} catch (error) {
760+
handleRenderFunctionError(error);
733761
} finally {
734762
readHookLog = hookLog;
735763
hookLog = [];

packages/react-debug-tools/src/__tests__/ReactHooksInspection-test.js

+14-2
Original file line numberDiff line numberDiff line change
@@ -276,10 +276,20 @@ describe('ReactHooksInspection', () => {
276276
},
277277
};
278278

279+
let didCatch = false;
279280
expect(() => {
280-
expect(() => {
281+
// mock the Error constructor to check the internal of the error instance
282+
try {
281283
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;
283293
}).toErrorDev(
284294
'Invalid hook call. Hooks can only be called inside of the body of a function component. This could happen for' +
285295
' one of the following reasons:\n' +
@@ -289,6 +299,8 @@ describe('ReactHooksInspection', () => {
289299
'See https://reactjs.org/link/invalid-hook-call for tips about how to debug and fix this problem.',
290300
{withoutStack: true},
291301
);
302+
// avoid false positive if no error was thrown at all
303+
expect(didCatch).toBe(true);
292304

293305
expect(getterCalls).toBe(1);
294306
expect(setterCalls).toHaveLength(2);

packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.js

+19-9
Original file line numberDiff line numberDiff line change
@@ -920,16 +920,26 @@ describe('ReactHooksInspectionIntegration', () => {
920920

921921
const renderer = ReactTestRenderer.create(<Foo />);
922922
const childFiber = renderer.root._currentFiber();
923-
expect(() => {
923+
924+
let didCatch = false;
925+
926+
try {
924927
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);
933943

934944
expect(getterCalls).toBe(1);
935945
expect(setterCalls).toHaveLength(2);

packages/react-devtools-shared/src/__tests__/inspectedElement-test.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -2145,7 +2145,7 @@ describe('InspectedElement', () => {
21452145
expect(value).toBe(null);
21462146

21472147
const error = errorBoundaryInstance.state.error;
2148-
expect(error.message).toBe('Expected');
2148+
expect(error.message).toBe('Error rendering inspected component');
21492149
expect(error.stack).toContain('inspectHooksOfFiber');
21502150
});
21512151

0 commit comments

Comments
 (0)