Jest master doesn't seem to catch errors originating from React

[gaearon]

As reported in #4761 (comment), I tried 21.3.0-beta.3 on React codebase, and tests like

    test('wat', () => {
      const container = document.createElement('div');
      function App() {
        throw new Error('hi');
      }
      expect(() => {
        ReactDOM.render(<App />, container);
      }).toThrow();
    });

now fail. (The error blows up the test despite bring caught.)

However, they pass with 21.2.1.

To repro, you can take the test above, put it into ReactDOMComponent-test on React master, update Jest packages to 21.3.0-beta.3 and run yarn test ReactDOMComponent-test.

I haven't had time to verify if it's isolated to React Jest setup or not. But if it affects us, could affect someone else (or everyone) too.

The only unusual thing we do in React wrt error handling is that we wrap our code into a browser event so that browsers report unhandled (but caught and rethrown by us) errors as unhandled. This used to work with Jest too. You can disable this logic by changing __DEV__ to false here and then you'll see it goes back to normal (test passes).

So something related to how errors are handled in jsdom browser event loop has changed, and will likely break any tests relying on throwing (and intentionally catching) errors with React 16. Maybe jsdom was updated or something like this?

Note that if this got broken, it will affect anyone using React, not just us.

Here is the diff of lockfile: https://gist.github.com/gaearon/fc30d89381e6b4c6a2494b66f04455cf. Something here causes the issue.

[SimenB]

@gaearon there is #4669 which feels like it might be related. Could you try reverting that changeset and see if the error is gone? (On mobile, so can't test it myself ATM)

[gaearon]

Pretty sure this is related. Will test in the office.

[SimenB]

Yeah, based on your explanation of how React emits browser errors, it looks like a smoking gun.

We can make the behaviour opt-out to accommodate React's use case, but I don't think it should be reverted.
Normally I guess we'd ask developers to create their own custom test environment, but seeing as it's React's test suite we're breaking, I'm guessing we will adapt the environment which comes out of the box 🙂

[gaearon]

If this is working as designed, what is the suggested way to test catching errors inside React components? It currently seems impossible.

(To be clear, it doesn't affect just React test suite, it affects any project using React.)

[gaearon]

Would it make sense to somehow track if we’re inside toThrow() and in that case, not treat it as an unhandled error?

[cpojer]

If you can make that work, I’d be ok with that as a solution.

…

________________________________ From: Dan Abramov <notifications@github.com> Sent: Thursday, October 26, 2017 11:11:18 AM To: facebook/jest Cc: Subscribed Subject: Re: [facebook/jest] Jest master doesn't seem to catch errors originating from React (#4764) Would it make sense to somehow track if were inside toThrow() and in that case, not treat it as an unhandled error? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub<#4764 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AAA0KKficrUYb3Z4xSxDlMt5oTfmQeGrks5swFrGgaJpZM4QG3MP>.

[SimenB]

What about instead of emitting uncaughtException we just rethrow? Not sure if it'll be caught in the try-catch of toThrow or if it happens in a later tick so it's too late

[gaearon]

Here's an integration test for ya. #4767

[gaearon]

What about instead of emitting uncaughtException we just rethrow

Seems like this fails the test from #4669. It never finishes.

[gaearon]

Maybe we could override global.addEventListener and count error listeners. If there is more than zero when the error is caught, assume user is running their own (or React's) error handling mechanism and don't report it as unhandled.

[SimenB]

That sounds interesting. Something like

const originalAddListener = this.global.addEventListener;

this.global.addEventListener = (name, ...args) => {
  if (name === 'error') {
    this.global.removeEventListener('error', this.errorEventListener);
  }

  return originalAddListener(name, ...args);
};

?

[SimenB]

Seems to work. I'll push it to Dan's branch to trigger CI

[ooade]

@gaearon something seems strange with this process. Although jest now catches the error because the test passes, yet it still displays them to the console.

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.