Thoughts and feedback on the @catch directive
Opened this issue · 1 comments
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.
type Book implements Node {
id: ID!
title: String!
author: String!
coAuthor: String
}
type Query {
bookById(id: ID!): Book
}
Using @catch
in a schema with non-null fields
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:
const data = query BookQuery($id: ID!) {
bookById(id: $id) @catch {
id
title
author
coAuthor
}
}
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>
}
... render Book
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.
const data = query BookQuery($id: ID!) {
bookById(id: $id) @catch {
id
title
author
coAuthor @catch(to: NULL)
}
}
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.
const data = query BookQuery($id: ID!) {
bookById(id: $id) @catch {
id
title
author
coAuthor @catch(to: NULL)
publisher @catch(to: NULL)
language @catch(to: NULL)
averageRating @catch(to: NULL)
...
}
}
Using @catch
in a fully nullable schema
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
const data = query BookQuery($id: ID!) {
bookById(id: $id) @catch {
id
title @required(action: THROW)
author @required(action: THROW)
coAuthor
}
}
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.
const data = query BookQuery($id: ID!) {
bookById(id: $id) @catch {
id # Errors here cause an ErrorResult above
title # Errors here cause an ErrorResult above
author # Errors here cause an ErrorResult above
coAuthor # 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.
const data = query BookQuery($id: ID!) {
bookById(id: $id) @catch {
id
title
author
coAuthor @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.
const data = query BookQuery($id: ID!) {
bookById(id: $id) @catch {
id
title @required(action: THROW)
author @required(action: THROW)
coAuthor
}
}
However another alternative would be to make this behavior opt-in.
const data = query BookQuery($id: ID!) {
bookById(id: $id) @catch(nonNullOnly: true) {
id
title
author
coAuthor
}
}
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.
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.