Skip to content

Commit

Permalink
Always rethrow original error when we replay errors (facebook#20425)
Browse files Browse the repository at this point in the history
We replay errors so you can break on paused exceptions. This is done in
the second pass so that the first pass can ignore suspense.

Originally this threw the original error. For suppression purposes
we copied the flag onto the original error.

https://github.com/facebook/react/blob/f1dc626b29b8bf0f14c75a8525e8650b7ea94a47/packages/react-reconciler/src/ReactFiberScheduler.old.js#L367-L369

During this refactor it changed to just throw the retried error:

facebook#15151

We're not sure exactly why but it was likely just an optimization or
clean up.

So we can go back to throwing the original error. That helps in the case
where a memoized function is naively not rethrowing each time such as
in Node's module system.

Unfortunately this doesn't fix the problem fully.
Because invokeGuardedCallback captures the error and logs it to the browser.
So you still end up seeing the wrong message in the logs.

This just fixes so that the error boundary sees the first one.
  • Loading branch information
sebmarkbage authored Dec 10, 2020
1 parent b15d6e9 commit cdfde3a
Show file tree
Hide file tree
Showing 4 changed files with 4,069 additions and 2,685 deletions.
29 changes: 29 additions & 0 deletions fixtures/dom/src/components/fixtures/error-handling/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,20 @@ class TrySilenceFatalError extends React.Component {
}
}

function naiveMemoize(fn) {
let memoizedEntry;
return function() {
if (!memoizedEntry) {
memoizedEntry = {result: null};
memoizedEntry.result = fn();
}
return memoizedEntry.result;
};
}
let memoizedFunction = naiveMemoize(function() {
throw new Error('Passed');
});

export default class ErrorHandlingTestCases extends React.Component {
render() {
return (
Expand Down Expand Up @@ -288,6 +302,21 @@ export default class ErrorHandlingTestCases extends React.Component {
}}
/>
</TestCase>
<TestCase title="Throwing memoized result" description="">
<TestCase.Steps>
<li>Click the "Trigger error" button</li>
<li>Click the reset button</li>
</TestCase.Steps>
<TestCase.ExpectedResult>
The "Trigger error" button should be replaced with "Captured an
error: Passed". Clicking reset should reset the test case.
</TestCase.ExpectedResult>
<Example
doThrow={() => {
memoizedFunction().value;
}}
/>
</TestCase>
<TestCase
title="Cross-origin errors (development mode only)"
description="">
Expand Down
Loading

0 comments on commit cdfde3a

Please sign in to comment.