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

Triggering Error Boundaries from DevTools #340

Closed
bvaughn opened this issue Jun 27, 2019 · 2 comments
Closed

Triggering Error Boundaries from DevTools #340

bvaughn opened this issue Jun 27, 2019 · 2 comments
Labels
😎 enhancement New feature or request

Comments

@bvaughn
Copy link
Owner

bvaughn commented Jun 27, 2019

It would be useful to force components into an error state, in order to test error boundaries (similar to how the suspense toggle works).

@bvaughn bvaughn added the 😎 enhancement New feature or request label Jun 27, 2019
@bvaughn
Copy link
Owner Author

bvaughn commented Jul 29, 2019

I took a quick pass at this this afternoon but I didn't get finished. Got a little hung up on the best way to tell if an error boundary was in an "errored" state. Seems particularly tricky for "legacy boundaries" since React itself uses a Map to track these.

So here's a dump of the partial changes I made to react:

diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js
index 18c076bd3..bc25b680d 100644
--- a/packages/react-reconciler/src/ReactFiberBeginWork.js
+++ b/packages/react-reconciler/src/ReactFiberBeginWork.js
@@ -110,7 +110,7 @@ import {
   registerSuspenseInstanceRetry,
 } from './ReactFiberHostConfig';
 import type {SuspenseInstance} from './ReactFiberHostConfig';
-import {shouldSuspend} from './ReactFiberReconciler';
+import {shouldError, shouldSuspend} from './ReactFiberReconciler';
 import {pushHostContext, pushHostContainer} from './ReactFiberHostContext';
 import {
   suspenseStackCursor,
@@ -606,6 +606,13 @@ function updateFunctionComponent(
   nextProps: any,
   renderExpirationTime,
 ) {
+  // This is used by DevTools to force a boundary to suspend.
+  if (__DEV__) {
+    if (shouldError(workInProgress)) {
+      workInProgress.effectTag |= DidCapture;
+    }
+  }
+
   if (__DEV__) {
     if (workInProgress.type !== workInProgress.elementType) {
       // Lazy component props can't be validated in createElement
@@ -701,6 +708,13 @@ function updateClassComponent(
   nextProps,
   renderExpirationTime: ExpirationTime,
 ) {
+  // This is used by DevTools to force a boundary to suspend.
+  if (__DEV__) {
+    if (shouldError(workInProgress)) {
+      workInProgress.effectTag |= DidCapture;
+    }
+  }
+
   if (__DEV__) {
     if (workInProgress.type !== workInProgress.elementType) {
       // Lazy component props can't be validated in createElement
diff --git a/packages/react-reconciler/src/ReactFiberReconciler.js b/packages/react-reconciler/src/ReactFiberReconciler.js
index aade7ccb1..1d2f6366e 100644
--- a/packages/react-reconciler/src/ReactFiberReconciler.js
+++ b/packages/react-reconciler/src/ReactFiberReconciler.js
@@ -381,8 +381,13 @@ export function findHostInstanceWithNoPortals(
   return hostFiber.stateNode;
 }
 
+let shouldErrorImpl = fiber => false;
 let shouldSuspendImpl = fiber => false;
 
+export function shouldError(fiber: Fiber): boolean {
+  return shouldErrorImpl(fiber);
+}
+
 export function shouldSuspend(fiber: Fiber): boolean {
   return shouldSuspendImpl(fiber);
 }
@@ -390,6 +395,7 @@ export function shouldSuspend(fiber: Fiber): boolean {
 let overrideHookState = null;
 let overrideProps = null;
 let scheduleUpdate = null;
+let setErrorHandler = null;
 let setSuspenseHandler = null;
 
 if (__DEV__) {
@@ -470,6 +476,10 @@ if (__DEV__) {
     scheduleWork(fiber, Sync);
   };
 
+  setErrorHandler = (newShouldErrorImpl: Fiber => boolean) => {
+    shouldErrorImpl = newShouldErrorImpl;
+  };
+
   setSuspenseHandler = (newShouldSuspendImpl: Fiber => boolean) => {
     shouldSuspendImpl = newShouldSuspendImpl;
   };
@@ -483,6 +493,7 @@ export function injectIntoDevTools(devToolsConfig: DevToolsConfig): boolean {
     ...devToolsConfig,
     overrideHookState,
     overrideProps,
+    setErrorHandler,
     setSuspenseHandler,
     scheduleUpdate,
     currentDispatcherRef: ReactCurrentDispatcher,

And here's a dump of the partial changes I made tp DevTools:

diff --git a/shells/dev/app/index.js b/shells/dev/app/index.js
index b0609a8..f4d6d1c 100644
--- a/shells/dev/app/index.js
+++ b/shells/dev/app/index.js
@@ -11,6 +11,7 @@ import DeeplyNestedComponents from './DeeplyNestedComponents';
 import Iframe from './Iframe';
 import EditableProps from './EditableProps';
 import ElementTypes from './ElementTypes';
+import ErrorBoundaries from './ErrorBoundaries';
 import Hydration from './Hydration';
 import InspectableElements from './InspectableElements';
 import InteractionTracing from './InteractionTracing';
@@ -42,6 +43,7 @@ function mountTestApp() {
   mountHelper(Hydration);
   mountHelper(ElementTypes);
   mountHelper(EditableProps);
+  mountHelper(ErrorBoundaries);
   mountHelper(PriorityLevels);
   mountHelper(ReactNativeWeb);
   mountHelper(Toggle);
diff --git a/src/backend/agent.js b/src/backend/agent.js
index a711080..10553f8 100644
--- a/src/backend/agent.js
+++ b/src/backend/agent.js
@@ -50,6 +50,12 @@ type InspectElementParams = {|
   rendererID: number,
 |};
 
+type OverrideErrorParams = {|
+  id: number,
+  rendererID: number,
+  forceError: boolean,
+|};
+
 type OverrideHookParams = {|
   id: number,
   hookID: number,
@@ -120,6 +126,7 @@ export default class Agent extends EventEmitter<{|
     bridge.addListener('inspectElement', this.inspectElement);
     bridge.addListener('logElementToConsole', this.logElementToConsole);
     bridge.addListener('overrideContext', this.overrideContext);
+    bridge.addListener('overrideError', this.overrideError);
     bridge.addListener('overrideHookState', this.overrideHookState);
     bridge.addListener('overrideProps', this.overrideProps);
     bridge.addListener('overrideState', this.overrideState);
@@ -304,6 +311,19 @@ export default class Agent extends EventEmitter<{|
     }
   };
 
+  overrideError = ({
+    id,
+    rendererID,
+    forceError,
+  }: OverrideErrorParams) => {
+    const renderer = this._rendererInterfaces[rendererID];
+    if (renderer == null) {
+      console.warn(`Invalid renderer id "${rendererID}" for element "${id}"`);
+    } else {
+      renderer.overrideError(id, forceError);
+    }
+  };
+
   overrideHookState = ({
     id,
     hookID,
diff --git a/src/backend/renderer.js b/src/backend/renderer.js
index 979eefd..2b53dd8 100644
--- a/src/backend/renderer.js
+++ b/src/backend/renderer.js
@@ -126,6 +126,7 @@ type ReactTypeOfWorkType = {|
 |};
 
 type ReactTypeOfSideEffectType = {|
+  DidCapture: number,
   NoEffect: number,
   PerformedWork: number,
   Placement: number,
@@ -175,6 +176,7 @@ export function getInternalReactConstants(
   };
 
   const ReactTypeOfSideEffect: ReactTypeOfSideEffectType = {
+    DidCapture: 0b1000000,
     NoEffect: 0b00,
     PerformedWork: 0b01,
     Placement: 0b10,
@@ -431,7 +433,12 @@ export function attach(
     ReactSymbols,
     ReactTypeOfSideEffect,
   } = getInternalReactConstants(renderer.version);
-  const { NoEffect, PerformedWork, Placement } = ReactTypeOfSideEffect;
+  const {
+    DidCapture,
+    NoEffect,
+    PerformedWork,
+    Placement,
+  } = ReactTypeOfSideEffect;
   const {
     FunctionComponent,
     ClassComponent,
@@ -477,9 +484,15 @@ export function attach(
   const {
     overrideHookState,
     overrideProps,
+    setErrorHandler,
     setSuspenseHandler,
     scheduleUpdate,
   } = renderer;
+
+  const supportsTogglingError =
+    typeof setErrorHandler === 'function' &&
+    typeof scheduleUpdate === 'function';
+
   const supportsTogglingSuspense =
     typeof setSuspenseHandler === 'function' &&
     typeof scheduleUpdate === 'function';
@@ -1113,6 +1126,23 @@ export function attach(
     return stringID;
   }
 
+  function isErrorBoundary(fiber: Fiber): boolean {
+    const { tag, type } = fiber;
+
+    switch (tag) {
+      case ClassComponent:
+      case IncompleteClassComponent:
+        const instance = fiber.stateNode;
+        return (
+          typeof type.getDerivedStateFromError === 'function' ||
+          (instance !== null &&
+            typeof instance.componentDidCatch === 'function')
+        );
+      default:
+        return false;
+    }
+  }
+
   function recordMount(fiber: Fiber, parentFiber: Fiber | null) {
     const isRoot = fiber.tag === HostRoot;
     const id = getFiberID(getPrimaryFiber(fiber));
@@ -1151,6 +1181,7 @@ export function attach(
       pushOperation(elementType);
       pushOperation(parentID);
       pushOperation(ownerID);
+      pushOperation(isErrorBoundary(fiber) ? 1 : 0);
       pushOperation(displayNameStringID);
       pushOperation(keyStringID);
     }
@@ -2237,6 +2268,8 @@ export function attach(
       }
     }
 
+    const isErrored = false; // TODO How do we calculate this?
+
     return {
       id,
 
@@ -2246,6 +2279,14 @@ export function attach(
       // Does the current renderer support editable function props?
       canEditFunctionProps: typeof overrideProps === 'function',
 
+      canToggleError:
+        supportsTogglingError &&
+        // If it's showing the real content, we can always flip it into an error state.
+        (!isErrored ||
+          // If it's showing an error state because we previously forced it to,
+          // allow toggling it back to remove the error boundary.
+          forceErrorForFiberIDs.has(id)),
+
       canToggleSuspense:
         supportsTogglingSuspense &&
         // If it's showing the real content, we can always flip fallback.
@@ -2257,6 +2298,9 @@ export function attach(
       // Can view component source location.
       canViewSource,
 
+      // Is this element an error boundary currently in an errored state?
+      isErrored,
+
       displayName: getDisplayNameForFiber(fiber),
       type: getElementTypeForFiber(fiber),
 
@@ -2705,16 +2749,52 @@ export function attach(
   // React will switch between these implementations depending on whether
   // we have any manually suspended Fibers or not.
 
+  function shouldErrorFiberAlwaysFalse() {
+    return false;
+  }
+
   function shouldSuspendFiberAlwaysFalse() {
     return false;
   }
 
+  let forceErrorForFiberIDs = new Set();
+  function shouldErrorFiberAccordingToSet(fiber) {
+    const id = getFiberID(getPrimaryFiber(((fiber: any): Fiber)));
+    return forceErrorForFiberIDs.has(id);
+  }
+
   let forceFallbackForSuspenseIDs = new Set();
   function shouldSuspendFiberAccordingToSet(fiber) {
     const id = getFiberID(getPrimaryFiber(((fiber: any): Fiber)));
     return forceFallbackForSuspenseIDs.has(id);
   }
 
+  function overrideError(id, forceError) {
+    if (
+      typeof setErrorHandler !== 'function' ||
+      typeof scheduleUpdate !== 'function'
+    ) {
+      throw new Error(
+        'Expected overrideError() to not get called for earlier React versions.'
+      );
+    }
+    if (forceError) {
+      forceErrorForFiberIDs.add(id);
+      if (forceErrorForFiberIDs.size === 1) {
+        // First override is added. Switch React to slower path.
+        setErrorHandler(shouldErrorFiberAccordingToSet);
+      }
+    } else {
+      forceErrorForFiberIDs.delete(id);
+      if (forceErrorForFiberIDs.size === 0) {
+        // Last override is gone. Switch React back to fast path.
+        setErrorHandler(shouldErrorFiberAlwaysFalse);
+      }
+    }
+    const fiber = idToFiberMap.get(id);
+    scheduleUpdate(fiber);
+  }
+
   function overrideSuspense(id, forceFallback) {
     if (
       typeof setSuspenseHandler !== 'function' ||
@@ -2984,6 +3064,7 @@ export function attach(
     inspectElement,
     logElementToConsole,
     prepareViewElementSource,
+    overrideError,
     overrideSuspense,
     renderer,
     selectElement,
diff --git a/src/backend/types.js b/src/backend/types.js
index c0a2722..4719a5b 100644
--- a/src/backend/types.js
+++ b/src/backend/types.js
@@ -134,6 +134,7 @@ export type ReactRenderer = {
 
   // 16.9+
   scheduleUpdate?: ?(fiber: Object) => void,
+  setErrorHandler?: ?(shouldError: (fiber: Object) => boolean) => void,
   setSuspenseHandler?: ?(shouldSuspend: (fiber: Object) => boolean) => void,
 
   // Only injected by React v16.8+ in order to support hooks inspection.
diff --git a/src/devtools/store.js b/src/devtools/store.js
index adaccfe..f8847fb 100644
--- a/src/devtools/store.js
+++ b/src/devtools/store.js
@@ -756,6 +756,7 @@ export default class Store extends EventEmitter<{|
             );
           }
 
+          let isErrorBoundary: boolean = false;
           let ownerID: number = 0;
           let parentID: number = ((null: any): number);
           if (type === ElementTypeRoot) {
@@ -798,6 +799,9 @@ export default class Store extends EventEmitter<{|
             ownerID = ((operations[i]: any): number);
             i++;
 
+            isErrorBoundary = ((operations[i]: any): number) > 0;
+            i++;
+
             const displayNameStringID = operations[i];
             const displayName = stringTable[displayNameStringID];
             i++;
@@ -836,6 +840,7 @@ export default class Store extends EventEmitter<{|
               hocDisplayNames,
               id,
               isCollapsed: this._collapseNodesByDefault,
+              isErrorBoundary,
               key,
               ownerID,
               parentID: parentElement.id,
diff --git a/src/devtools/views/ButtonIcon.js b/src/devtools/views/ButtonIcon.js
index 6c3887f..0bf28c4 100644
--- a/src/devtools/views/ButtonIcon.js
+++ b/src/devtools/views/ButtonIcon.js
@@ -12,6 +12,7 @@ export type IconType =
   | 'copy'
   | 'delete'
   | 'down'
+  | 'error'
   | 'expanded'
   | 'export'
   | 'filter'
@@ -63,6 +64,9 @@ export default function ButtonIcon({ className = '', type }: Props) {
     case 'down':
       pathData = PATH_DOWN;
       break;
+    case 'error':
+      pathData = PATH_ERROR;
+      break;
     case 'expanded':
       pathData = PATH_EXPANDED;
       break;
@@ -165,6 +169,9 @@ const PATH_DELETE = `
 
 const PATH_DOWN = 'M7.41 8.59L12 13.17l4.59-4.58L18 10l-6 6-6-6 1.41-1.41z';
 
+const PATH_ERROR =
+  'M12 2C6.48 2 2 6.48 2 12s4.48 10 10 10 10-4.48 10-10S17.52 2 12 2zm1 15h-2v-2h2v2zm0-4h-2V7h2v6z';
+
 const PATH_EXPANDED = 'M7 10l5 5 5-5z';
 
 const PATH_EXPORT = 'M15.82,2.14v7H21l-9,9L3,9.18H8.18v-7ZM3,20.13H21v1.73H3Z';
diff --git a/src/devtools/views/Components/InspectedElementContext.js b/src/devtools/views/Components/InspectedElementContext.js
index 8ba1b77..0e52748 100644
--- a/src/devtools/views/Components/InspectedElementContext.js
+++ b/src/devtools/views/Components/InspectedElementContext.js
@@ -149,8 +149,10 @@ function InspectedElementContextController({ children }: Props) {
           const {
             canEditFunctionProps,
             canEditHooks,
+            canToggleError,
             canToggleSuspense,
             canViewSource,
+            isErrored,
             source,
             type,
             owners,
@@ -163,8 +165,10 @@ function InspectedElementContextController({ children }: Props) {
           const inspectedElement: InspectedElementFrontend = {
             canEditFunctionProps,
             canEditHooks,
+            canToggleError,
             canToggleSuspense,
             canViewSource,
+            isErrored,
             id,
             source,
             type,
diff --git a/src/devtools/views/Components/SelectedElement.js b/src/devtools/views/Components/SelectedElement.js
index 30e9bd5..3279e04 100644
--- a/src/devtools/views/Components/SelectedElement.js
+++ b/src/devtools/views/Components/SelectedElement.js
@@ -100,15 +100,68 @@ export default function SelectedElement(_: Props) {
     (canViewElementSourceFunction === null ||
       canViewElementSourceFunction(inspectedElement));
 
+  const isErrored =
+    inspectedElement != null &&
+    inspectedElement.isErrored != null;
+
   const isSuspended =
     element !== null &&
     element.type === ElementTypeSuspense &&
     inspectedElement != null &&
     inspectedElement.state != null;
 
+  const canToggleError =
+    inspectedElement != null && inspectedElement.canToggleError;
   const canToggleSuspense =
     inspectedElement != null && inspectedElement.canToggleSuspense;
 
+  // TODO (error toggle) Would be nice to eventually use a two setState pattern here as well.
+  const toggleErrored = useCallback(() => {
+    let nearestErrorBoundary = null;
+    let currentElement = element;
+    while (currentElement !== null) {
+      if (currentElement.isErrorBoundary) {
+        nearestErrorBoundary = currentElement;
+        break;
+      } else if (currentElement.parentID > 0) {
+        currentElement = store.getElementByID(currentElement.parentID);
+      } else {
+        currentElement = null;
+      }
+    }
+
+    // If we didn't find an error boundary ancestor, we can't throw.
+    // Instead we can show a warning to the user.
+    if (nearestErrorBoundary === null) {
+      modalDialogDispatch({
+        type: 'SHOW',
+        content: <CannotThrowWarningMessage />,
+      });
+    } else {
+      const nearestErrorBoundaryID = nearestErrorBoundary.id;
+
+      // If we're suspending from an arbitary (non-Suspense) component, select the nearest Suspense element in the Tree.
+      // This way when the fallback UI is shown and the current element is hidden, something meaningful is selected.
+      if (nearestErrorBoundary !== element) {
+        dispatch({
+          type: 'SELECT_ELEMENT_BY_ID',
+          payload: nearestErrorBoundaryID,
+        });
+      }
+
+      const rendererID = store.getRendererIDForElement(nearestErrorBoundaryID);
+
+      // Toggle suspended
+      if (rendererID !== null) {
+        bridge.send('overrideError', {
+          id: nearestErrorBoundaryID,
+          rendererID,
+          forceError: !isErrored,
+        });
+      }
+    }
+  }, [bridge, dispatch, element, isSuspended, modalDialogDispatch, store]);
+
   // TODO (suspense toggle) Would be nice to eventually use a two setState pattern here as well.
   const toggleSuspended = useCallback(() => {
     let nearestSuspenseElement = null;
@@ -175,6 +228,20 @@ export default function SelectedElement(_: Props) {
           </div>
         </div>
 
+        {canToggleError && (
+          <Toggle
+            className={styles.IconButton}
+            isChecked={isSuspended}
+            onChange={toggleErrored}
+            title={
+              isErrored
+                ? 'Clear the forced error'
+                : 'Force the selected component into an errored state'
+            }
+          >
+            <ButtonIcon type="error" />
+          </Toggle>
+        )}
         {canToggleSuspense && (
           <Toggle
             className={styles.IconButton}
@@ -433,6 +500,35 @@ function OwnerView({
   );
 }
 
+function CannotThrowWarningMessage() {
+  const store = useContext(StoreContext);
+  const areClassComponentsHidden = !!store.componentFilters.find(
+    filter =>
+      filter.type === ComponentFilterElementType &&
+      filter.value === ElementTypeClass &&
+      filter.isEnabled
+  );
+
+  // Has the user filted out class nodes from the tree?
+  // If so, the selected element might actually be in an error boundary,
+  // but we have no way to know.
+  if (areClassComponentsHidden) {
+    return (
+      <div className={styles.CannotSuspendWarningMessage}>
+        Error state cannot be toggled while class components are hidden. Disable
+        the filter and try agan.
+      </div>
+    );
+  } else {
+    return (
+      <div className={styles.CannotSuspendWarningMessage}>
+        The selected element is not within an error boundary. Breaking it would
+        cause an error.
+      </div>
+    );
+  }
+}
+
 function CannotSuspendWarningMessage() {
   const store = useContext(StoreContext);
   const areSuspenseElementsHidden = !!store.componentFilters.find(

@bvaughn
Copy link
Owner Author

bvaughn commented Aug 19, 2019

This repository is being merged into the main React repo (github.com/facebook/react). As part of this, I am moving all issues to that repository as well and closing them here.

This issue has been relocated to:
facebook/react#16469

@bvaughn bvaughn closed this as completed Aug 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
😎 enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant