facebook / facebook/relay

Thoughts and feedback on the @catch directive

Open
#4,867 3 comments 3 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
19k
Forks
1.9k
PR merge metrics
No merged PRs in 30d

Description

*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.

```graphql
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:

```graphql
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.

```javascript
if(!data.bookById.ok){
return

Book failed to load, please try again.

}

if(data.bookById.value === null){
return

Book not found!

}
... 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.

```graphql
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.

```graphql
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

```graphql
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.

```graphql
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.

```graphql
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.

```graphql
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.

```graphql
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.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.