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

Show different error boundary UI for timeouts than normal errors #22483

Merged
merged 2 commits into from Oct 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 21 additions & 0 deletions 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';
}
}
7 changes: 4 additions & 3 deletions packages/react-devtools-shared/src/backendAPI.js
Expand Up @@ -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,
Expand Down Expand Up @@ -102,6 +103,7 @@ export function inspectElement({
requestID,
'inspectedElement',
bridge,
`Timed out while inspecting element ${id}.`,
);

bridge.send('inspectElement', {
Expand Down Expand Up @@ -144,6 +146,7 @@ function getPromiseForRequestID<T>(
requestID: number,
eventType: $Keys<BackendEvents>,
bridge: FrontendBridge,
timeoutMessage: string,
): Promise<T> {
return new Promise((resolve, reject) => {
const cleanup = () => {
Expand All @@ -161,9 +164,7 @@ function getPromiseForRequestID<T>(

const onTimeout = () => {
cleanup();
reject(
new Error(`Timed out waiting for event '${eventType}' from bridge`),
);
reject(new TimeoutError(timeoutMessage));
};

bridge.addListener(eventType, onInspectedElement);
Expand Down
Expand Up @@ -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,
Expand All @@ -27,6 +29,7 @@ type State = {|
componentStack: string | null,
errorMessage: string | null,
hasError: boolean,
isTimeout: boolean,
|};

const InitialState: State = {
Expand All @@ -35,6 +38,7 @@ const InitialState: State = {
componentStack: null,
errorMessage: null,
hasError: false,
isTimeout: false,
};

export default class ErrorBoundary extends Component<Props, State> {
Expand All @@ -48,6 +52,8 @@ export default class ErrorBoundary extends Component<Props, State> {
? error.message
: String(error);

const isTimeout = error instanceof TimeoutError;

const callStack =
typeof error === 'object' &&
error !== null &&
Expand All @@ -62,6 +68,7 @@ export default class ErrorBoundary extends Component<Props, State> {
callStack,
errorMessage,
hasError: true,
isTimeout,
};
}

Expand Down Expand Up @@ -93,26 +100,40 @@ export default class ErrorBoundary extends Component<Props, State> {
componentStack,
errorMessage,
hasError,
isTimeout,
} = this.state;

if (hasError) {
return (
<ErrorView
callStack={callStack}
componentStack={componentStack}
dismissError={
canDismissProp || canDismissState ? this._dismissError : null
}
errorMessage={errorMessage}>
<Suspense fallback={<SearchingGitHubIssues />}>
<SuspendingErrorView
callStack={callStack}
componentStack={componentStack}
errorMessage={errorMessage}
/>
</Suspense>
</ErrorView>
);
if (isTimeout) {
return (
<TimeoutView
callStack={callStack}
componentStack={componentStack}
dismissError={
canDismissProp || canDismissState ? this._dismissError : null
}
errorMessage={errorMessage}
/>
);
} else {
return (
<ErrorView
callStack={callStack}
componentStack={componentStack}
dismissError={
canDismissProp || canDismissState ? this._dismissError : null
}
errorMessage={errorMessage}>
<Suspense fallback={<SearchingGitHubIssues />}>
<SuspendingErrorView
callStack={callStack}
componentStack={componentStack}
errorMessage={errorMessage}
/>
</Suspense>
</ErrorView>
);
}
}

return children;
Expand Down
Expand Up @@ -32,7 +32,7 @@ export default function ErrorView({
{children}
<div className={styles.ErrorInfo}>
<div className={styles.HeaderRow}>
<div className={styles.Header}>
<div className={styles.ErrorHeader}>
Uncaught Error: {errorMessage || ''}
</div>
{dismissError !== null && (
Expand All @@ -43,12 +43,12 @@ export default function ErrorView({
)}
</div>
{!!callStack && (
<div className={styles.Stack}>
<div className={styles.ErrorStack}>
The error was thrown {callStack.trim()}
</div>
)}
{!!componentStack && (
<div className={styles.Stack}>
<div className={styles.ErrorStack}>
The error occurred {componentStack.trim()}
</div>
)}
Expand Down
@@ -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 (
<div className={styles.ErrorBoundary}>
{children}
<div className={styles.ErrorInfo}>
<div className={styles.HeaderRow}>
<div className={styles.TimeoutHeader}>
{errorMessage || 'Timed out waiting'}
</div>
<Button className={styles.CloseButton} onClick={dismissError}>
Retry
<ButtonIcon className={styles.CloseButtonIcon} type="close" />
</Button>
</div>
{!!componentStack && (
<div className={styles.TimeoutStack}>
The timeout occurred {componentStack.trim()}
</div>
)}
</div>
</div>
);
}
Expand Up @@ -30,6 +30,7 @@
background-color: var(--color-background);
display: flex;
flex-direction: column;
border: 1px solid var(--color-border);
}

.ErrorInfo {
Expand All @@ -42,28 +43,46 @@
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;
white-space: nowrap;
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 {
Expand Down
8 changes: 5 additions & 3 deletions packages/react-devtools-shared/src/inspectedElementCache.js
Expand Up @@ -38,7 +38,7 @@ type ResolvedRecord<T> = {|

type RejectedRecord = {|
status: 2,
value: string,
value: Error | string,
|};

type Record<T> = PendingRecord | ResolvedRecord<T> | RejectedRecord;
Expand Down Expand Up @@ -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);

Expand All @@ -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();
},
Expand Down