@samitouri / QOS-React / commits / 0304008a65

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.

Andrew Clark committed Feb 8, 2017 at 14:20 UTC 0304008a6541f8e235d7fda653746ed9ce25ef10
6 files changed +136 -64
scripts/fiber/tests-passing.txt
+9 -4
@@ -1667,11 +1667,16 @@ src/renderers/shared/stack/reconciler/__tests__/Transaction-test.js
1667 * should allow nesting of transactions
1668
1669 src/renderers/shared/utils/__tests__/ReactErrorUtils-test.js
1670 -* should call the callback with only the passed argument
1671 -* should catch errors
1670 +* should call the callback the passed arguments
1671 +* should call the callback with the provided context
1672 +* should return a caught error
1673 +* should return null if no error is thrown
1674 * should rethrow caught errors
1673 -* should call the callback with only the passed argument
1674 -* should use invokeGuardedCallbackWithCatch in production
1675 +* should call the callback the passed arguments
1676 +* should call the callback with the provided context
1677 +* should return a caught error
1678 +* should return null if no error is thrown
1679 +* should use invokeGuardedCallbackProd in production
1680
1681 src/renderers/shared/utils/__tests__/accumulateInto-test.js
1682 * throws if the second item is null
src/renderers/native/__tests__/ReactNativeEvents-test.js
-7
@@ -13,7 +13,6 @@
13
14 var RCTEventEmitter;
15 var React;
16 -var ReactErrorUtils;
16 var ReactNative;
17 var ResponderEventPlugin;
18 var UIManager;
@@ -24,16 +23,10 @@ beforeEach(() => {
23
24 RCTEventEmitter = require('RCTEventEmitter');
25 React = require('React');
27 - ReactErrorUtils = require('ReactErrorUtils');
26 ReactNative = require('ReactNative');
27 ResponderEventPlugin = require('ResponderEventPlugin');
28 UIManager = require('UIManager');
29 createReactNativeComponentClass = require('createReactNativeComponentClass');
32 -
33 - // Ensure errors from event callbacks are properly surfaced (otherwise,
34 - // jest/jsdom swallows them when we do the .dispatchEvent call)
35 - ReactErrorUtils.invokeGuardedCallback =
36 - ReactErrorUtils.invokeGuardedCallbackWithCatch;
30 });
31
32 it('handles events', () => {
src/renderers/shared/shared/event/EventPluginUtils.js
+1 -9
@@ -89,15 +89,7 @@ if (__DEV__) {
89 function executeDispatch(event, simulated, listener, inst) {
90 var type = event.type || 'unknown-event';
91 event.currentTarget = EventPluginUtils.getNodeFromInstance(inst);
92 - if (simulated) {
93 - ReactErrorUtils.invokeGuardedCallbackWithCatch(
94 - type,
95 - listener,
96 - event
97 - );
98 - } else {
99 - ReactErrorUtils.invokeGuardedCallback(type, listener, event);
100 - }
92 + ReactErrorUtils.invokeGuardedCallbackAndCatchFirstError(type, listener, undefined, event);
93 event.currentTarget = null;
94 }
95
src/renderers/shared/stack/reconciler/ReactCompositeComponent.js
+1 -1
@@ -557,7 +557,7 @@ var ReactCompositeComponent = {
557 if (safely) {
558 if (!skipLifecycle) {
559 var name = this.getName() + '.componentWillUnmount()';
560 - ReactErrorUtils.invokeGuardedCallback(name, inst.componentWillUnmount.bind(inst));
560 + ReactErrorUtils.invokeGuardedCallbackAndCatchFirstError(name, inst.componentWillUnmount, inst);
561 }
562 } else {
563 if (__DEV__) {
src/renderers/shared/utils/ReactErrorUtils.js
+71 -24
@@ -16,33 +16,62 @@ var caughtError = null;
16
17 /**
18 * Call a function while guarding against errors that happens within it.
19 + * Returns an error if it throws, otherwise null.
20 *
21 * @param {String} name of the guard to use for logging or debugging
22 * @param {Function} func The function to invoke
22 - * @param {*} a Argument
23 + * @param {*} context The context to use when calling the function
24 + * @param {...*} args Arguments for function
25 */
24 -function invokeGuardedCallback<A>(
25 - name: string,
26 - func: (a: A) => void,
26 +function invokeGuardedCallback<A, B, C, D, E, F, Context>(
27 + name: string | null,
28 + func: (A, B, C, D, E, F) => void,
29 + context: Context,
30 a: A,
28 -): void {
31 + b: B,
32 + c: C,
33 + d: D,
34 + e: E,
35 + f: F,
36 +): Error | null {
37 + const funcArgs = Array.prototype.slice.call(arguments, 3);
38 try {
30 - func(a);
31 - } catch (x) {
32 - if (caughtError === null) {
33 - caughtError = x;
34 - }
39 + func.apply(context, funcArgs);
40 + } catch (error) {
41 + return error;
42 }
43 + return null;
44 }
45
46 var ReactErrorUtils = {
39 - invokeGuardedCallback: invokeGuardedCallback,
47 + invokeGuardedCallback,
48 + invokeGuardedCallbackProd: invokeGuardedCallback,
49
50 /**
42 - * Invoked by ReactTestUtils.Simulate so that any errors thrown by the event
43 - * handler are sure to be rethrown by rethrowCaughtError.
51 + * Same as invokeGuardedCallback, but instead of returning an error, it stores
52 + * it in a global so it can be rethrown by `rethrowCaughtError` later.
53 + *
54 + * @param {String} name of the guard to use for logging or debugging
55 + * @param {Function} func The function to invoke
56 + * @param {*} context The context to use when calling the function
57 + * @param {...*} args Arguments for function
58 */
45 - invokeGuardedCallbackWithCatch: invokeGuardedCallback,
59 + invokeGuardedCallbackAndCatchFirstError: function<A, B, C, D, E, F, Context>(
60 + name: string | null,
61 + func: (A, B, C, D, E, F) => void,
62 + context: Context,
63 + a: A,
64 + b: B,
65 + c: C,
66 + d: D,
67 + e: E,
68 + f: F,
69 + ): void {
70 + const error = ReactErrorUtils.invokeGuardedCallback.apply(this, arguments);
71 + if (error !== null && caughtError === null) {
72 + caughtError = error;
73 + }
74 + },
75
76 /**
77 * During execution of guarded functions we will capture the first error which
@@ -66,21 +95,39 @@ if (__DEV__) {
95 typeof window.dispatchEvent === 'function' &&
96 typeof document !== 'undefined' &&
97 typeof document.createEvent === 'function') {
69 - var fakeNode = document.createElement('react');
70 - ReactErrorUtils.invokeGuardedCallback = function<A>(
71 - name: string,
72 - func: (a: A) => void,
73 - a: A,
74 - ): void {
75 - var boundFunc = function() {
76 - func(a);
98 +
99 + const fakeNode = document.createElement('react');
100 + let fakeEventError = null;
101 + const onFakeEventError = function(event) {
102 + fakeEventError = event.error;
103 + };
104 +
105 + ReactErrorUtils.invokeGuardedCallback = function(
106 + name,
107 + func,
108 + context,
109 + a,
110 + b,
111 + c,
112 + d,
113 + e,
114 + f
115 + ) {
116 + const funcArgs = Array.prototype.slice.call(arguments, 3);
117 + const boundFunc = function() {
118 + func.apply(context, funcArgs);
119 };
78 - var evtType = `react-${name}`;
120 + const evtType = 'react-' + (name ? name : 'invokeguardedcallback');
121 + window.addEventListener('error', onFakeEventError);
122 fakeNode.addEventListener(evtType, boundFunc, false);
80 - var evt = document.createEvent('Event');
123 + const evt = document.createEvent('Event');
124 evt.initEvent(evtType, false, false);
125 fakeNode.dispatchEvent(evt);
126 fakeNode.removeEventListener(evtType, boundFunc, false);
127 + window.removeEventListener('error', onFakeEventError);
128 + const returnVal = fakeEventError;
129 + fakeEventError = null;
130 + return returnVal;
131 };
132 }
133 }
src/renderers/shared/utils/__tests__/ReactErrorUtils-test.js
+54 -19
@@ -19,44 +19,79 @@ describe('ReactErrorUtils', () => {
19 ReactErrorUtils = require('ReactErrorUtils');
20 });
21
22 - describe('invokeGuardedCallbackWithCatch', () => {
23 - it('should call the callback with only the passed argument', () => {
22 + describe('invokeGuardedCallbackProd', () => {
23 + it('should call the callback the passed arguments', () => {
24 var callback = jest.fn();
25 - ReactErrorUtils.invokeGuardedCallbackWithCatch('foo', callback, 'arg');
26 - expect(callback).toBeCalledWith('arg');
25 + ReactErrorUtils.invokeGuardedCallbackProd('foo', callback, null, 'arg1', 'arg2');
26 + expect(callback).toBeCalledWith('arg1', 'arg2');
27 });
28
29 - it('should catch errors', () => {
30 - var callback = function() {
31 - throw new Error('foo');
32 - };
33 - expect(
34 - () => ReactErrorUtils.invokeGuardedCallbackWithCatch('foo', callback)
35 - ).not.toThrow();
29 + it('should call the callback with the provided context', () => {
30 + var context = { didCall: false };
31 + ReactErrorUtils.invokeGuardedCallbackProd('foo', function() {
32 + this.didCall = true;
33 + }, context);
34 + expect(context.didCall).toBe(true);
35 + });
36 +
37 + it('should return a caught error', () => {
38 + const error = new Error();
39 + const returnValue = ReactErrorUtils.invokeGuardedCallbackProd('foo', function() {
40 + throw error;
41 + }, null, 'arg1', 'arg2');
42 + expect(returnValue).toBe(error);
43 + });
44 +
45 + it('should return null if no error is thrown', () => {
46 + var callback = jest.fn();
47 + const returnValue = ReactErrorUtils.invokeGuardedCallbackProd('foo', callback, null);
48 + expect(returnValue).toBe(null);
49 });
50 });
51
39 - describe('rethrowCaughtError', () => {
52 + describe('invokeGuardedCallbackAndCatchFirstError', () => {
53 it('should rethrow caught errors', () => {
54 var err = new Error('foo');
55 var callback = function() {
56 throw err;
57 };
45 - ReactErrorUtils.invokeGuardedCallbackWithCatch('foo', callback);
58 + ReactErrorUtils.invokeGuardedCallbackAndCatchFirstError('foo', callback, null);
59 expect(() => ReactErrorUtils.rethrowCaughtError()).toThrow(err);
60 });
61 });
62
63 describe('invokeGuardedCallback', () => {
51 - it('should call the callback with only the passed argument', () => {
64 + it('should call the callback the passed arguments', () => {
65 + var callback = jest.fn();
66 + ReactErrorUtils.invokeGuardedCallback('foo', callback, null, 'arg1', 'arg2');
67 + expect(callback).toBeCalledWith('arg1', 'arg2');
68 + });
69 +
70 + it('should call the callback with the provided context', () => {
71 + var context = { didCall: false };
72 + ReactErrorUtils.invokeGuardedCallback('foo', function() {
73 + this.didCall = true;
74 + }, context);
75 + expect(context.didCall).toBe(true);
76 + });
77 +
78 + it('should return a caught error', () => {
79 + const error = new Error();
80 + const returnValue = ReactErrorUtils.invokeGuardedCallback('foo', function() {
81 + throw error;
82 + }, null, 'arg1', 'arg2');
83 + expect(returnValue).toBe(error);
84 + });
85 +
86 + it('should return null if no error is thrown', () => {
87 var callback = jest.fn();
53 - ReactErrorUtils.invokeGuardedCallback('foo', callback, 'arg');
54 - expect(callback).toBeCalledWith('arg');
88 + const returnValue = ReactErrorUtils.invokeGuardedCallback('foo', callback, null);
89 + expect(returnValue).toBe(null);
90 });
91
57 - it('should use invokeGuardedCallbackWithCatch in production', () => {
92 + it('should use invokeGuardedCallbackProd in production', () => {
93 expect(ReactErrorUtils.invokeGuardedCallback).not.toEqual(
59 - ReactErrorUtils.invokeGuardedCallbackWithCatch
94 + ReactErrorUtils.invokeGuardedCallbackProd
95 );
96 __DEV__ = false;
97 var oldProcess = process;
@@ -66,7 +101,7 @@ describe('ReactErrorUtils', () => {
101 jest.resetModules();
102 ReactErrorUtils = require('ReactErrorUtils');
103 expect(ReactErrorUtils.invokeGuardedCallback).toEqual(
69 - ReactErrorUtils.invokeGuardedCallbackWithCatch
104 + ReactErrorUtils.invokeGuardedCallbackProd
105 );
106 __DEV__ = true;
107 global.process = oldProcess;