bvaughn/react-devtools-experimental

Triggering Error Boundaries from DevTools

bvaughn opened this issue · 2 comments

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

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(

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