invokeGuardedCallback returns a thrown error

We can use this more flexible version for error handling in Fiber.

The DEV mode version uses same fake event trick as before to preserve
"break on uncaught exception" behavior when debugging.

Author: acdlite

scripts/fiber/tests-passing.txt

@@ -1667,11 +1667,16 @@ src/renderers/shared/stack/reconciler/__tests__/Transaction-test.js
 * should allow nesting of transactions
 
 src/renderers/shared/utils/__tests__/ReactErrorUtils-test.js
-* should call the callback with only the passed argument
-* should catch errors
+* should call the callback the passed arguments
+* should call the callback with the provided context
+* should return a caught error
+* should return null if no error is thrown
 * should rethrow caught errors
-* should call the callback with only the passed argument
-* should use invokeGuardedCallbackWithCatch in production
+* should call the callback the passed arguments
+* should call the callback with the provided context
+* should return a caught error
+* should return null if no error is thrown
+* should use invokeGuardedCallbackProd in production
 
 src/renderers/shared/utils/__tests__/accumulateInto-test.js
 * throws if the second item is null

src/renderers/native/__tests__/ReactNativeEvents-test.js

@@ -13,7 +13,6 @@
 
 var RCTEventEmitter;
 var React;
-var ReactErrorUtils;
 var ReactNative;
 var ResponderEventPlugin;
 var UIManager;
@@ -24,16 +23,10 @@ beforeEach(() => {
 
   RCTEventEmitter = require('RCTEventEmitter');
   React = require('React');
-  ReactErrorUtils = require('ReactErrorUtils');
   ReactNative = require('ReactNative');
   ResponderEventPlugin = require('ResponderEventPlugin');
   UIManager = require('UIManager');
   createReactNativeComponentClass = require('createReactNativeComponentClass');
-
-  // Ensure errors from event callbacks are properly surfaced (otherwise,
-  // jest/jsdom swallows them when we do the .dispatchEvent call)
-  ReactErrorUtils.invokeGuardedCallback =
-    ReactErrorUtils.invokeGuardedCallbackWithCatch;
 });
 
 it('handles events', () => {

src/renderers/shared/shared/event/EventPluginUtils.js

@@ -89,15 +89,7 @@ if (__DEV__) {
 function executeDispatch(event, simulated, listener, inst) {
   var type = event.type || 'unknown-event';
   event.currentTarget = EventPluginUtils.getNodeFromInstance(inst);
-  if (simulated) {
-    ReactErrorUtils.invokeGuardedCallbackWithCatch(
-      type,
-      listener,
-      event
-    );
-  } else {
-    ReactErrorUtils.invokeGuardedCallback(type, listener, event);
-  }
+  ReactErrorUtils.invokeGuardedCallbackAndCatchFirstError(type, listener, undefined, event);
   event.currentTarget = null;
 }
 

src/renderers/shared/stack/reconciler/ReactCompositeComponent.js

@@ -557,7 +557,7 @@ var ReactCompositeComponent = {
       if (safely) {
         if (!skipLifecycle) {
           var name = this.getName() + '.componentWillUnmount()';
-          ReactErrorUtils.invokeGuardedCallback(name, inst.componentWillUnmount.bind(inst));
+          ReactErrorUtils.invokeGuardedCallbackAndCatchFirstError(name, inst.componentWillUnmount, inst);
         }
       } else {
         if (__DEV__) {

src/renderers/shared/utils/ReactErrorUtils.js

@@ -16,33 +16,62 @@ var caughtError = null;
 
 /**
  * Call a function while guarding against errors that happens within it.
+ * Returns an error if it throws, otherwise null.
  *
  * @param {String} name of the guard to use for logging or debugging
  * @param {Function} func The function to invoke
- * @param {*} a Argument
+ * @param {*} context The context to use when calling the function
+ * @param {...*} args Arguments for function
  */
-function invokeGuardedCallback<A>(
-  name: string,
-  func: (a: A) => void,
+function invokeGuardedCallback<A, B, C, D, E, F, Context>(
+  name: string | null,
+  func: (A, B, C, D, E, F) => void,
+  context: Context,
   a: A,
-): void {
+  b: B,
+  c: C,
+  d: D,
+  e: E,
+  f: F,
+): Error | null {
+  const funcArgs = Array.prototype.slice.call(arguments, 3);
   try {
-    func(a);
-  } catch (x) {
-    if (caughtError === null) {
-      caughtError = x;
-    }
+    func.apply(context, funcArgs);
+  } catch (error) {
+    return error;
   }
+  return null;
 }
 
 var ReactErrorUtils = {
-  invokeGuardedCallback: invokeGuardedCallback,
+  invokeGuardedCallback,
+  invokeGuardedCallbackProd: invokeGuardedCallback,
 
   /**
-   * Invoked by ReactTestUtils.Simulate so that any errors thrown by the event
-   * handler are sure to be rethrown by rethrowCaughtError.
+   * Same as invokeGuardedCallback, but instead of returning an error, it stores
+   * it in a global so it can be rethrown by `rethrowCaughtError` later.
+   *
+   * @param {String} name of the guard to use for logging or debugging
+   * @param {Function} func The function to invoke
+   * @param {*} context The context to use when calling the function
+   * @param {...*} args Arguments for function
    */
-  invokeGuardedCallbackWithCatch: invokeGuardedCallback,
+  invokeGuardedCallbackAndCatchFirstError: function<A, B, C, D, E, F, Context>(
+    name: string | null,
+    func: (A, B, C, D, E, F) => void,
+    context: Context,
+    a: A,
+    b: B,
+    c: C,
+    d: D,
+    e: E,
+    f: F,
+  ): void {
+    const error = ReactErrorUtils.invokeGuardedCallback.apply(this, arguments);
+    if (error !== null && caughtError === null) {
+      caughtError = error;
+    }
+  },
 
   /**
    * During execution of guarded functions we will capture the first error which
@@ -66,21 +95,39 @@ if (__DEV__) {
       typeof window.dispatchEvent === 'function' &&
       typeof document !== 'undefined' &&
       typeof document.createEvent === 'function') {
-    var fakeNode = document.createElement('react');
-    ReactErrorUtils.invokeGuardedCallback = function<A>(
-      name: string,
-      func: (a: A) => void,
-      a: A,
-    ): void {
-      var boundFunc = function() {
-        func(a);
+
+    const fakeNode = document.createElement('react');
+    let fakeEventError = null;
+    const onFakeEventError = function(event) {
+      fakeEventError = event.error;
+    };
+
+    ReactErrorUtils.invokeGuardedCallback = function(
+      name,
+      func,
+      context,
+      a,
+      b,
+      c,
+      d,
+      e,
+      f
+    ) {
+      const funcArgs = Array.prototype.slice.call(arguments, 3);
+      const boundFunc = function() {
+        func.apply(context, funcArgs);
       };
-      var evtType = `react-${name}`;
+      const evtType = 'react-' + (name ? name : 'invokeguardedcallback');
+      window.addEventListener('error', onFakeEventError);
       fakeNode.addEventListener(evtType, boundFunc, false);
-      var evt = document.createEvent('Event');
+      const evt = document.createEvent('Event');
       evt.initEvent(evtType, false, false);
       fakeNode.dispatchEvent(evt);
       fakeNode.removeEventListener(evtType, boundFunc, false);
+      window.removeEventListener('error', onFakeEventError);
+      const returnVal = fakeEventError;
+      fakeEventError = null;
+      return returnVal;
     };
   }
 }

src/renderers/shared/utils/__tests__/ReactErrorUtils-test.js

@@ -19,44 +19,79 @@ describe('ReactErrorUtils', () => {
     ReactErrorUtils = require('ReactErrorUtils');
   });
 
-  describe('invokeGuardedCallbackWithCatch', () => {
-    it('should call the callback with only the passed argument', () => {
+  describe('invokeGuardedCallbackProd', () => {
+    it('should call the callback the passed arguments', () => {
       var callback = jest.fn();
-      ReactErrorUtils.invokeGuardedCallbackWithCatch('foo', callback, 'arg');
-      expect(callback).toBeCalledWith('arg');
+      ReactErrorUtils.invokeGuardedCallbackProd('foo', callback, null, 'arg1', 'arg2');
+      expect(callback).toBeCalledWith('arg1', 'arg2');
     });
 
-    it('should catch errors', () => {
-      var callback = function() {
-        throw new Error('foo');
-      };
-      expect(
-        () => ReactErrorUtils.invokeGuardedCallbackWithCatch('foo', callback)
-      ).not.toThrow();
+    it('should call the callback with the provided context', () => {
+      var context = { didCall: false };
+      ReactErrorUtils.invokeGuardedCallbackProd('foo', function() {
+        this.didCall = true;
+      }, context);
+      expect(context.didCall).toBe(true);
+    });
+
+    it('should return a caught error', () => {
+      const error = new Error();
+      const returnValue = ReactErrorUtils.invokeGuardedCallbackProd('foo', function() {
+        throw error;
+      }, null, 'arg1', 'arg2');
+      expect(returnValue).toBe(error);
+    });
+
+    it('should return null if no error is thrown', () => {
+      var callback = jest.fn();
+      const returnValue = ReactErrorUtils.invokeGuardedCallbackProd('foo', callback, null);
+      expect(returnValue).toBe(null);
     });
   });
 
-  describe('rethrowCaughtError', () => {
+  describe('invokeGuardedCallbackAndCatchFirstError', () => {
     it('should rethrow caught errors', () => {
       var err = new Error('foo');
       var callback = function() {
         throw err;
       };
-      ReactErrorUtils.invokeGuardedCallbackWithCatch('foo', callback);
+      ReactErrorUtils.invokeGuardedCallbackAndCatchFirstError('foo', callback, null);
       expect(() => ReactErrorUtils.rethrowCaughtError()).toThrow(err);
     });
   });
 
   describe('invokeGuardedCallback', () => {
-    it('should call the callback with only the passed argument', () => {
+    it('should call the callback the passed arguments', () => {
+      var callback = jest.fn();
+      ReactErrorUtils.invokeGuardedCallback('foo', callback, null, 'arg1', 'arg2');
+      expect(callback).toBeCalledWith('arg1', 'arg2');
+    });
+
+    it('should call the callback with the provided context', () => {
+      var context = { didCall: false };
+      ReactErrorUtils.invokeGuardedCallback('foo', function() {
+        this.didCall = true;
+      }, context);
+      expect(context.didCall).toBe(true);
+    });
+
+    it('should return a caught error', () => {
+      const error = new Error();
+      const returnValue = ReactErrorUtils.invokeGuardedCallback('foo', function() {
+        throw error;
+      }, null, 'arg1', 'arg2');
+      expect(returnValue).toBe(error);
+    });
+
+    it('should return null if no error is thrown', () => {
       var callback = jest.fn();
-      ReactErrorUtils.invokeGuardedCallback('foo', callback, 'arg');
-      expect(callback).toBeCalledWith('arg');
+      const returnValue = ReactErrorUtils.invokeGuardedCallback('foo', callback, null);
+      expect(returnValue).toBe(null);
     });
 
-    it('should use invokeGuardedCallbackWithCatch in production', () => {
+    it('should use invokeGuardedCallbackProd in production', () => {
       expect(ReactErrorUtils.invokeGuardedCallback).not.toEqual(
-        ReactErrorUtils.invokeGuardedCallbackWithCatch
+        ReactErrorUtils.invokeGuardedCallbackProd
       );
       __DEV__ = false;
       var oldProcess = process;
@@ -66,7 +101,7 @@ describe('ReactErrorUtils', () => {
       jest.resetModules();
       ReactErrorUtils = require('ReactErrorUtils');
       expect(ReactErrorUtils.invokeGuardedCallback).toEqual(
-        ReactErrorUtils.invokeGuardedCallbackWithCatch
+        ReactErrorUtils.invokeGuardedCallbackProd
       );
       __DEV__ = true;
       global.process = oldProcess;