Skip to content

Commit 42844bf

Browse files
author
Brian Vaughn
committed
Surface backend errors during inspection in the frontend UI
Previously, if a component were to throw an error while being inspected, the backend would not return a response, causing the frontend to eventually timeout. The fix for this is to catch the error and return it as a new error type response. This allows the frontend to show a more actionable message to the user.
1 parent 1e247ff commit 42844bf

File tree

5 files changed

+99
-13
lines changed

5 files changed

+99
-13
lines changed

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

Lines changed: 69 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,9 @@ describe('InspectedElement', () => {
3535
let legacyRender;
3636
let testRendererInstance;
3737

38+
let ErrorBoundary;
39+
let errorBoundaryInstance;
40+
3841
beforeEach(() => {
3942
utils = require('./utils');
4043
utils.beforeEachProfiling();
@@ -69,6 +72,23 @@ describe('InspectedElement', () => {
6972
testRendererInstance = TestRenderer.create(null, {
7073
unstable_isConcurrent: true,
7174
});
75+
76+
errorBoundaryInstance = null;
77+
78+
ErrorBoundary = class extends React.Component {
79+
state = {error: null};
80+
componentDidCatch(error) {
81+
this.setState({error});
82+
}
83+
render() {
84+
errorBoundaryInstance = this;
85+
86+
if (this.state.error) {
87+
return null;
88+
}
89+
return this.props.children;
90+
}
91+
};
7292
});
7393

7494
afterEach(() => {
@@ -109,7 +129,11 @@ describe('InspectedElement', () => {
109129

110130
function noop() {}
111131

112-
async function inspectElementAtIndex(index, useCustomHook = noop) {
132+
async function inspectElementAtIndex(
133+
index,
134+
useCustomHook = noop,
135+
shouldThrow = false,
136+
) {
113137
let didFinish = false;
114138
let inspectedElement = null;
115139

@@ -124,17 +148,21 @@ describe('InspectedElement', () => {
124148

125149
await utils.actAsync(() => {
126150
testRendererInstance.update(
127-
<Contexts
128-
defaultSelectedElementID={id}
129-
defaultSelectedElementIndex={index}>
130-
<React.Suspense fallback={null}>
131-
<Suspender id={id} index={index} />
132-
</React.Suspense>
133-
</Contexts>,
151+
<ErrorBoundary>
152+
<Contexts
153+
defaultSelectedElementID={id}
154+
defaultSelectedElementIndex={index}>
155+
<React.Suspense fallback={null}>
156+
<Suspender id={id} index={index} />
157+
</React.Suspense>
158+
</Contexts>
159+
</ErrorBoundary>,
134160
);
135161
}, false);
136162

137-
expect(didFinish).toBe(true);
163+
if (!shouldThrow) {
164+
expect(didFinish).toBe(true);
165+
}
138166

139167
return inspectedElement;
140168
}
@@ -2069,6 +2097,34 @@ describe('InspectedElement', () => {
20692097
expect(inspectedElement.rootType).toMatchInlineSnapshot(`"createRoot()"`);
20702098
});
20712099

2100+
it('should gracefully surface backend errors on the frontend rather than timing out', async () => {
2101+
spyOn(console, 'error');
2102+
2103+
let shouldThrow = false;
2104+
2105+
const Example = () => {
2106+
const [count] = React.useState(0);
2107+
2108+
if (shouldThrow) {
2109+
throw Error('Expected');
2110+
} else {
2111+
return count;
2112+
}
2113+
};
2114+
2115+
await utils.actAsync(() => {
2116+
const container = document.createElement('div');
2117+
ReactDOM.createRoot(container).render(<Example />);
2118+
}, false);
2119+
2120+
shouldThrow = true;
2121+
2122+
const value = await inspectElementAtIndex(0, noop, true);
2123+
2124+
expect(value).toBe(null);
2125+
expect(errorBoundaryInstance.state.error.message).toBe('Expected');
2126+
});
2127+
20722128
describe('$r', () => {
20732129
it('should support function components', async () => {
20742130
const Example = () => {
@@ -2656,7 +2712,7 @@ describe('InspectedElement', () => {
26562712

26572713
describe('error boundary', () => {
26582714
it('can toggle error', async () => {
2659-
class ErrorBoundary extends React.Component<any> {
2715+
class LocalErrorBoundary extends React.Component<any> {
26602716
state = {hasError: false};
26612717
static getDerivedStateFromError(error) {
26622718
return {hasError: true};
@@ -2666,13 +2722,14 @@ describe('InspectedElement', () => {
26662722
return hasError ? 'has-error' : this.props.children;
26672723
}
26682724
}
2725+
26692726
const Example = () => 'example';
26702727

26712728
await utils.actAsync(() =>
26722729
legacyRender(
2673-
<ErrorBoundary>
2730+
<LocalErrorBoundary>
26742731
<Example />
2675-
</ErrorBoundary>,
2732+
</LocalErrorBoundary>,
26762733
document.createElement('div'),
26772734
),
26782735
);

packages/react-devtools-shared/src/backend/renderer.js

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3471,7 +3471,19 @@ export function attach(
34713471

34723472
hasElementUpdatedSinceLastInspected = false;
34733473

3474-
mostRecentlyInspectedElement = inspectElementRaw(id);
3474+
try {
3475+
mostRecentlyInspectedElement = inspectElementRaw(id);
3476+
} catch (error) {
3477+
console.error('Error inspecting element.\n\n', error);
3478+
3479+
return {
3480+
type: 'error',
3481+
id,
3482+
responseID: requestID,
3483+
value: error.message,
3484+
};
3485+
}
3486+
34753487
if (mostRecentlyInspectedElement === null) {
34763488
return {
34773489
id,

packages/react-devtools-shared/src/backend/types.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,10 +267,18 @@ export type InspectedElement = {|
267267
rendererVersion: string | null,
268268
|};
269269

270+
export const InspectElementErrorType = 'error';
270271
export const InspectElementFullDataType = 'full-data';
271272
export const InspectElementNoChangeType = 'no-change';
272273
export const InspectElementNotFoundType = 'not-found';
273274

275+
export type InspectElementError = {|
276+
id: number,
277+
responseID: number,
278+
type: 'error',
279+
value: string,
280+
|};
281+
274282
export type InspectElementFullData = {|
275283
id: number,
276284
responseID: number,
@@ -299,6 +307,7 @@ export type InspectElementNotFound = {|
299307
|};
300308

301309
export type InspectedElementPayload =
310+
| InspectElementError
302311
| InspectElementFullData
303312
| InspectElementHydratedPath
304313
| InspectElementNoChange

packages/react-devtools-shared/src/devtools/views/Components/types.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ export type OwnersList = {|
5858
|};
5959

6060
export type InspectedElementResponseType =
61+
| 'error'
6162
| 'full-data'
6263
| 'hydrated-path'
6364
| 'no-change'

packages/react-devtools-shared/src/inspectedElementMutableSource.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {fillInPath} from 'react-devtools-shared/src/hydration';
1818
import type {LRUCache} from 'react-devtools-shared/src/types';
1919
import type {FrontendBridge} from 'react-devtools-shared/src/bridge';
2020
import type {
21+
InspectElementError,
2122
InspectElementFullData,
2223
InspectElementHydratedPath,
2324
} from 'react-devtools-shared/src/backend/types';
@@ -79,6 +80,11 @@ export function inspectElement({
7980

8081
let inspectedElement;
8182
switch (type) {
83+
case 'error':
84+
const error = ((data: any): InspectElementError);
85+
86+
throw new Error(error.value);
87+
8288
case 'no-change':
8389
// This is a no-op for the purposes of our cache.
8490
inspectedElement = inspectedElementCache.get(id);
@@ -94,6 +100,7 @@ export function inspectElement({
94100
// If the Element is still in the Store, we can eagerly remove it from the Map.
95101
inspectedElementCache.remove(id);
96102

103+
console.log('mutable source: "not-found"');
97104
throw Error(`Element "${id}" not found`);
98105

99106
case 'full-data':

0 commit comments

Comments
 (0)