From 12749e6e67a5f4b98be303c7b7d6a8448414696c Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Fri, 1 Oct 2021 14:31:25 -0400 Subject: [PATCH 1/2] Show different error boundary UI for timeoutes while inspecting elements --- .../react-devtools-shared/src/TimeoutError.js | 21 +++++++ .../react-devtools-shared/src/backendAPI.js | 5 +- .../views/ErrorBoundary/ErrorBoundary.js | 55 +++++++++++++------ .../devtools/views/ErrorBoundary/ErrorView.js | 6 +- .../views/ErrorBoundary/TimeoutView.js | 51 +++++++++++++++++ .../devtools/views/ErrorBoundary/shared.css | 29 ++++++++-- .../src/inspectedElementCache.js | 8 ++- 7 files changed, 146 insertions(+), 29 deletions(-) create mode 100644 packages/react-devtools-shared/src/TimeoutError.js create mode 100644 packages/react-devtools-shared/src/devtools/views/ErrorBoundary/TimeoutView.js diff --git a/packages/react-devtools-shared/src/TimeoutError.js b/packages/react-devtools-shared/src/TimeoutError.js new file mode 100644 index 0000000000000..351e88dfb8e0f --- /dev/null +++ b/packages/react-devtools-shared/src/TimeoutError.js @@ -0,0 +1,21 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +export default class TimeoutError extends Error { + constructor(message: string) { + super(message); + + // Maintains proper stack trace for where our error was thrown (only available on V8) + if (Error.captureStackTrace) { + Error.captureStackTrace(this, TimeoutError); + } + + this.name = 'TimeoutError'; + } +} diff --git a/packages/react-devtools-shared/src/backendAPI.js b/packages/react-devtools-shared/src/backendAPI.js index 2aa5b30cd22e6..4c2188288f7ae 100644 --- a/packages/react-devtools-shared/src/backendAPI.js +++ b/packages/react-devtools-shared/src/backendAPI.js @@ -10,6 +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 type { InspectedElement as InspectedElementBackend, @@ -162,7 +163,9 @@ function getPromiseForRequestID( const onTimeout = () => { cleanup(); reject( - new Error(`Timed out waiting for event '${eventType}' from bridge`), + new TimeoutError( + `Timed out waiting for event '${eventType}' from bridge`, + ), ); }; diff --git a/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ErrorBoundary.js b/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ErrorBoundary.js index ea4f54b1a9b8b..3dbfb21abafc2 100644 --- a/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ErrorBoundary.js +++ b/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ErrorBoundary.js @@ -13,6 +13,8 @@ import Store from 'react-devtools-shared/src/devtools/store'; import ErrorView from './ErrorView'; import SearchingGitHubIssues from './SearchingGitHubIssues'; import SuspendingErrorView from './SuspendingErrorView'; +import TimeoutView from './TimeoutView'; +import TimeoutError from 'react-devtools-shared/src/TimeoutError'; type Props = {| children: React$Node, @@ -27,6 +29,7 @@ type State = {| componentStack: string | null, errorMessage: string | null, hasError: boolean, + isTimeout: boolean, |}; const InitialState: State = { @@ -35,6 +38,7 @@ const InitialState: State = { componentStack: null, errorMessage: null, hasError: false, + isTimeout: false, }; export default class ErrorBoundary extends Component { @@ -48,6 +52,8 @@ export default class ErrorBoundary extends Component { ? error.message : String(error); + const isTimeout = error instanceof TimeoutError; + const callStack = typeof error === 'object' && error !== null && @@ -62,6 +68,7 @@ export default class ErrorBoundary extends Component { callStack, errorMessage, hasError: true, + isTimeout, }; } @@ -93,26 +100,40 @@ export default class ErrorBoundary extends Component { componentStack, errorMessage, hasError, + isTimeout, } = this.state; if (hasError) { - return ( - - }> - - - - ); + if (isTimeout) { + return ( + + ); + } else { + return ( + + }> + + + + ); + } } return children; diff --git a/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ErrorView.js b/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ErrorView.js index baff24e522e1b..7443707496aa8 100644 --- a/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ErrorView.js +++ b/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ErrorView.js @@ -32,7 +32,7 @@ export default function ErrorView({ {children}
-
+
Uncaught Error: {errorMessage || ''}
{dismissError !== null && ( @@ -43,12 +43,12 @@ export default function ErrorView({ )}
{!!callStack && ( -
+
The error was thrown {callStack.trim()}
)} {!!componentStack && ( -
+
The error occurred {componentStack.trim()}
)} diff --git a/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/TimeoutView.js b/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/TimeoutView.js new file mode 100644 index 0000000000000..45c64312be137 --- /dev/null +++ b/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/TimeoutView.js @@ -0,0 +1,51 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +import * as React from 'react'; +import Button from '../Button'; +import ButtonIcon from '../ButtonIcon'; +import styles from './shared.css'; + +type Props = {| + callStack: string | null, + children: React$Node, + componentStack: string | null, + dismissError: Function, + errorMessage: string | null, +|}; + +export default function TimeoutView({ + callStack, + children, + componentStack, + dismissError = null, + errorMessage, +}: Props) { + return ( +
+ {children} +
+
+
+ {errorMessage || 'Timed out waiting'} +
+ +
+ {!!componentStack && ( +
+ The timeout occurred {componentStack.trim()} +
+ )} +
+
+ ); +} diff --git a/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/shared.css b/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/shared.css index bef5c945c226e..ce8ad5f79ce24 100644 --- a/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/shared.css +++ b/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/shared.css @@ -30,6 +30,7 @@ background-color: var(--color-background); display: flex; flex-direction: column; + border: 1px solid var(--color-border); } .ErrorInfo { @@ -42,10 +43,10 @@ flex-direction: row; font-size: var(--font-size-sans-large); font-weight: bold; - color: var(--color-error-text); } -.Header { +.ErrorHeader, +.TimeoutHeader { flex: 1 1 auto; overflow: hidden; text-overflow: ellipsis; @@ -53,17 +54,35 @@ min-width: 0; } -.Stack { +.ErrorHeader { + color: var(--color-error-text); +} +.TimeoutHeader { + color: var(--color-text); +} + +.ErrorStack, +.TimeoutStack { margin-top: 0.5rem; white-space: pre-wrap; font-family: var(--font-family-monospace); font-size: var(--font-size-monospace-normal); -webkit-font-smoothing: initial; + border-radius: 0.25rem; + padding: 0.5rem; + overflow: auto; +} + +.ErrorStack { background-color: var(--color-error-background); border: 1px solid var(--color-error-border); color: var(--color-error-text); - border-radius: 0.25rem; - padding: 0.5rem; +} + +.TimeoutStack { + background-color: var(--color-console-warning-background); + color: var(--color-console-warning-text); + border: var(--color-console-warning-border) } .LoadingIcon { diff --git a/packages/react-devtools-shared/src/inspectedElementCache.js b/packages/react-devtools-shared/src/inspectedElementCache.js index e0e43e4022b20..6442a84b0be64 100644 --- a/packages/react-devtools-shared/src/inspectedElementCache.js +++ b/packages/react-devtools-shared/src/inspectedElementCache.js @@ -38,7 +38,7 @@ type ResolvedRecord = {| type RejectedRecord = {| status: 2, - value: string, + value: Error | string, |}; type Record = PendingRecord | ResolvedRecord | RejectedRecord; @@ -113,7 +113,9 @@ export function inspectElement( if (rendererID == null) { const rejectedRecord = ((newRecord: any): RejectedRecord); rejectedRecord.status = Rejected; - rejectedRecord.value = `Could not inspect element with id "${element.id}". No renderer found.`; + rejectedRecord.value = new Error( + `Could not inspect element with id "${element.id}". No renderer found.`, + ); map.set(element, record); @@ -139,7 +141,7 @@ export function inspectElement( const rejectedRecord = ((newRecord: any): RejectedRecord); rejectedRecord.status = Rejected; - rejectedRecord.value = `Could not inspect element with id "${element.id}". Error thrown:\n${error.message}`; + rejectedRecord.value = error; wake(); }, From c8172831108217f9efcc07659c14472d826c618f Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Fri, 1 Oct 2021 14:39:22 -0400 Subject: [PATCH 2/2] Tweaked timeout message text --- packages/react-devtools-shared/src/backendAPI.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/react-devtools-shared/src/backendAPI.js b/packages/react-devtools-shared/src/backendAPI.js index 4c2188288f7ae..23b975e38d6ea 100644 --- a/packages/react-devtools-shared/src/backendAPI.js +++ b/packages/react-devtools-shared/src/backendAPI.js @@ -103,6 +103,7 @@ export function inspectElement({ requestID, 'inspectedElement', bridge, + `Timed out while inspecting element ${id}.`, ); bridge.send('inspectElement', { @@ -145,6 +146,7 @@ function getPromiseForRequestID( requestID: number, eventType: $Keys, bridge: FrontendBridge, + timeoutMessage: string, ): Promise { return new Promise((resolve, reject) => { const cleanup = () => { @@ -162,11 +164,7 @@ function getPromiseForRequestID( const onTimeout = () => { cleanup(); - reject( - new TimeoutError( - `Timed out waiting for event '${eventType}' from bridge`, - ), - ); + reject(new TimeoutError(timeoutMessage)); }; bridge.addListener(eventType, onInspectedElement);