Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unhandled error is displayed twice #10384

Closed
gaearon opened this issue Aug 4, 2017 · 12 comments
Closed

Unhandled error is displayed twice #10384

gaearon opened this issue Aug 4, 2017 · 12 comments

Comments

@gaearon
Copy link
Collaborator

gaearon commented Aug 4, 2017

This was a bit weird:

screen shot 2017-08-04 at 3 51 29 pm

Look at the last log.

Notice how our internal error that we're supposed to just give to the error boundary is said to be uncaught. I wonder why. It doesn't look like intentional.

@gaearon
Copy link
Collaborator Author

gaearon commented Aug 4, 2017

Hmm. I didn’t see this before because I was testing the “swallowing Promise” case specifically.

@gaearon
Copy link
Collaborator Author

gaearon commented Aug 4, 2017

The annoying thing is that even if you set up CORS, you still see the same error twice. Unless you’re swallowing it.

screen shot 2017-08-04 at 4 30 52 pm

Once because it is thrown in the inner event, and the other time because it is rethrown in the scheduler. And we can’t avoid rethrowing because that would change the flow between DEV and PROD.

I wish there was just some way to silence the second one.

@gaearon gaearon changed the title Internal error is displayed as uncaught Unhandled error is displayed twice Aug 4, 2017
@gaearon
Copy link
Collaborator Author

gaearon commented Oct 4, 2017

For context, the reason it happens is because we intentionally let the browser interpret an error as uncaught before we rethrow. The justification is discussed in #10474.

If you follow the advice in the error and add an error boundary (which you should!), you will only see the error once. So that's not a huge problem in day-to-day workflow. In fact it nudges people to add error boundaries which is nice.

Another justification for this is that if you accidentally swallow an error we still print it once. That alone makes this worth it IMO. We’ve had hundreds of comments and dozens of issues caused by people swallowing their own errors, and the fact that we will print them at least once now seems like enough benefit to compensate for printing them twice when they’re uncaught, un-swallowed, and don’t have a boundary.

@mjomble
Copy link
Contributor

mjomble commented Oct 29, 2017

Is there currently a way to prevent dispatching the error event twice in a case as simple as this?

window.addEventListener('error', () => console.log('error event dispatched'))
const onClick = () => { throw new Error('foo') }
ReactDOM.render(React.createElement('button', { onClick }, 'test'), container)

Because the error is thrown in a click handler rather than render(), error boundaries don't seem to help.

PS. Not sure if this belongs here or a on different issue or if I should open a new one.

@gaearon
Copy link
Collaborator Author

gaearon commented Oct 29, 2017

Does an approach like #10474 (comment) help?

@mjomble
Copy link
Contributor

mjomble commented Oct 29, 2017

Probably. Something like this works as well:

window.addEventListener('error', (evt) => {
  if (evt.error.markedForReactWeirdness) {
    evt.preventDefault()
    return
  }

  evt.error.markedForReactWeirdness = true
  // ...regular error handling here...
})

But I considered that a temporary hack, hoping that a more elegant solution is (or will be) available :)

@gaearon
Copy link
Collaborator Author

gaearon commented Oct 29, 2017

Why is this important for you? The duplication only happens in development mode. And presumably error logging etc should only happen in production.

@mjomble
Copy link
Contributor

mjomble commented Oct 29, 2017

Ah, good point, I forgot that this wouldn't be an issue in production mode.

In that case it's just more of a minor annoyance.
My app displays errors caught in the global handler, so while developing the error display feature, they always show up twice, unless I hack the duplicates away.

@gaearon
Copy link
Collaborator Author

gaearon commented Oct 31, 2017

Tagging as wontfix.
The behavior is intentional as explained in #10384 (comment).

@gaearon
Copy link
Collaborator Author

gaearon commented Jan 5, 2018

If someone comes here looking for a solution for jsdom, a few related links:

jestjs/jest#5223 (comment)
#11098 (comment)

@jgentes
Copy link

jgentes commented Jan 5, 2018

@gaearon could you clarify what you mean by The duplication only happens in development mode. I'm running my app in production mode, with webpack, etc, and I see duplicate errors in my console (and sent to Bugsnag).

My app config specifies the boundary above Provider:

ReactDOM.render(
      <ErrorBoundary>
        <Provider store={store}>
          <Routes history={browserHistory}/>
        </Provider>
      </ErrorBoundary>,
      document.getElementById('app')
    );

@gaearon
Copy link
Collaborator Author

gaearon commented Jan 6, 2018

@jgentes "The above error..." stuff is only logged in development.

If you're seeing some other symptoms please file a new issue with an example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants