diff --git a/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js b/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js index 211cd27b1522f..68d014de3ad47 100644 --- a/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js +++ b/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js @@ -35,6 +35,9 @@ describe('InspectedElement', () => { let legacyRender; let testRendererInstance; + let ErrorBoundary; + let errorBoundaryInstance; + beforeEach(() => { utils = require('./utils'); utils.beforeEachProfiling(); @@ -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(() => { @@ -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; @@ -124,17 +148,21 @@ describe('InspectedElement', () => { await utils.actAsync(() => { testRendererInstance.update( - - - - - , + + + + + + + , ); }, false); - expect(didFinish).toBe(true); + if (!shouldThrow) { + expect(didFinish).toBe(true); + } return inspectedElement; } @@ -2069,6 +2097,37 @@ 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(); + }, false); + + shouldThrow = true; + + const value = await inspectElementAtIndex(0, noop, true); + + expect(value).toBe(null); + + const error = errorBoundaryInstance.state.error; + expect(error.message).toBe('Expected'); + expect(error.stack).toContain('inspectHooksOfFiber'); + }); + describe('$r', () => { it('should support function components', async () => { const Example = () => { @@ -2656,7 +2715,7 @@ describe('InspectedElement', () => { describe('error boundary', () => { it('can toggle error', async () => { - class ErrorBoundary extends React.Component { + class LocalErrorBoundary extends React.Component { state = {hasError: false}; static getDerivedStateFromError(error) { return {hasError: true}; @@ -2666,13 +2725,14 @@ describe('InspectedElement', () => { return hasError ? 'has-error' : this.props.children; } } + const Example = () => 'example'; await utils.actAsync(() => legacyRender( - + - , + , document.createElement('div'), ), ); diff --git a/packages/react-devtools-shared/src/backend/renderer.js b/packages/react-devtools-shared/src/backend/renderer.js index 19155cb4e354f..218b255388371 100644 --- a/packages/react-devtools-shared/src/backend/renderer.js +++ b/packages/react-devtools-shared/src/backend/renderer.js @@ -3471,7 +3471,20 @@ 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, + message: error.message, + stack: error.stack, + }; + } + if (mostRecentlyInspectedElement === null) { return { id, diff --git a/packages/react-devtools-shared/src/backend/types.js b/packages/react-devtools-shared/src/backend/types.js index bbf574ed21a50..3f623ea656e48 100644 --- a/packages/react-devtools-shared/src/backend/types.js +++ b/packages/react-devtools-shared/src/backend/types.js @@ -267,10 +267,19 @@ 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', + message: string, + stack: string, +|}; + export type InspectElementFullData = {| id: number, responseID: number, @@ -299,6 +308,7 @@ export type InspectElementNotFound = {| |}; export type InspectedElementPayload = + | InspectElementError | InspectElementFullData | InspectElementHydratedPath | InspectElementNoChange diff --git a/packages/react-devtools-shared/src/devtools/views/Components/types.js b/packages/react-devtools-shared/src/devtools/views/Components/types.js index d48e8e32e1f2b..a40b72f4cc8e0 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/types.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/types.js @@ -58,6 +58,7 @@ export type OwnersList = {| |}; export type InspectedElementResponseType = + | 'error' | 'full-data' | 'hydrated-path' | 'no-change' diff --git a/packages/react-devtools-shared/src/inspectedElementMutableSource.js b/packages/react-devtools-shared/src/inspectedElementMutableSource.js index 9302cd782a0cf..5cb5370d1c21d 100644 --- a/packages/react-devtools-shared/src/inspectedElementMutableSource.js +++ b/packages/react-devtools-shared/src/inspectedElementMutableSource.js @@ -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'; @@ -79,6 +80,15 @@ export function inspectElement({ let inspectedElement; switch (type) { + case 'error': + const {message, stack} = ((data: any): InspectElementError); + + // The backend's stack (where the error originated) is more meaningful than this stack. + const error = new Error(message); + error.stack = stack; + + throw error; + case 'no-change': // This is a no-op for the purposes of our cache. inspectedElement = inspectedElementCache.get(id);