Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ReactDevTools] add custom error view for caught error #2

Closed
wants to merge 17 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 0 additions & 11 deletions .github/ISSUE_TEMPLATE/react_18.md

This file was deleted.

104 changes: 52 additions & 52 deletions CHANGELOG.md

Large diffs are not rendered by default.

16 changes: 8 additions & 8 deletions ReactVersions.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,26 +18,26 @@
//
// 0.0.0-experimental-241c4467e-20200129

const ReactVersion = '18.0.0';
const ReactVersion = '18.1.0';

// The label used by the @next channel. Represents the upcoming release's
// stability. Could be "alpha", "beta", "rc", etc.
const nextChannelLabel = 'next';

const stablePackages = {
'create-subscription': ReactVersion,
'eslint-plugin-react-hooks': '4.4.0',
'jest-react': '0.12.1',
'eslint-plugin-react-hooks': '4.5.0',
'jest-react': '0.13.1',
react: ReactVersion,
'react-art': ReactVersion,
'react-dom': ReactVersion,
'react-is': ReactVersion,
'react-reconciler': '0.27.0',
'react-refresh': '0.12.0',
'react-reconciler': '0.28.0',
'react-refresh': '0.13.0',
'react-test-renderer': ReactVersion,
'use-subscription': '1.6.0',
'use-sync-external-store': '1.0.0',
scheduler: '0.21.0',
'use-subscription': '1.7.0',
'use-sync-external-store': '1.1.0',
scheduler: '0.22.0',
};

// These packages do not exist in the @next or @latest channel, only
Expand Down
30 changes: 30 additions & 0 deletions fixtures/ssr2/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# SSR Fixtures

A set of test cases for quickly identifying issues with server-side rendering.

## Setup

To reference a local build of React, first run `npm run build` at the root
of the React project. Then:

```
cd fixtures/ssr2
yarn
yarn start
```

The `start` command runs a webpack dev server and a server-side rendering server in development mode with hot reloading.

**Note: whenever you make changes to React and rebuild it, you need to re-run `yarn` in this folder:**

```
yarn
```

If you want to try the production mode instead run:

```
yarn start:prod
```

This will pre-build all static resources and then start a server-side rendering HTTP server that hosts the React app and service the static resources (without hot reloading).
1 change: 0 additions & 1 deletion fixtures/ssr2/scripts/build.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ webpack(
console.error(err.details);
}
process.exit(1);
return;
}
const info = stats.toJson();
if (stats.hasErrors()) {
Expand Down
8 changes: 6 additions & 2 deletions fixtures/ssr2/server/render.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ let assets = {
};

module.exports = function render(url, res) {
const data = createServerData();
// This is how you would wire it up previously:
//
// res.send(
Expand All @@ -36,14 +37,17 @@ module.exports = function render(url, res) {
console.error('Fatal', error);
});
let didError = false;
const data = createServerData();
const {pipe, abort} = renderToPipeableStream(
<DataProvider data={data}>
<App assets={assets} />
</DataProvider>,
{
bootstrapScripts: [assets['main.js']],
onCompleteShell() {
onAllReady() {
// Full completion.
// You can use this for SSG or crawlers.
},
onShellReady() {
// If something errored before we started streaming, we set the error code appropriately.
res.statusCode = didError ? 500 : 200;
res.setHeader('Content-type', 'text/html');
Expand Down
4 changes: 4 additions & 0 deletions packages/eslint-plugin-react-hooks/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 4.4.0

* No changes, this was an automated release together with React 18.

## 4.3.0

* Support ESLint 8. ([@MichaelDeBoey](https:/MichaelDeBoey) in [#22248](https:/facebook/react/pull/22248))
Expand Down
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
37 changes: 37 additions & 0 deletions packages/react-devtools-shared/src/backend/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -3606,6 +3606,43 @@ export function attach(
try {
mostRecentlyInspectedElement = inspectElementRaw(id);
} catch (error) {
// the error name is synced with ReactDebugHooks
if (error.name === 'ReactDebugToolsRenderError') {
let message = 'Error rendering inspected element.';
let stack;
// Log error & cause for user to debug
console.error(message + '\n\n', error);
if (error.cause != null) {
console.error(
'Original error causing above error: \n\n',
error.cause,
);
if (error.cause instanceof Error) {
message = error.cause.message || message;
stack = error.cause.stack;
}
}

return {
type: 'user-error',
id,
responseID: requestID,
message,
stack,
};
}

// the error name is synced with ReactDebugHooks
if (error.name === 'ReactDebugToolsUnsupportedHookError') {
return {
type: 'unsupported-feature',
id,
responseID: requestID,
message: 'Unsupported feature: ' + error.message,
};
}

// Log Uncaught Error
console.error('Error inspecting element.\n\n', error);

return {
Expand Down
19 changes: 19 additions & 0 deletions packages/react-devtools-shared/src/backend/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,8 @@ 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';
Expand All @@ -293,6 +295,21 @@ export type InspectElementError = {|
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,
|};

export type InspectElementFullData = {|
id: number,
responseID: number,
Expand Down Expand Up @@ -322,6 +339,8 @@ export type InspectElementNotFound = {|

export type InspectedElementPayload =
| InspectElementError
| InspectElementUserError
| InspectElementUnsupportedFeatureError
| InspectElementFullData
| InspectElementHydratedPath
| InspectElementNoChange
Expand Down
2 changes: 1 addition & 1 deletion packages/react-devtools-shared/src/backendAPI.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import {hydrate, fillInPath} from 'react-devtools-shared/src/hydration';
import {separateDisplayNameAndHOCs} from 'react-devtools-shared/src/utils';
import Store from 'react-devtools-shared/src/devtools/store';
import TimeoutError from 'react-devtools-shared/src/TimeoutError';
import TimeoutError from 'react-devtools-shared/src/errors/TimeoutError';

import type {
InspectedElement as InspectedElementBackend,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ export type OwnersList = {|

export type InspectedElementResponseType =
| 'error'
| 'user-error'
| 'full-data'
| 'hydrated-path'
| 'no-change'
Expand Down
Loading