The fake event trick for rethrowing errors in DEV fires unexpected global error handlers and makes testing harder

I'm trying to make use of componentDidCatch in the React 16 beta. I already had a global window error handler which was working fine, but it unexpectedly catches errors that I would expect componentDidCatch to have handled. That is, component-local errors are being treated as window-global errors in dev builds.

The problem seems to stem from invokeGuardedCallbackDev in ReactErrorUtils.js. I think that this entire __DEV__ block of code is problematic. The stated rational is:

  // In DEV mode, we swap out invokeGuardedCallback for a special version
  // that plays more nicely with the browser's DevTools. The idea is to preserve
  // "Pause on exceptions" behavior. Because React wraps all user-provided
  // functions in invokeGuardedCallback, and the production version of
  // invokeGuardedCallback uses a try-catch, all user exceptions are treated
  // like caught exceptions, and the DevTools won't pause unless the developer
  // takes the extra step of enabling pause on caught exceptions. This is
  // untintuitive, though, because even though React has caught the error, from
  // the developer's perspective, the error is uncaught.

This is misguided because it's not about pausing on exceptions, it's about "pause on uncaught exceptions." However, componentDidCatch makes exceptions caught!

Rather than switching on prod vs dev and using try/catch in prod and window's error handler in dev, React should always use try/catch, but rethrow if you reach the root without hitting a componentDidCatch handler. This would preserve the correct "pause on uncaught exceptions" behavior without messing with global error handlers.

Workaround:

window.addEventListener("error", function (event) {
  let {error} = event;
  // XXX Ignore errors that will be processed by componentDidCatch.
  // SEE: https://github.com/facebook/react/issues/10474
  if (error.stack && error.stack.indexOf('invokeGuardedCallbackDev') >= 0) {
    return true;
  }
  // Original event handler here...
});

The way I use that button is:

Pause on exception: I try to use this whenever possible whenever I'm trying to debug an unexpected exception in my code.
Pause on caught exception: I use this when I think the exception might get caught by a try/catch, but this also tends to include lots of feature tests which I don't care about.

In React we don't recommend you use exceptions except for programmer errors, so I prefer (as a React user) that they always show up in "pause on exception" despite there being an error boundary. Error boundaries are a failsafe check that you don't expect to hit in normal operation.

Not sure if other folks have a different perspective.

Honestly what I would love to do is annotate individual try/catch blocks in the source as "this is just a feature test" vs "this is error handling for real errors" and have the latter always stop but the former stop only in a special mode.

Whatever your preference, there are plenty of libraries out there that internally use exceptions. I've never found "pause on caught exception" to ever be useful, since it produces an incredible number of false positives. Still, what React is doing is against the grain of the browser's provided exception debugging tools (which I agree are inadequate).

One reason we chose to do it this way is that we expect the best practice for React apps will be to always have a top-level error boundary — if you don't, then the whole app will unmount whenever there's an uncaught error. So if we disable this feature inside error boundaries, we will effectively disable it for all errors that originate inside a React lifecycle or render method.

It would be nice to get some actual data on how often people rely on this feature. Personally, the number of times I've used it is small, but it's not zero.

A top-level component error boundary is fine for errors that occur during rendering or lifecycle callbacks, but does not address code that responds to network or other top-level events. For that, you need to hook window.onerror, which is precisely how I ran in to this problem.

@brandonbloom

@brandonbloom Right, I just mean to say that if we decide to always use try-catch inside error boundaries, then we're effectively deciding to use try-catch everywhere. In which case we should remove invokeGuardedCallback entirely and just use try-catch directly. That might be the right thing to do, but it's a trade-off either way.

I think it's the right thing to do, so let's discuss the tradeoff.

On one hand, by removing this logic and replacing it with standard try/catch behavior, you have 1) less code and 2) better match with the standard "pause on caught exception" behavior. And 3) There is no wonkiness necessary for dev vs prod with respect to window.onerror. The workaround for (3) is quite kludgey, even if you provided some way to better distinguish these sorts of error events.

On the other hand, debugging caught exceptions is slightly more annoying (although no more annoying than it is in general, outside of React). The workaround for this annoyance is to simply disable error boundary components, either with a configuration flag, or by commenting out componentDidCatch, just like commenting out a catch block. Moreover, if a stack-trace rendering boundary component gets popular, this workaround is unlikely to be necessary. It's only if you want to actively debug/inspect the error object. In that case, adding a debugger; statement does the trick once you get the stack trace from either the browser's console or the boundary component's rendering.

Am I overlooking other things in favor of keeping this logic?

@sebmarkbage

I think that's about right. Another benefit is it removes the problem of cross-origin errors being obscured on the error event in DEV.

@sebmarkbage had the strongest feelings about preserving this feature, if I recall correctly. There could be additional factors that I've forgotten.

I noticed some voodoo about that, but wasn't immediately affected by it, so didn't want to comment. In general, this feels like one of those situations where the complications are sufficient to justify doing less in the interest of avoiding compounding complexity.

To be clear: This doesn't just affect error boundaries. It'll have to be all errors that are "caught". Because, to preserve consistency, we effectively have a built-in boundary at the root. Even if we rethrow, that is still considered "caught". This is not common practice outside React so it would be significant downside.

The ecosystem is full of things that cause the noise. E.g. just open http://jsbin.com/ with "Pause On Caught Exception". You could possibly find a fix to all of those callsites. But it only takes a few popular libraries to not agree, to change the momentum for everyone. The game theory here plays out such that pausing only on uncaught exceptions is the winning strategy.

Therefore, I think that this proposal effectively becomes "pausing on exception is not viable by default in React apps".

The flag to turn on and off would technically work, but 1) it's not very discoverable 2) it's not very scalable for every lib to do this 3) we know from previous versions that people get very confused by subsequent errors that happens due to inconsistencies when we can't properly clean up. This would make the debugging experience worse.

These are things that every developer is affected by.

Meanwhile, top level onerror handling is relatively rare to deal with explicitly. If you deal with it at all on your site, it is usually just one person that deals with its implementation details.

For error boundaries, we typically log the errors anyway in DEV mode just in case someone forgets to in their handling. It turns out that it is way too easy to silence errors and make debugging silent ones a pain. So this part happens anyway. It similar to unhandled Promises.

Maybe we should always raise top level onerror events even in PROD?

@brandonbloom

@brandonbloom Can you elaborate more about your set up and why you wouldn't want to handle any error, including in boundaries, in onerror? E.g. for logging and such I'd expect anything treated as an error to be logged somewhere.

Even if we rethrow, that is still considered "caught".

Huh?

I just tried these individually with the various options in Firefox and Chrome:

try { throw new Error() } catch(e) { }
try { throw new Error() } catch(e) { throw e; }

The behavior matches my expectations: Rethrowing is considered uncaught.

Meanwhile, top level onerror handling is relatively rare to deal with explicitly. If you deal with it at all on your site, it is usually just one person that deals with its implementation details.

I'll freely admit: I'm that person on pretty much every project I ever work on.

Rethrowing is considered uncaught.

Yea, I meant the rethrow callsite is where the break point happens. That would be somewhere in the React internals way later after the source of the error happened. In that case it doesn't provide any more value than a log.

No matter how React handles the error (printing to console, calling error boundary, re-throwing a top level exception, etc), the main DX causality I'm feeling with this system is that I can't have access to the stack and environment conditions that caused an exception to be originally thrown.

I'd really appreciate if my "pause on uncaught exception" debugger flag would pause exactly on the original throw, with the entire context available, but I understand it may be impossible to support this case while also supporting error boundaries, unless there is a flag you can turn on on a __DEV__ build of React to completely disable error boundaries.

@mweststrate

Hmmm... I totally agree with @mweststrate. I wasted about half a day researching why, after adding error boundaries to our app and through live testing seeing the errors get caught/handled, the fallback element wasn't being rendered and instead the error propagated further and the app crashed. This isn't the expected behavior based on the error boundary description. Perhaps it should be mentioned in the docs that by design the fallback only gets rendered in production. I too wanted to test the fallback rendering should an invalid prop be passed to a child component or the child component falls over. We should be able to test error boundary branching and 'control flow', since that's what it's actually doing, from within the dev environment.

[edit 1/11/18]
I must revise my complaint. The app wasn't crashing and the fallback element was getting rendered, it's just that a full screen error pops up, covering my app, where it wasn't immediately or overtly clear it could be dismissed. Now knowing this is the intended behavior I don't have an issue. Docs could be a little more informational regarding this.

@awreese

I'm writing a set of components and part of this are common ErrorBoundary objects that I'd like to render to visually demonstrate what happens how an error is handled. At present, I see the same behaviour as @awreese in that demo exceptions are caught, the fallback is getting rendered but the re-thrown exception covers up the app.

This lead to my initial impression that I wasn't catching the exceptions because the console states uncaught exception: [object]. So after hours of hunting & second guessing, I finally ended up here after noting the componentDidCatch functions are being called and this problem doesn't affect non-dev environments (eg test, prod etc).

In my case, the exceptions are meaningless and they are being caught; seeing a full-screen overlay just leads to confusion. Any manner of avoiding this would be helpful -- a way of marking exceptions as handled; an option for not re-throwing exceptions already handled at a boundary; or some way of turning off the traceback overlay etc.

Quick-and-dirty workaround in public/index.html to just hide the overlay:

    <style>
      body > iframe { display: none; }
    </style>

I also had the problem that I first thought I did something wrong as I always see the last resort error view, which is triggered by a global "onerror" handler. At very least, the docs should mention this.

Another thing is that I cannot really believe global error handles are that rare, but lacking of real data as well. As React "only" catches the errors in renders, a global error handler needs to be used to catch all errors in a last resort manner. I personally did this already before, to catch expected errors in e.g. network errors in sagas.

It seems really unintuitive that React calls it componentDidCatch, when in reality it is not "catching" it is more of componentOnError.

As some context on the scenario this is causing issues for us on. Now that we've added intelligent error boundaries to our app, we want to start using that same boundary to handle an API error. So we are throwing when we hit an API error, and then it bubbles up to the nearest error boundary, and is handled just like any other error. Unfortunately, in dev, this causes redbox to appear, and since our dev environment is complex, a couple api calls pretty regularly fail. So this redbox appearing a couple seconds into every page load is extremely frustrating.

@Alphy11

@Alphy11 componentDidCatch is actually a rather intuitive name as it did catch an error produced by one of its descendants. Is your API actually producing errors, or have you defined your application to be in some error state given certain API call results? If the error boundary 'red box' is happening that much in dev, a workaround could be to set a dev env var and basically suspend the error throwing on specific api calls that routinely falsely fail in dev.

@brandonbloom

@brandonbloom :

This is misguided because it's not about pausing on exceptions, it's about "pause on uncaught exceptions."

I keep running into the same thing. I'd like to have Dev Tools stop right when an exception happens in my code. But it'll still fly over that because I don't enable Pause on caught expressions. Because, as you say,

there are plenty of libraries out there that internally use exceptions. I've never found "pause on caught exception" to ever be useful, since it produces an incredible number of false positives

Yes, for example Babel routinely throws and catches exceptions. Exceptions are no longer exceptions, they're the norm. This feels wrong, but I'm unfamiliar with the constraints Babel development faced. It's one of the reasons why I avoid Babel on new projects and prefer Bublé or just no transpilation during development (ES2015 is rather well covered by browsers of 2018). In general I'd prefer if transpilation were removed from development time (browser reload latency, sourcemap needs, exception pollution etc.), relegated to become part of a release build and automated tests; not sure why we as a community cling to transpiling large codebases on every single line change. But it's how it's done in most shops.

In short, i'm trying to use a facility available for decades: be in the stack where my exception happens, without other libraries constraining this freedom.

@sebmarkbage :

it only takes a few popular libraries to not agree, to change the momentum for everyone. The game theory here plays out such that pausing only on uncaught exceptions is the winning strategy. Therefore, I think that this proposal effectively becomes "pausing on exception is not viable by default in React apps".

Side question: why are you adding "by default"? Is there a flag that makes it happen?

Maybe browser vendors could help restore sanity when it comes to uncaught exception? Browser blackboxing could be generalized; this, or something analogous to this could cover the suppression of uncaught exceptions (yes, sounds weird). I think that 'uncaught-blackboxing' a few libraries such as jQuery and Babel would help restore this otherwise useful function. Sure it's a hassle but on any specific project, there may not be more than a handful of offending libraries, and it's a one-time blackboxing step. Or maybe all could be blackboxed and one's own app code whitelisted. Btw. i'd like to have blackboxing for performance measurement too 😄

@Kovensky : completely agreeing with your comment and think such a flag would be feasible

@sophiebits

@sophiebits wrote:

In React we don't recommend you use exceptions except for programmer errors, so I prefer (as a React user) that they always show up in "pause on exception" despite there being an error boundary. Error boundaries are a failsafe check that you don't expect to hit in normal operation.

Does suspense change your perspective here? I'm wondering whether error boundaries should be the appropriate mechanism with which to handle data fetch failures, simply mapping as promise rejection, or whether to wrap as Promise<Result<T>> to handle separately to programmer errors.

I'd just like to point out that that behavior should be clearly explained in the error boudaries / componentDidCatch documentation (it is not at the moment).

I had missing error logs in production builds because I was using the window error event to capture errors globally. It was working in dev, but not in production with error boundaries (which seems like the right thing to do, but confusing when the different behavior is not documented).

I think the core point of catch rethrowing errors in tests or with componentDidCatch is to limit or reduce the noises introduced by known error situations, which should be handled instead of interrupting the whole javascript thread.

Sadly this doesn't play nice with other tools like Cypress which fully stops on uncaught errors which confused us a lot initially when tests run fine in production, but not locally.

Any updates on this? The docs aren't still very clear that this is the behavior in development. I spent a lot of time head scratching and wondering why my error boundaries didn't catch the error and I still got our general error handler. It wasn't until i googled my way here that I actually understood what the trouble was.

It would be super useful for this to be configurable or at least the errors tagged in some way so we can discard them in the general error handler as handled.

Personally I think it's a bad idea to behave differently in development and production. It's confusing for the developer, not to mention any non developers testing on development builds, like testers and UX.

Chiming in here. Because of this issue we (I mean me) deployed some code to production that broke logging errors caught by an error boundary to Sentry.

Personally, I would prefer code behaving the same way in dev and prod over being able to pause of caught exceptions in the browser's dev tools.

If anyone is curious how I worked around this for Sentry I did so by proxying the captureException method on the hub.

/**
 * Filters events that will be captured a second time by a React error boundary
 * in development. Events in React behave differently in development than they
 * do in production. In development, errors that will be caught by an error
 * boundary will still propagate to the Window object.
 *
 * Because of this Sentry will have already "seen" that error and will ignore it
 * when it is captured a second time by our error boundary, preventing any
 * context we attach to the error in our error boundary from being sent to
 * Sentry!
 *
 * Note: We only need to filter these events in development. Vite will eliminate
 * this block entirely for optimized builds.
 *
 * @see https://github.com/facebook/react/issues/10474
 * @see https://github.com/getsentry/sentry-javascript/blob/develop/packages/utils/src/misc.ts#L190
 */
if (!import.meta.env.PROD) {
  const hub = getCurrentHub();
  const { captureException } = hub;

  hub.captureException = (...args) => {
    const stackTrace = new Error().stack;
    const inReactGuardedCallbackDev = stackTrace?.includes(
      "invokeGuardedCallbackDev"
    );
    const inSentryWrappedEventTarget = stackTrace?.includes("sentryWrapped");

    return inReactGuardedCallbackDev && inSentryWrappedEventTarget
      ? ""
      : captureException.call(hub, ...args);
  };
}

Workaround:

window.addEventListener("error", function (event) {
  let {error} = event;
  // XXX Ignore errors that will be processed by componentDidCatch.
  // SEE: https://github.com/facebook/react/issues/10474
  if (error.stack && error.stack.indexOf('invokeGuardedCallbackDev') >= 0) {
    return true;
  }
  // Original event handler here...
});

Its better to check the stack of a new error rather than the error that the event handler caught:

window.addEventListener("error", function (event) {
  let {error} = event;
  // XXX Ignore errors that will be processed by componentDidCatch.
  // SEE: https://github.com/facebook/react/issues/10474
  if (
      ((error) =>
      error.stack && error.stack.indexOf("invokeGuardedCallbackDev") >= 0)(
      new Error()
      )
  ) {
      return true;
  }
  // Original event handler here...
});

I had to do it this way because I'm using TanStack Query and I'm throwing on HTTP errors so I can use error boundaries to handle those in a general way. But that means that the error that is caught by the error boundary is actually a promise rejection, which may not have invokeGuardedCallbackDev in its stack.

I think this all changed in React 19. It looks like errors are no longer re-thrown in development?

Can this issue be closed now?

Also, the docs need updating: reactjs/react.dev#2168 (comment)

Navigation Menu

Repository navigation

The fake event trick for rethrowing errors in DEV fires unexpected global error handlers and makes testing harder #10474

22 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

The fake event trick for rethrowing errors in DEV fires unexpected global error handlers and makes testing harder #10474

Description

Activity

brandonbloom commented on Aug 16, 2017

sophiebits commented on Aug 16, 2017

sophiebits commented on Aug 16, 2017

brandonbloom commented on Aug 16, 2017

acdlite commented on Aug 16, 2017

acdlite commented on Aug 16, 2017

brandonbloom commented on Aug 16, 2017

acdlite commented on Aug 16, 2017

brandonbloom commented on Aug 16, 2017

acdlite commented on Aug 16, 2017

brandonbloom commented on Aug 16, 2017

sebmarkbage commented on Aug 16, 2017

sebmarkbage commented on Aug 16, 2017

brandonbloom commented on Aug 16, 2017

sebmarkbage commented on Aug 16, 2017

22 remaining items

Jessidhia commented on Dec 22, 2017

awreese commented on Jan 10, 2018

davidjb commented on Feb 7, 2018

hoschi commented on Feb 21, 2018

Alphy11 commented on May 11, 2018

awreese commented on May 11, 2018

monfera commented on Jun 3, 2018

lrowe commented on Sep 5, 2018

guillaume86 commented on Sep 14, 2018

noscripter commented on Nov 14, 2018

donaldpipowitch commented on Jan 29, 2019

davidlgj commented on Sep 13, 2022

daniel-nagy commented on Feb 10, 2023

zeorin commented on Jul 24, 2023

OliverJAsh commented on Feb 19, 2025

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions