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

Can an error boundary prevent React's error logging? #15069

Open
silverwind opened this issue Mar 8, 2019 · 37 comments
Open

Can an error boundary prevent React's error logging? #15069

silverwind opened this issue Mar 8, 2019 · 37 comments

Comments

@silverwind
Copy link

I noticed this unconditional console.error which I'd like to prevent to keep the console clean from errors that are already "caught" in an error boundary.

Maybe a condition on capturedError.errorBoundaryFound could prevent this logging?

@silverwind silverwind changed the title Should an error boundary prevent console.error? Can an error boundary prevent React's error logging? Mar 8, 2019
@gaearon
Copy link
Collaborator

gaearon commented Mar 12, 2019

For now we'd rather not allow it because it makes it too easy to swallow errors (such as if the error boundary itself is buggy).

@silverwind
Copy link
Author

Right, I see reasoning in that, preventing error logging should require more than just the existance of a boundary.

Browsers allow to suppress logging uncaught errors to the console via event.preventDefault() in a error event handler on window. Maybe a something similar could be done from a error boundary too, like error.preventDefault().

@anilanar
Copy link

Error swallowing should be opt-in. I'd like to swallow certain errors and not others:

componentDidCatch(error) {
  // I'd like to swallow the error
  if (error instanceof MyExpectedError) {
    // somehow suppress the error?
  }
}

@jamstooks
Copy link

jamstooks commented Apr 9, 2019

I'd support this feature as it would be nice to de-clutter our test output.

@EyalPerry
Copy link

EyalPerry commented May 6, 2019

@gaearon

consider the following production issue we are facing:
We are using a third party logging service, that logs all console.error calls automagically- which is debatably awesome and at the same time a given fact.

Naturally, we have our own ErrorBoundary implementation:
We want to slap on extra information special to the boundary instance-
So our error boundary component receives this information as props / context,
And logs the error along this information using the third party's sdk when an error occurs.

As a result of this implementation, when a component fails, we have two separate errors logged in our monitoring pipeline:
One which react logs, with barely any info, which comes from console.error
And one with everything we need to know, which we explicitly logged via the third party library.

Since this redundancy is confusing and not acceptable
We are now facing quite a dilemma:

If we don't explicitly log, we lose error specific info.
We also rely on react to do the logging for us, which is a bad idea in the long run.

if we explicitly log - we get two separate events in our monitoring pipeline, which is confusing and noisy, since our monitoring pipeline is automated and triggers a whole lot of internal processes.

I bet that we both believe console.error to be a side effect.
As an app developer, wouldn't you agree with me that such a side effect should not be invoked by a UI library without the possibility to opt out?

It's great that I can isolate failures to granular regions in the application- anything other than that, should not be decided for me as a developer.
I wouldn't want to see anything in the console which I did not put there.
Especially if it renders the entire stack of my component tree structure, uglified or not.

On the other hand, I do understand where you're coming from by saying you don't want it to be easy to swallow errors, so how about this as middle ground:
You could move the invocation of console.error to componentDidCatch,
Which would be the default implementation unless overriden.

if you think of it, this is the correct thing to do, since componentDidCatch is meant for exactly this.
To quote the docs:

componentDidCatch() is called during the “commit” phase, so side-effects are permitted. It should be used for things like logging errors

you say console.error and I say thirdPartyLibrary.logError.

The means of logging should be up to app developers, especially if they went through the trouble of setting up Error Boundaries.

Since Error Boundaries are basically logging components with some conditional rendering, and since I bet our logging provider is not the only one which logs console.error calls, wouldn't you say this issue should be solved in a more extensible manner?

As for buggy error boundaries, IMHO, you should not care about them.
It's just another failed component, so let it fail until it either crashes the entire tree or another boundary catches it. Since react is solely a UI rendering library, why should it decide what happens when rendering fails?

@anilanar
Copy link

anilanar commented May 8, 2019

@EyalPerry hit the nail on the head. Is there anything more to say? Shall we patch console.error?

@tonix-tuft
Copy link

tonix-tuft commented Oct 30, 2019

@anilanar I completely agree with you, I think we should really have the possibility to suppress the logging of some errors which have been handled in error boundaries.

I think that we should have the power and responsibility to decide what to log and what not to log, especially in production.

In my case, if my app has been deployed in production and an error boundary catches an error, I wouldn't want to log the error to the console leading to information disclosure for some "more smart" users who open the browser's console to see what went wrong, or at least, I would want to be able to decide whether to log a certain error captured by an error boundary or not.

Also, I think that, when using create-react-app, it would be great to have the ability to suppress the error overlay for some errors caught by the error boundary (not all, but some), but that's another story...

@ElForastero
Copy link
Contributor

ElForastero commented Dec 9, 2019

It would be nice to have a control on logging errors to console.

For example, I like to use Error Boundaries for handling 404 exceptions in my apps. This way I don't need to call a redirect to some route or put the rendering logic inside my components.

Regularly I just throw a NotFoundException and seeing an error in console is not something I expect.

I would agree with @EyalPerry that moving the logging into default componentDidCatch would be a nice solution.

@stale
Copy link

stale bot commented Mar 8, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution.

@stale stale bot added the Resolution: Stale Automatically closed due to inactivity label Mar 8, 2020
@anilanar
Copy link

anilanar commented Mar 8, 2020

Don’t close. We can’t seriously unconditionally log this anymore.

@stale stale bot removed the Resolution: Stale Automatically closed due to inactivity label Mar 8, 2020
@art-in
Copy link

art-in commented Mar 30, 2020

I have the opposite problem. Error is not logged to console in dev build.

The above error occurred in the..., but nothing is above.

So if I console.error it in componentDidCatch it works in dev, but now I have two duplicate errors logged in prod.

v16.13.1

@zordone
Copy link

zordone commented Jun 25, 2020

I'd like this to be fixed as well. The solution @EyalPerry suggested would be perfect.

@mariuscosta
Copy link

I vote for this too. I am also using an error handling mechanism like the one described by @ElForastero.

@kumar303
Copy link

kumar303 commented Aug 2, 2020

Does anyone have a workaround for this while testing? I want to real-render a component under test, catch the error, and make an assertion without seeing error logging, especially as one can't hide logging in jest.

@MatiasManevi
Copy link

MatiasManevi commented Sep 1, 2020

Does anyone have a workaround for this while testing? I want to real-render a component under test, catch the error, and make an assertion without seeing error logging, especially as one can't hide logging in jest.

This is my hacky workaround:

const swallowErrors = yourTestFn => {
     const error = console.error
     console.error = () => {}
     yourTestFn()
     console.error = error
}

// Then your test
it('tests something without console log' , () => {
      swallowErrors(()=>{
            const wrap = mount(<ErBoundary><Child/></ErBoundary>)
            // ... expect errors ...
      })
})

Or, you can do as this guy here

@stale
Copy link

stale bot commented Dec 25, 2020

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

@stale stale bot added the Resolution: Stale Automatically closed due to inactivity label Dec 25, 2020
@anilanar

This comment has been minimized.

@stale stale bot removed the Resolution: Stale Automatically closed due to inactivity label Dec 29, 2020
@ZackWard

This comment has been minimized.

@Arithmetics

This comment has been minimized.

@advl

This comment has been minimized.

@nathggns
Copy link

nathggns commented May 5, 2021

This is resulting in double errors in our logging system (ErrorBoundary logs it but we also capture console.error) – extremely frustrating.

@raveclassic
Copy link

The errors are also unconditionally logged during tests where I expect components to throw and catch the errors myself with error boundaries. This is frustrating and pollutes test output.

@anilanar
Copy link

I suppose this is still not fixed in React 18 alpha. We still have hopes for stable 18 release.

@BYK
Copy link

BYK commented Nov 5, 2021

@gaearon is there any way to prioritize this or would you accept PRs to fix it?

@ask-imran
Copy link

We are running into a similar problem as @EyalPerry mentioned. It would be great to fix this

@leotg130
Copy link

@gaearon Would also like to see this fixed. I think ErrorBoundary should have same semantics as exception handling, once an exception has been caught, it's caught.

If you want to log your exceptions, you should log them in the catch(), having a helper that logs the error exactly like it's currently logged might be useful. I guess this would be a breaking change, but IMO it's worth to have the same semantics as normal exception handling. If breaking change is too big of a problem, then having a .preventDefault()/.preventLogging() works too.

If the exception handler (ErrorBoundary) itself causes an exception, that's a different case, and should cause an exception to bubble up (which presumably could be caught by a higher level handler/ErrorBoundary).

For me there's two main reasons:

  1. Nicer DX/UX, we know that in some cases some components will fail, and thus use fallback components, spamming the console isn't very nice. The console messages would lead people to think that something is broken, even though it has already been handled gracefully, by the fallback component.
  2. E2e tests, we use playwright, and it would be nice to have all tests confirm at the end of the test that there were no log entries. It's too easy for developers to not notice something in the log, and having e2e tests that double check is useful.

@timredband
Copy link

My team ran into this issue as well. Would really like a fix.

@olee
Copy link

olee commented Feb 22, 2022

Yeah, being able to control this logging behavior would be awesome.
We are using an extensive logging system and it would be nice to be able to combine the error and the component stack into a single log message this way.
While just hiding the second component stack error message is not difficult, detecting the previous "uncaught" window error is the major issue as there's no ways to identify that it originated from within react.

@pleunv
Copy link
Contributor

pleunv commented Jul 13, 2022

I'm exploring a pattern in which I throw specific errors when running into unexpected scenarios (i.e. missing or invalid parameters, invalid routes, ...) with the intention of moving all of this UI state logic to error boundaries instead rather than repeating it in every page/view. It makes things a lot cleaner, but the unfortunate side-effect is indeed that all of these errors now show up as uncaught errors in the console. Would really love a way to be able to bail out of logging expected errors.

I do understand that intentionally throwing errors does not mean that they are also caught, if you for example forget to handle them in an error boundary, or simply forget to wrap your tree with an error boundary component. Not really sure what the best way around this would be.

@kevinhaube
Copy link

Error stacks are ludicrous when testing error boundaries 😬 yikes.

@epferrari
Copy link

bump

@samettttt
Copy link

bump

@AbhiPrasad
Copy link

The duplicate logging (once from console, once from error boundary), is troublesome for user's who've configured Sentry error monitoring that way.

Considering there exists API to do this for hydration errors with onRecoverableError #23207, is there more appetite for exposing this more publically?

I wouldn't even mind if it's a method I can monkeypatch with a Proxy, if labelled appropriately consumers can take the necessary risks. I did something similar with Next.js (vercel/next.js#36641) to log out hydration errors since Next.js framework doesn't expose this themselves.

@AlexGalays
Copy link

AlexGalays commented Aug 14, 2023

This is a huge pain and makes the console unreadable in dev mode as well as distracting/scary in prod mode. Some of our errors are "normal" / "handled" for some users or configurations. For instance, if a component doesn't have the rights to do something, an ErrorBoundary will catch that and show a nice UI. We also use Suspense and the two tie well together.

What I ended up doing to workaround this limitation:

  1. Tag (e.g, InvariantError extends Error) my custom Errors
  2. override console.error with my own to ignore any instance of the above custom error. This is for production builds.
  3. Likewise, add an error listener on window and event.preventDefault() if event.error is an instance of my custom error. This is for dev builds. This is thanks to this amazing code: https://github.com/facebook/react/blob/main/packages/react-reconciler/src/ReactFiberErrorLogger.js#L93

It really shouldn't be this complicated and undocumented.

@timglabisch
Copy link

bump

@staticshock
Copy link

I'm running into this as well for reasons similar to those explained by @AlexGalays. This is a rather unintuitive limitation.

@jfnault-seedbox
Copy link

bump

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