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

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

Closed
selbekk opened this issue Dec 19, 2017 · 20 comments
Closed

Comments

@selbekk
Copy link
Contributor

selbekk commented Dec 19, 2017

Is this a bug report?

Yes

Can you also reproduce the problem with npm 4.x?

Yes

Which terms did you search for in User Guide?

componentDidCatch, error boundaries

Environment

  1. node -v: 8.9.2

  2. npm -v: 5.5.1

  3. yarn --version (if you use Yarn): 0.17.10

  4. npm ls react-scripts (if you haven’t ejected): 1.0.0

  5. Operating system: Linux

  6. Browser and version (if relevant): Chrome 63

Steps to Reproduce

  1. bootstrap app with create-react-app
  2. create an error boundary
  3. trigger an error in a child of that error boundary

Expected Behavior

I would assume React dealt with the error, and no overlay was necessary.

Actual Behavior

The error overlay is placed on top of the app, like an uncaught error occurred. If I manually remove the
error overlay from the DOM (via devtools), the correct UI is rendered below.

Reproducible Demo

https://codesandbox.io/s/jvrwr388wv

@gaearon
Copy link
Contributor

gaearon commented Dec 19, 2017

This is intentional. An unexpected error is still an error. (We don’t recommend using error boundaries for expected errors or control flow.)

Error boundaries are primarily useful for production, but in development we want to make errors as highly visible as possible.

@gaearon gaearon closed this as completed Dec 19, 2017
@selbekk
Copy link
Contributor Author

selbekk commented Dec 19, 2017

Ah alright, thanks :) It had me wondering what was happening, since the error boundary UI was shown at first, before it was hidden again.

Could it be an idea to somehow indicate that the error was dealt with in an error boundary in the UI of the error overlay?

@gaearon
Copy link
Contributor

gaearon commented Dec 19, 2017

before it was hidden again.

I wouldn’t expect your custom boundary to get hidden though. (Error overlay doesn’t do anything to dismiss it.)

Could it be an idea to somehow indicate that the error was dealt with in an error boundary in the UI of the error overlay?

I think this would be reasonable. Not sure how to do it though.

@selbekk
Copy link
Contributor Author

selbekk commented Dec 19, 2017

I wouldn’t expect your custom boundary to get hidden though. (Error overlay doesn’t do anything to dismiss it.)

It's not dismissed, it's simply hidden behind the overlay (you can reproduce it in the codesandbox-demo above). If you remove the overlay from the DOM manually, you can see that my ErrorBoundary component renders as usual. Perhaps some bad choice of words on my part :)

I think this would be reasonable. Not sure how to do it though.

As far as I can tell, React tells us that it will try to recreate the component tree in the error message:

React will try to recreate this component tree from scratch using the 
error boundary you provided, ErrorBoundary.

You can see it in action if you pop open the console in my codesandbox example.

Perhaps we can check whether the error includes parts of this text - and if so, show some indication in the error-overlay UI?

@gaearon
Copy link
Contributor

gaearon commented Dec 19, 2017

Fair enough. I'll rename this issue to keep track of this.

@gaearon gaearon reopened this Dec 19, 2017
@gaearon gaearon changed the title Error overlay shown when error is caught with componentDidCatch Make it clearer in error overlay when error boundary has caught an error Dec 19, 2017
@SylarRuby
Copy link

🙈 Never even see the "x" to close the overlay and to know this wont show in production is music to my ears. Thanks

@philraj
Copy link

philraj commented Jan 22, 2018

I didn't notice the X to close the error overlay at first either. It might help to add a label like X Close. Though this is somewhat off-topic and might be better mentioned in a different issue.

@Timer
Copy link
Contributor

Timer commented Jan 23, 2018

There's a message at the bottom that says only visible in development (right?).

@gaearon
Copy link
Contributor

gaearon commented Jan 23, 2018

We might actually want to amend that wording to link to error boundary documentation. Since that's the thing that will show up in production instead.

@Timer
Copy link
Contributor

Timer commented Jan 23, 2018

We could also amend the message to say "press [escape] to close or X in upper right" or similar.

@iMerica
Copy link

iMerica commented Jan 31, 2018

How do we actually test our error boundaries locally when using Create React App? Can we disable the full page debug iframe?

@selbekk
Copy link
Contributor Author

selbekk commented Jan 31, 2018

You can close the overlay with Escape or by pressing the x in the corner @iMerica 😊

@iMerica
Copy link

iMerica commented Jan 31, 2018

Thanks @selbekk 👍

@Jakhotiya
Copy link

This issue is a Gotcha for people who are new to ErrorBoundaries and are just trying out the concept with create-react-app or on sandbox.io. You end up debugging for an hour before you realize that react-error-overlay is just hiding the boundary and boundary actually worked.
@gaearon I would like to work on this issue. How can I help? How would you like to fix it?

@sag1v
Copy link
Contributor

sag1v commented Mar 19, 2018

On some cases you can't even hide the overlay (not using create-react-app). Well you can via the dev tools and manually delete the node but then you won't see the alternative component that the boundary should render.

@javierfernandes
Copy link

@Jakhotiya not 1, but like 4hrs in my case.

I think that this is inconsistent. Is like a VM generating error messages for exceptions being try/catched just in case you swallow them.
If I wrote an error boundary then I'm a grown up and I should be telling react/CRA: "let ME handle the errors".
If you wrote an error boundary and you are swallowing errors, then that's your problem man :). No language or framework will protect you from bad error handling :)

Disabling the overlay is not an option because I do want it for others "real uncaught" errors.
As it is right now, it will probably end up confusing like those bad linter error that one end up saying "oh.. that's ok, that's not an error", and eventually one ignores a real error.

I suggest at least it should detect and inform that the error was caught by an error boundary. A really big feedback note there :)
And/or to mention it in the docs (?)

Thanks !

@raix
Copy link
Contributor

raix commented Mar 29, 2018

@gaearon I didn't know that we were limited to only throw errors?

imho I don't mind the extra error in console when in development - but the screen overlay for "already handled" errors etc. is annoying.

@rivertam
Copy link

rivertam commented Apr 5, 2018

I definitely understand why it's not best practice to use error boundaries for control flow. My usecase is that I'm using a complex third party component for a debugging tool on the back side of my application. This third party component keeps throwing errors, which I believe is a bug of the third party component. Honestly, I'm not even sure. I'm basically just throwing together a quick prototype, and I want to fall back to a reasonable behavior when the component fails. By encouraging me to go with best practice here, you're basically telling me I need to either abandon this convenient component, fix the convenient component, or come up with an even hackier workaround.

@rojasmi1
Copy link

rojasmi1 commented Aug 8, 2018

Had exactly the same problem.

I totally agree on showing the error in development, but the small/grey letters at the bottom are far from visible and the documentation doesn't clearly states that the overlay will still be displayed in development mode and can be dismissed. It'd be nice to have that documented and make the message at the bottom pop out a bit more.

@stale
Copy link

stale bot commented Nov 2, 2018

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in 7 days if no further activity occurs.

@stale stale bot added the stale label Nov 2, 2018
@Timer Timer closed this as completed Nov 2, 2018
@lock lock bot locked and limited conversation to collaborators Jan 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests