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
Note: I'm certain that the @catch directive has had a ton of design work and has been implemented internally at Meta, so it's entirely possible I'm missing something here. I'd love to be wrong and find out that there's a simple solution to this problem.
Our team has recently upgraded to Relay 18 and we've been attempting to consume the new @catch directive as a better way to handle errors. Prior to this we had implemented our own custom functions to attempt to expose the errors information in a GraphQL response to components. The hope was that we'd be able to completely remove those functions in favor of a Relay-native solution.
And in some cases, that's been true. We really enjoy the Result type on fields that are annotated with @catch as it makes error handling easy and functional. There are other places, however, where it feels like more iteration is needed on these new directives for us to be able to fully adopt the feature.
The hope with this issue is to bring up some of these issues and hopefully influence how the @catch directive (and potentially also the @throwOnFieldError directive) behave once they’re in a non-experimental state.
Example schema
Here's a simple schema that we'll refer to throughout.
I have a component that wants to query the bookById field and display the book information or an error if the book couldn't be retrieved or didn't exist. And I want to make sure I differentiate between those two error states so that I can either show a "Book failed to load" vs as "Book not found" message. My first inclination would be to do something like this:
Now when I render my component I'm able to differentiate my error states correctly.
if(!data.bookById.ok){return<div>Book failed to load, please try again.</div>}if(data.bookById.value===null){return<div>Book not found!</div>}
... renderBook
This is greatly improved on prior error handling in Relay because I'm able to easily differentiate between a field being null because of an error vs being null because of a resolver error. However, this has subtly created a different problem. If any of the sub-fields on a Book fail to resolve, I'll tell the user that we couldn't load the book, when in reality it was only a single sub-field of the Book that we couldn't load. This might make sense for the fields that are marked as non-null (id, title, author), but it doesn't feel like it makes sense for the coAuthor field which is nullable.
If I'm trying to render my Book object and the coAuthor field resolver fails, it will propagate that failure up and take out all other fields on the Book object. This is similar to the null bubbling behavior that GraphQL servers implement. However, with @catch it's different in that the coAuthor field is non-null so this null-bubbling is unexpected.
Now one way to solve this would be to add an additional @catch directive.
Note: My assumption here is that this component is ok with coercing expected-null and error-null on the coAuthor field. We don't feel the need to tell the user that we couldn't resolve the coAuthor field, we'll just hide that portion of the UI in both cases.
With this query, any failures within the coAuthor field will cause the field to be coerced to null, which our component handles correctly. It will also prevent us from taking out the id, title, and author fields from the Book object if that field fails to resolve.
However, this solution doesn't feel scalable. I'd first have to muddy up all of my queries with a number of @catch directives, let alone have to remember to add it to any new non-null fields I add to this query.
The situation doesn't change all that much in a fully nullable schema, which our team uses. In a fully nullable schema the query from above might change to something like this
But the same issues from above exists here where an error on the coAuthor field will cause the entire Book object to return a ResultError.
What could this do instead?
Only bubble errors on non-null fields
Initially, let’s assume we’re using our non-null schema from above where id, title, and author are non-null in the schema.
One option would be for Relay to change its behavior of @catch to only bubble up errors that happen on non-null child fields. This would more accurately represent the error bubbling that happens on the server.
constdata = queryBookQuery($id: ID!) {
bookById(id: $id) @catch {
id # Errors here cause an ErrorResult abovetitle # Errors here cause an ErrorResult aboveauthor # Errors here cause an ErrorResult abovecoAuthor # Any error that happens here would cause this field to be null
}
}
If my component needed to be able to differentiate between legitimate null and error null for the coAuthor field, I could always put a localized @catch directive on that field to handle that field specifically.
constdata = queryBookQuery($id: ID!) {
bookById(id: $id) @catch {
idtitleauthorcoAuthor@catch # I want to know specifically if this field had an error
}
}
If we shift to a fully nullable schema (for sake of simplicity let’s ignore the id field for a second), we see some interesting behavior. In that case, the bookById field would only ever be an ErrorResult if that specific field contained an error. That may feel odd for an @catch directive on a parent field, but if it’s combined with the @required directive it starts to visually make more sense.
I haven't thought through all of the implications of changing this behavior and I'm certain that there are use cases and edge cases where this would be undesirable. But I also feel like the use case from above is fairly common and something that should have a supported solution. I also suspect that there is behavior overlap here with the @throwOnFieldError directive. We’re also hesitant to use that directive as well because of the same granularity problems; we don’t want errors on certain nullable fields to cause an entire query/fragment to throw an error.
Also, I’ve specifically avoided talking about the @semanticNonNull directive as part of this, mostly because I’m trying to keep the discussion simple for now, but also because I believe the @catch directive should work for the above use case without relying on a solution that requires the @semanticNonNull directive.
The text was updated successfully, but these errors were encountered:
Hi @ernieturner - thanks! This is rally great feedback. I think some of the expectations here differ a bit from our assumptions when building @catch. You're right that we did a ton of ergonomics and use-case-enumerating work, but of course there's always feedback like this that surfaces what we didn't expect. I'll reread this in a bit and give my thoughts.
Note: I'm certain that the
@catch
directive has had a ton of design work and has been implemented internally at Meta, so it's entirely possible I'm missing something here. I'd love to be wrong and find out that there's a simple solution to this problem.Our team has recently upgraded to Relay 18 and we've been attempting to consume the new
@catch
directive as a better way to handle errors. Prior to this we had implemented our own custom functions to attempt to expose theerrors
information in a GraphQL response to components. The hope was that we'd be able to completely remove those functions in favor of a Relay-native solution.And in some cases, that's been true. We really enjoy the
Result
type on fields that are annotated with@catch
as it makes error handling easy and functional. There are other places, however, where it feels like more iteration is needed on these new directives for us to be able to fully adopt the feature.The hope with this issue is to bring up some of these issues and hopefully influence how the
@catch
directive (and potentially also the@throwOnFieldError
directive) behave once they’re in a non-experimental state.Example schema
Here's a simple schema that we'll refer to throughout.
Using
@catch
in a schema with non-null fieldsI have a component that wants to query the
bookById
field and display the book information or an error if the book couldn't be retrieved or didn't exist. And I want to make sure I differentiate between those two error states so that I can either show a "Book failed to load" vs as "Book not found" message. My first inclination would be to do something like this:Now when I render my component I'm able to differentiate my error states correctly.
This is greatly improved on prior error handling in Relay because I'm able to easily differentiate between a field being null because of an error vs being null because of a resolver error. However, this has subtly created a different problem. If any of the sub-fields on a
Book
fail to resolve, I'll tell the user that we couldn't load the book, when in reality it was only a single sub-field of theBook
that we couldn't load. This might make sense for the fields that are marked as non-null (id
,title
,author
), but it doesn't feel like it makes sense for thecoAuthor
field which is nullable.If I'm trying to render my
Book
object and thecoAuthor
field resolver fails, it will propagate that failure up and take out all other fields on theBook
object. This is similar to the null bubbling behavior that GraphQL servers implement. However, with@catch
it's different in that thecoAuthor
field is non-null so this null-bubbling is unexpected.Now one way to solve this would be to add an additional
@catch
directive.Note: My assumption here is that this component is ok with coercing expected-null and error-null on the
coAuthor
field. We don't feel the need to tell the user that we couldn't resolve thecoAuthor
field, we'll just hide that portion of the UI in both cases.With this query, any failures within the
coAuthor
field will cause the field to be coerced tonull
, which our component handles correctly. It will also prevent us from taking out theid
,title
, andauthor
fields from theBook
object if that field fails to resolve.However, this solution doesn't feel scalable. I'd first have to muddy up all of my queries with a number of
@catch
directives, let alone have to remember to add it to any new non-null fields I add to this query.Using
@catch
in a fully nullable schemaThe situation doesn't change all that much in a fully nullable schema, which our team uses. In a fully nullable schema the query from above might change to something like this
But the same issues from above exists here where an error on the
coAuthor
field will cause the entireBook
object to return aResultError
.What could this do instead?
Only bubble errors on non-null fields
Initially, let’s assume we’re using our non-null schema from above where
id
,title
, andauthor
are non-null in the schema.One option would be for Relay to change its behavior of
@catch
to only bubble up errors that happen on non-null child fields. This would more accurately represent the error bubbling that happens on the server.If my component needed to be able to differentiate between legitimate null and error null for the
coAuthor
field, I could always put a localized@catch
directive on that field to handle that field specifically.If we shift to a fully nullable schema (for sake of simplicity let’s ignore the
id
field for a second), we see some interesting behavior. In that case, thebookById
field would only ever be anErrorResult
if that specific field contained an error. That may feel odd for an@catch
directive on a parent field, but if it’s combined with the@required
directive it starts to visually make more sense.However another alternative would be to make this behavior opt-in.
I haven't thought through all of the implications of changing this behavior and I'm certain that there are use cases and edge cases where this would be undesirable. But I also feel like the use case from above is fairly common and something that should have a supported solution. I also suspect that there is behavior overlap here with the
@throwOnFieldError
directive. We’re also hesitant to use that directive as well because of the same granularity problems; we don’t want errors on certain nullable fields to cause an entire query/fragment to throw an error.Also, I’ve specifically avoided talking about the
@semanticNonNull
directive as part of this, mostly because I’m trying to keep the discussion simple for now, but also because I believe the@catch
directive should work for the above use case without relying on a solution that requires the@semanticNonNull
directive.The text was updated successfully, but these errors were encountered: