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

Discussion: Should an error in onUpdate trigger the container's error boundary? #220

Open
obweger opened this issue Dec 30, 2023 · 3 comments

Comments

@obweger
Copy link
Contributor

obweger commented Dec 30, 2023

Team,

just something that hit me by surprise the other day: If an error is thrown in an RSS container's onInit, the error appears to be considered a rendering error and hence bubbles up to and triggers the next error boundary. However, if this happens in onUpdate, the error doesn't appear to be considered a rendering error and hence doesn't end up triggering the error boundary.

I've now worked around this by catching the error myself in onUpdate, calling an onUpdateError handler in the container's props, and triggering the error boundary myself - but my gut feeling is that this should happen automatically.

Just food for thought and discussion - grazie!

@albertogasparin
Copy link
Collaborator

Good observation, didn't realise that.
The difference is indeed that onInit is called as part of a hook initialisation while onUpdate is called by the subscription, so as part of the schedule/batching process.
I'm not sure however that onInit behaviour is what we should normalise. The reason is that being able to catch it via error boundary is possible only for sync actions. Async actions will always need to be manually caught.

So my worry is that devs might not realise they need an error boundary in the first place. I feel having an out of render error is slightly more annoying to catch but less fatal. And if you use async actions you need to manually handle it regardless.
But open to hear your counter arguments :)

@theKashey
Copy link

Errors in general should not happen, but if they do - I think it's important to break things at the right place.

Personally - I prefer when things breaks and I "see" it, not then they break silently and I have to idea why UI became glitchy

@obweger
Copy link
Contributor Author

obweger commented Jan 2, 2024

Agree with @theKashey here - my philosophy is also that if something breaks, it should break fully.

For async actions @albertogasparin, I personally think it's fine for these to not trigger the error boundary; after all, I can also trigger any async stuff in a render function and it would also not trigger the error boundary.

My 2c.

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

No branches or pull requests

3 participants