Skip to content

Commit

Permalink
Surface backend errors during inspection in the frontend UI
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Brian Vaughn committed Oct 12, 2021
1 parent 1e247ff commit 2f94443
Show file tree
Hide file tree
Showing 5 changed files with 98 additions and 13 deletions.
Expand Up @@ -35,6 +35,9 @@ describe('InspectedElement', () => {
let legacyRender;
let testRendererInstance;

let ErrorBoundary;
let errorBoundaryInstance;

beforeEach(() => {
utils = require('./utils');
utils.beforeEachProfiling();
Expand Down Expand Up @@ -69,6 +72,23 @@ describe('InspectedElement', () => {
testRendererInstance = TestRenderer.create(null, {
unstable_isConcurrent: true,
});

errorBoundaryInstance = null;

ErrorBoundary = class extends React.Component {
state = {error: null};
componentDidCatch(error) {
this.setState({error});
}
render() {
errorBoundaryInstance = this;

if (this.state.error) {
return null;
}
return this.props.children;
}
};
});

afterEach(() => {
Expand Down Expand Up @@ -109,7 +129,11 @@ describe('InspectedElement', () => {

function noop() {}

async function inspectElementAtIndex(index, useCustomHook = noop) {
async function inspectElementAtIndex(
index,
useCustomHook = noop,
shouldThrow = false,
) {
let didFinish = false;
let inspectedElement = null;

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

await utils.actAsync(() => {
testRendererInstance.update(
<Contexts
defaultSelectedElementID={id}
defaultSelectedElementIndex={index}>
<React.Suspense fallback={null}>
<Suspender id={id} index={index} />
</React.Suspense>
</Contexts>,
<ErrorBoundary>
<Contexts
defaultSelectedElementID={id}
defaultSelectedElementIndex={index}>
<React.Suspense fallback={null}>
<Suspender id={id} index={index} />
</React.Suspense>
</Contexts>
</ErrorBoundary>,
);
}, false);

expect(didFinish).toBe(true);
if (!shouldThrow) {
expect(didFinish).toBe(true);
}

return inspectedElement;
}
Expand Down Expand Up @@ -2069,6 +2097,34 @@ describe('InspectedElement', () => {
expect(inspectedElement.rootType).toMatchInlineSnapshot(`"createRoot()"`);
});

it('should gracefully surface backend errors on the frontend rather than timing out', async () => {
spyOn(console, 'error');

let shouldThrow = false;

const Example = () => {
const [count] = React.useState(0);

if (shouldThrow) {
throw Error('Expected');
} else {
return count;
}
};

await utils.actAsync(() => {
const container = document.createElement('div');
ReactDOM.createRoot(container).render(<Example />);
}, false);

shouldThrow = true;

const value = await inspectElementAtIndex(0, noop, true);

expect(value).toBe(null);
expect(errorBoundaryInstance.state.error.message).toBe('Expected');
});

describe('$r', () => {
it('should support function components', async () => {
const Example = () => {
Expand Down Expand Up @@ -2656,7 +2712,7 @@ describe('InspectedElement', () => {

describe('error boundary', () => {
it('can toggle error', async () => {
class ErrorBoundary extends React.Component<any> {
class LocalErrorBoundary extends React.Component<any> {
state = {hasError: false};
static getDerivedStateFromError(error) {
return {hasError: true};
Expand All @@ -2666,13 +2722,14 @@ describe('InspectedElement', () => {
return hasError ? 'has-error' : this.props.children;
}
}

const Example = () => 'example';

await utils.actAsync(() =>
legacyRender(
<ErrorBoundary>
<LocalErrorBoundary>
<Example />
</ErrorBoundary>,
</LocalErrorBoundary>,
document.createElement('div'),
),
);
Expand Down
14 changes: 13 additions & 1 deletion packages/react-devtools-shared/src/backend/renderer.js
Expand Up @@ -3471,7 +3471,19 @@ export function attach(

hasElementUpdatedSinceLastInspected = false;

mostRecentlyInspectedElement = inspectElementRaw(id);
try {
mostRecentlyInspectedElement = inspectElementRaw(id);
} catch (error) {
console.error('Error inspecting element.\n\n', error);

return {
type: 'error',
id,
responseID: requestID,
value: error.message,
};
}

if (mostRecentlyInspectedElement === null) {
return {
id,
Expand Down
9 changes: 9 additions & 0 deletions packages/react-devtools-shared/src/backend/types.js
Expand Up @@ -267,10 +267,18 @@ export type InspectedElement = {|
rendererVersion: string | null,
|};

export const InspectElementErrorType = 'error';
export const InspectElementFullDataType = 'full-data';
export const InspectElementNoChangeType = 'no-change';
export const InspectElementNotFoundType = 'not-found';

export type InspectElementError = {|
id: number,
responseID: number,
type: 'error',
value: string,
|};

export type InspectElementFullData = {|
id: number,
responseID: number,
Expand Down Expand Up @@ -299,6 +307,7 @@ export type InspectElementNotFound = {|
|};

export type InspectedElementPayload =
| InspectElementError
| InspectElementFullData
| InspectElementHydratedPath
| InspectElementNoChange
Expand Down
Expand Up @@ -58,6 +58,7 @@ export type OwnersList = {|
|};

export type InspectedElementResponseType =
| 'error'
| 'full-data'
| 'hydrated-path'
| 'no-change'
Expand Down
Expand Up @@ -18,6 +18,7 @@ import {fillInPath} from 'react-devtools-shared/src/hydration';
import type {LRUCache} from 'react-devtools-shared/src/types';
import type {FrontendBridge} from 'react-devtools-shared/src/bridge';
import type {
InspectElementError,
InspectElementFullData,
InspectElementHydratedPath,
} from 'react-devtools-shared/src/backend/types';
Expand Down Expand Up @@ -79,6 +80,11 @@ export function inspectElement({

let inspectedElement;
switch (type) {
case 'error':
const error = ((data: any): InspectElementError);

throw new Error(error.value);

case 'no-change':
// This is a no-op for the purposes of our cache.
inspectedElement = inspectedElementCache.get(id);
Expand Down

0 comments on commit 2f94443

Please sign in to comment.