Skip to content

Commit f4ad51d

Browse files
committed
invokeGuardedCallback should work when nested
Added a guard in the global error event listener to prevent nested errors from being captured higher up the stack. Also found a bug where the DEV version of invokeGuardedCallback would infinite loop if there were nested invocations with the same name. Fixed by appending the depth to the fake event name. (I'm actually not sure why this is necessary.)
1 parent 49137df commit f4ad51d

File tree

3 files changed

+43
-8
lines changed

3 files changed

+43
-8
lines changed

scripts/fiber/tests-passing.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1670,6 +1670,8 @@ src/renderers/shared/utils/__tests__/ReactErrorUtils-test.js
16701670
* should call the callback with the provided context
16711671
* should return a caught error
16721672
* should return null if no error is thrown
1673+
* can nest with same debug name
1674+
* does not return nested errors
16731675
* should use invokeGuardedCallbackProd in production
16741676

16751677
src/renderers/shared/utils/__tests__/accumulateInto-test.js

src/renderers/shared/utils/ReactErrorUtils.js

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -97,10 +97,7 @@ if (__DEV__) {
9797
typeof document.createEvent === 'function') {
9898

9999
const fakeNode = document.createElement('react');
100-
let fakeEventError = null;
101-
const onFakeEventError = function(event) {
102-
fakeEventError = event.error;
103-
};
100+
let depth = 0;
104101

105102
ReactErrorUtils.invokeGuardedCallback = function(
106103
name,
@@ -113,21 +110,29 @@ if (__DEV__) {
113110
e,
114111
f
115112
) {
113+
depth++;
114+
const thisDepth = depth;
116115
const funcArgs = Array.prototype.slice.call(arguments, 3);
117116
const boundFunc = function() {
118117
func.apply(context, funcArgs);
119118
};
120-
const evtType = 'react-' + (name ? name : 'invokeguardedcallback');
119+
let fakeEventError = null;
120+
const onFakeEventError = function(event) {
121+
// Don't capture nested errors
122+
if (depth === thisDepth) {
123+
fakeEventError = event.error;
124+
}
125+
};
126+
const evtType = `react-${name ? name : 'invokeguardedcallback'}-${depth}`;
121127
window.addEventListener('error', onFakeEventError);
122128
fakeNode.addEventListener(evtType, boundFunc, false);
123129
const evt = document.createEvent('Event');
124130
evt.initEvent(evtType, false, false);
125131
fakeNode.dispatchEvent(evt);
126132
fakeNode.removeEventListener(evtType, boundFunc, false);
127133
window.removeEventListener('error', onFakeEventError);
128-
const returnVal = fakeEventError;
129-
fakeEventError = null;
130-
return returnVal;
134+
depth--;
135+
return fakeEventError;
131136
};
132137
}
133138
}

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,34 @@ describe('ReactErrorUtils', () => {
8989
expect(returnValue).toBe(null);
9090
});
9191

92+
it('can nest with same debug name', () => {
93+
const err1 = new Error();
94+
let err2;
95+
const err3 = new Error();
96+
const err4 = ReactErrorUtils.invokeGuardedCallback('foo', function() {
97+
err2 = ReactErrorUtils.invokeGuardedCallback('foo', function() {
98+
throw err1;
99+
}, null);
100+
throw err3;
101+
}, null);
102+
103+
expect(err2).toBe(err1);
104+
expect(err4).toBe(err3);
105+
});
106+
107+
it('does not return nested errors', () => {
108+
const err1 = new Error();
109+
let err2;
110+
const err3 = ReactErrorUtils.invokeGuardedCallback('foo', function() {
111+
err2 = ReactErrorUtils.invokeGuardedCallback('foo', function() {
112+
throw err1;
113+
}, null);
114+
}, null);
115+
116+
expect(err3).toBe(null); // Returns null because inner error was already captured
117+
expect(err2).toBe(err1);
118+
});
119+
92120
it('should use invokeGuardedCallbackProd in production', () => {
93121
expect(ReactErrorUtils.invokeGuardedCallback).not.toEqual(
94122
ReactErrorUtils.invokeGuardedCallbackProd

0 commit comments

Comments
 (0)