Skip to content

Commit

Permalink
[ReactDebugTools] wrap uncaught error from rendering user's component (
Browse files Browse the repository at this point in the history
…#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
  • Loading branch information
mondaychen authored and rickhanlonii committed Apr 14, 2022
1 parent 5527acd commit 636ca9c
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 13 deletions.
30 changes: 29 additions & 1 deletion packages/react-debug-tools/src/ReactDebugHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
},
};
Expand Down Expand Up @@ -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,
Expand All @@ -686,6 +710,8 @@ export function inspectHooks<Props>(
try {
ancestorStackError = new Error();
renderFunction(props);
} catch (error) {
handleRenderFunctionError(error);
} finally {
readHookLog = hookLog;
hookLog = [];
Expand Down Expand Up @@ -730,6 +756,8 @@ function inspectHooksOfForwardRef<Props, Ref>(
try {
ancestorStackError = new Error();
renderFunction(props, ref);
} catch (error) {
handleRenderFunctionError(error);
} finally {
readHookLog = hookLog;
hookLog = [];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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' +
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});

Expand Down

0 comments on commit 636ca9c

Please sign in to comment.