Make it clearer in error overlay when error boundary has caught an error

This issue was discussed in #3627, but it's been marked and stale as locked so I'm opening another issue.

As discussed in #3627, create-react-app will always display an error overlay for errors in development mode, even when the error is caught via an error boundary. This confused me at first, and it's also confusing users of the routing library I maintain. This library throws a NotFoundError when it can't match a URL segment, and uses a <NotFoundBoundary> component to allow the user to render a 404 page.

<div className='App'>
  <Sidebar />
  <main>
    <NotFoundBoundary render={() => <NotFoundPage />}>
      {/**
      <View /> throws a NotFoundError if they can't find a route any content for
      the part of the URL they correspond to. They can be nested, so even if
      this view doesn't throw, a child view might -- thus boundaries are 👌
      **/}
      <View />
    </NotFoundBoundary>
  </main>
</div>

I feel like the best approach would be to not show error overlays for caught errors (Dan mentioned it's on his todos). But just making it clearer that the error was caught would be a big improvement.

I'd like to bring this up again. Especially as ErrorBoundaries are increasing in use, having the error-overlay pop up when the error is handled is pretty disruptive. Would love for caught errors to not trigger the overlay.

I've been diving in a bit and it looks like "all" that would be needed is to somehow determine whether an error was handled by an ErrorBoundary (not sure whether React exposes that information) and update these to not handle errors that were passed to an ErrorBoundary: https://github.com/facebook/create-react-app/tree/master/packages/react-error-overlay/src/effects

What if the error overlay didn't take up the full screen and appeared as a intuitively dismissible modal instead?

This is the best balance IMO:

You can see that your error boundary triggered in the background and can "click-off" to dismiss the overlay
You still get the rich context provided by the overlay and the quick "launch in editor" shortcut

I think that would also be nice, but its presence is really annoying when working on error boundaries or if you're working on one part of the app but another part of the app has a failing network request but doesn't really matter because you're not working on that right now, etc.

An error boundary is a way to say: "this is handled" Having something jump out at you when you've handled something is annoying and unhelpful IMO.

I'm not convinced it'd be a DX win to see no overlay whatsoever for an unhandled error in your application. I guess this could become more annoying if you're leveraging error boundaries for "known" view states instead of unexpected errors.

I definitely do feel pain for network errors—in this case, the error overlay could remain dismissed if more errors continue to flow in:

I'm not convinced it'd be a DX win to see no overlay whatsoever for an unhandled error in your application.

I agree with you. I'm not talking about "unhandled errors" we're talking about handled errors. Errors handled by an error boundary. That's handling the error.

Personally, I consider an error boundary being triggered an unhandled error: your component tree crashed (notwithstanding experimental features). I can see how opinions would differ here, though.

How would you handle the case of an application that has a top-level (generic) error boundary that's used as a last resort to ask the user to refresh the application? You'd want to see the overlay in this case—it seems we'd want to make the distinction between intentional error boundaries and accidental ones.

edit: I'm going to try to reach out to Dan to learn about how they treat this case internally at FB.

intentional error boundaries and accidental ones.

I'd say including an error boundary at all is intentional. I think the developer will see that their error boundary was triggered and can investigate. I suppose it may be unfortunate that they would not get the overlay to help them investigate.

What if we do this... Add a class or ID to the iframe so it can be targeted with CSS to add display: none;? That's as good as any opt-out I can think of. Thoughts?

set react-error-overlay iframe ID

#9342 doesn't close this original issue (which I think should be addressed still), but it does make it easier for folks to hide the overlay if they don't want it to show up for their app.

Let me share some background on why it works this way.

Yes, error boundaries are a way to catch errors. However, error boundaries are a part of your UI. This means that, just like any part of UI, they might not be visible. Maybe they're inside a display: none tree. Maybe they're behind something else due to the z-index. Maybe the part that errored is below or above the viewport. Maybe it is in a portal (e.g. a tooltip). Maybe an error boundary wraps a component that currently has very little height and width, and is very easy to miss. As a result of all these plausible scenarios, you risk not noticing the error at all.

Next, I will claim that most errors are, in fact, going to be programming errors, and not intentional ones. People make mistakes all the time, and JavaScript is a dynamic language. There will be typos, null references, crashes, and so on. During the course of development you'll have a lot more of them than intentional throws. I think we should optimize the experience for catching bad errors early, even if it hurts some other cases.

Additionally, the distinction between "caught" and "uncaught" errors in this case doesn't have any substance. It would appear that even wrapping your whole app in one boundary would suddenly mark all errors as "caught". Since every app should have a boundary, and we'll likely add one by default into CRA and Next templates to encourage people to design them, there would be no "uncaught" errors in principle. So it is not a very meaningful distinction for rendering errors.

Due to these three points, I recommend against a blanket solution like "don't show dialog for caught errors". In practice, most of them should be "caught" so this is more or less equivalent to removing the error dialog altogether and hoping for the best.

I think it's fair to say that like in the original post, some errors are intentional. We don't have a very good mechanism for distinguishing them in React. We might eventually introduce one. But these are already shown in console, so it's also not like the dialog acts in a special way. It attempts to mirror what the console is doing.

I get that the dialog is obtrusive in these cases. As a compromise, I propose the following solution:

For ReferenceError or TypeError, always show the dialog. No exemptions.
If possible, filter out browser-generated network errors and never show them in a dialog.
For other types of error, check if the error was caught by a boundary. If it was caught by a boundary, do not show the dialog but show the error marker in the corner (like in Make it clearer in error overlay when error boundary has caught an error #6530 (comment)). Clicking the error marker shows the dialog. The error marker should have a small clear button so you can dismiss it without making the dialog appear. In case the error is expected.

This way, you're not going to miss important errors, but the dialog also doesn't distract you from seeing your own custom boundaries.

This would work fine for my use case as well. I think it's a solid compromise 👍

Replace error boundary with manual error handling

Why is the CRA equivalent of a blue screen of death a good idea when people are doing good things and placing error boundaries so that their users don't crash their entire application? That's signalling to the developer that they didn't just have an error, but they screwed up their entire error handling.

On the other hand, if the developer properly implements an error boundary that catches an error - great, now that error boundary can show a cool UI with stack trace and help them debug and such. Why do we have to forcefully crash the entire application when that debugging information has an explicit place in the application that the developer specified? Why not just make it easy to have an error boundary that - in development - gives the same cool stack trace information that this overlay has?

Why make compromises and prompt people to want to completely disable this incredibly valuable and useful feature? This is the same thinking that resulted in Windows ME. Of course, Microsoft knows best so they need to bombard you with messages so much you ignore them all and then want to throw your computer out the window.

I do like "For ReferenceError or TypeError, always show the dialog. No exemptions." though. 100% those are always real errors. But seriously, Networking Error does not mean my code is buggy.

At the very least, giving the overlay iframe a class name so we can target it with css would need enough for opt-out-ability...

@ntucker

Agree with @ntucker here... network 404's are perfectly reasonable responses depending on the UI. Any domain model that's built from the top down will get 404's when querying down the domain model...

Very frustrating to hit X every time... and very frustrating to explain to a non-tech person why were seeing giant "this is broken" errors when they pop-in for a quick look @ something.

Tip: If you delete the error stack trace, the dev error overlay will not show (at least in next.js)

👉 delete error.stack

We are doing that in Blitz for any errors coming from the server

On second thought, since CORS and network down errors are TypeErrors, this would also be bad to always die on.

Tip: If you delete the error stack trace, the dev error overlay will not show (at least in next.js)

👉 delete error.stack

We are doing that in Blitz for any errors coming from the server

Hmm, this didn't work for me. It seems the stack is added once you throw the error, even if deleted after creation. Oh wait, I was using CRA not next.js...probably my problem

@flybayer

If I set to undefined error.stack = undefined I get Uncaught Error: The error you provided does not contain a stack trace. in the console but no overlay shows up. @flybayer are you getting the same thing? Maybe this is just a hack to make the overlay break 😂

@ntucker

@ntucker no, not getting that. In next.js delete error.stack works perfectly.

hey I'm late for the party, but how about an option to swallow the error in error boundary?

componentDidCatch(error, errorInfo) {
  report(error, errorInfo);
  errorInfo.hasBeenCaught(); // or .catch(), .stopPropogation(), .markAsHandled(), etc

  // or
  return true; // doesn't feel explicit enough
 }

(I'd go with .catch() though it might be confusing with promises)

In my app we are allowing user code in some of the places, so showing the error overlay in spite of the boundary is useful to us. I'm also having the opposite case, where react errors are not shown in Error Overlay, but I am using a custom error overlay, it's probably related to that.

Additionally, the distinction between "caught" and "uncaught" errors in this case doesn't have any substance. It would appear that even wrapping your whole app in one boundary would suddenly mark all errors as "caught". Since every app should have a boundary, and we'll likely add one by default into CRA and Next templates to encourage people to design them, there would be no "uncaught" errors in principle. So it is not a very meaningful distinction for rendering errors.

I'm not sure I follow this reasoning. Your definition of "uncaught" is based on whether there is or is not an error boundary anywhere in the tree. When using a framework, the framework becomes the runtime, and the definition of "uncaught" should only be based on the existence of an error boundary in user code. It's perfectly fine for the framework all encompassing error boundary to show an overlay. If I know what I'm doing, I think I should be able to opt-out by adding my own.

I don't understand how this behavior will play nice with suspense for data fetching. Aren't error boundaries an inherent building block under that paradigm?

I would also like for there to be some option to not show an overlay in specific cases of an error boundary. It should definitely be explicit; so something should be called in order to mark the error as "handled" such that it shouldn't show an overlay.

A good analogy would be like exceptions: if an exception is caught, you wouldn't implicitly show a big error saying that an exception was thrown.

A use case of this right now is that I have a JSON editor where you can basically write the props a component would receive directly in the form of a JSON string. The resulting component will fail more often than not due to invalid props being passed, and we can't know ahead of time if the component is going to fail due to bad props. The error overlay popping up in this case is annoying to say the least, since for every character typed into the JSON editor, the error overlay will keep popping up over and over again and you have to dismiss it over and over again for each character you type.
This is why I would like to be able to "expect" that exceptions get thrown in the component, and that I can handle the exception 100% in my own way e.g. showing a failure message inline, and - during development - show an error stack trace too, but inline, not as an overlay.

Just to throw out another reason why IMO the "just accept the overlay" solution doesn't work great for me:

When we catch an error using an ErrorBoundary in our app, we open a Dialog using the Radix Dialog component. React then opens its Error Overlay over our Dialog, which... fine, I get it.

The problem is, when I click to close the React Error Overlay, Radix interprets that as me "clicking outside" our own Error Dialog and closes that too. Which makes it impossible to test.

So I'm left with the option of disabling click-outside-to-close in my Dialog but it's a bit disappointing that I have to make my app's UX worse in order to get around this issue.

Why is the CRA equivalent of a blue screen of death a good idea when people are doing good things and placing error boundaries so that their users don't crash their entire application? That's signalling to the developer that they didn't just have an error, but they screwed up their entire error handling.

On the other hand, if the developer properly implements an error boundary that catches an error - great, now that error boundary can show a cool UI with stack trace and help them debug and such. Why do we have to forcefully crash the entire application when that debugging information has an explicit place in the application that the developer specified? Why not just make it easy to have an error boundary that - in development - gives the same cool stack trace information that this overlay has?

Why make compromises and prompt people to want to completely disable this incredibly valuable and useful feature? This is the same thinking that resulted in Windows ME. Of course, Microsoft knows best so they need to bombard you with messages so much you ignore them all and then want to throw your computer out the window.

I do like "For ReferenceError or TypeError, always show the dialog. No exemptions." though. 100% those are always real errors. But seriously, Networking Error does not mean my code is buggy.

Yeah you are absolutely right. The problem with developers who start building an awesome new framework (like Nextjs) used by millions of developers is that if meanwhile they also develop some god complex and as a result they don't care about anymore the needs of the whole developer community. This case is a good example. Nextjs devs playing here the dad who takes off the error handling completely of the child's (dev community) hand cause the child is not grown up enough to have this responsibility.

Navigation Menu

Repository navigation

Make it clearer in error overlay when error boundary has caught an error #6530

1 remaining item

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Make it clearer in error overlay when error boundary has caught an error #6530

Description

Activity

kentcdodds commented on Jul 20, 2020

kentcdodds commented on Jul 20, 2020

Timer commented on Jul 20, 2020

kentcdodds commented on Jul 20, 2020

Timer commented on Jul 20, 2020

kentcdodds commented on Jul 20, 2020

Timer commented on Jul 20, 2020

kentcdodds commented on Jul 21, 2020

kentcdodds commented on Jul 21, 2020

gaearon commented on Jul 23, 2020

1 remaining item

jamesknelson commented on Jul 24, 2020

ntucker commented on Jan 27, 2021

kentcdodds commented on Jan 27, 2021

chadbr commented on Jan 27, 2021

flybayer commented on Jan 27, 2021

ntucker commented on Jan 27, 2021

ntucker commented on Jan 27, 2021

ntucker commented on Jan 27, 2021

flybayer commented on Jan 28, 2021

KutnerUri commented on Feb 18, 2021

Janpot commented on May 17, 2022

amcsi commented on Jun 27, 2022

erikpukinskis commented on Aug 12, 2023

martonhorvath1990 commented on May 8, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions