You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
In traditional GraphQL, positions that are likely to error are made nullable. Now we're fixing that so that we can indicate the semantic nullability, we're losing the information from the schema designers about where errors are likely to happen (problematic fields, service boundaries, etc). This is more a "hint to developers" than a true schema feature IMO, and moving these over time should be fine (non-breaking) - after all, an error can occur anywhere, and all errors should be handled.
I'm currently thinking an annotation such as @errorBoundary would work well - indicates you should consider adding an error boundary here. I don't think we'd need to give it a levels argument - that's up to the client developers to decide.
With the "throw on error" behavior, the first error to be rendered is going to be the one thrown; e.g. if you have this:
it might be that the "forbidden" error from post.message (non-nullable) would be the one that blew up the response in onError: PROPAGATE mode, but the E_SERVICE_UNAVAILABLE error from avatarUrl (nullable) would be the one that causes this component to raise an error in throw-on-error with onError: NULL, because it's the first one hit.
Rendering <p>Error: sorry one of our servers is having some troubles right now</p> wouldn't be wrong, but what we should really be rendering is <p>Forbidden; access to this content is disallowed</p> - we should have added an error boundary specifically around the avatar because we know that service is flaky.
This maybe hints that Relays approach - throwing from access to the fragment - is likely the more correct one, but not without additional information. Currently Relay can look at the schema to determine which error(s) were the critical ones (using the non-null to guide them), but once we move over to semantic nullability we lose this information. We either need to add info to errors, the schema, or both.
In the very worst case, imagine loading an entire page for the latest hundred posts from your city, only having a root error boundary, and suddenly rendering "This user has not set a username yet" because some lucky noob managed to post before setting a username. The user would be super confused!
Since we're rewriting error handling anyway, we should do what we can to make sure error handling is as natural and expected as possible... and I don't think we're quite there yet. Close, but not quite there. Maybe it's as simple as encouraging every component to render its own error boundary?
I don't have a concrete proposal here, just want to kick off the discussion.
reacted with thumbs up emoji reacted with thumbs down emoji reacted with laugh emoji reacted with hooray emoji reacted with confused emoji reacted with heart emoji reacted with rocket emoji reacted with eyes emoji
Uh oh!
There was an error while loading. Please reload this page.
-
In traditional GraphQL, positions that are likely to error are made nullable. Now we're fixing that so that we can indicate the semantic nullability, we're losing the information from the schema designers about where errors are likely to happen (problematic fields, service boundaries, etc). This is more a "hint to developers" than a true schema feature IMO, and moving these over time should be fine (non-breaking) - after all, an error can occur anywhere, and all errors should be handled.
I'm currently thinking an annotation such as
@errorBoundary
would work well - indicates you should consider adding an error boundary here. I don't think we'd need to give it alevels
argument - that's up to the client developers to decide.With the "throw on error" behavior, the first error to be rendered is going to be the one thrown; e.g. if you have this:
it might be that the "forbidden" error from
post.message
(non-nullable) would be the one that blew up the response inonError: PROPAGATE
mode, but theE_SERVICE_UNAVAILABLE
error fromavatarUrl
(nullable) would be the one that causes this component to raise an error in throw-on-error withonError: NULL
, because it's the first one hit.Rendering
<p>Error: sorry one of our servers is having some troubles right now</p>
wouldn't be wrong, but what we should really be rendering is<p>Forbidden; access to this content is disallowed</p>
- we should have added an error boundary specifically around the avatar because we know that service is flaky.This maybe hints that Relays approach - throwing from access to the fragment - is likely the more correct one, but not without additional information. Currently Relay can look at the schema to determine which error(s) were the critical ones (using the non-null to guide them), but once we move over to semantic nullability we lose this information. We either need to add info to errors, the schema, or both.
In the very worst case, imagine loading an entire page for the latest hundred posts from your city, only having a root error boundary, and suddenly rendering "This user has not set a username yet" because some lucky noob managed to post before setting a username. The user would be super confused!
Since we're rewriting error handling anyway, we should do what we can to make sure error handling is as natural and expected as possible... and I don't think we're quite there yet. Close, but not quite there. Maybe it's as simple as encouraging every component to render its own error boundary?
I don't have a concrete proposal here, just want to kick off the discussion.
Beta Was this translation helpful? Give feedback.
All reactions