graphql / graphql/graphql-spec

[RFC] Require selection of error type in data

Open
#528 6 comments 6 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
14.6k
Forks
1.2k
PR merge metrics
No merged PRs in 30d

Description

The GraphQL community has been pretty vocal that the `errors` dictionary is confusing for representing domain-level errors. At the same time @leebyron has stated the position that `errors` is only for exceptional things (e.g. GraphQL parsing errors, systems being down, etc) and not for user triggered errors (e.g. malformed input). See #135 & #391

As Braintree has been transitioning [our public API](https://graphql.braintreepayments.com/) in GraphQL, we've found that there are pros and cons with using `errors` to represent user triggered errors. The most obvious pro is all of the errors are co-located so that a client does not need to know about another concept of error. If something didn't work, you look in `errors`.

While being co-located is nice, using `errors` to represent user errors becomes difficult and loses the benefits of GraphQL, most notably documentation and types. The spec allows for "extensions" to `errors`, which becomes a dumping ground of undocumented, untyped keys and values. How do users know the structure, types, and cause of these errors in standard tooling besides trial and error?

On the other hand, putting these errors in `data` means a user not only has to handle errors by looking at two different locations, but the user could not know they need to select domain errors as part of the mutation, causing clients to miss out on information that is challenging to replicate. You'll probably only make this mistake once, but if you're integrating with a few different mutations over a larger period of time or across teams, the mistake may be repeated multiple times. In the end, this forces people to _always_ select these `error` fields, which leads to my proposal.

I think we could use the GraphQL validation system to ensure errors are selected for the response. Note the following schema definition:

```graphql
type Mutation {
createUser(input: UserInput!): UserPayload
}

input UserInput {
name: String
email: String
...
}

type UserPayload {
id: ID
name: String
...
mutationErrors: [MutationError]
}

error MutationError {
message: String
code: String

"""
Input field on which caused the error
example: ["input", "name"]
"""
inputPath: [String]
}
```

If the client were to execute a query _without_ selecting the `mutationErrors` field, the GraphQL validation system would fail the query, placing the error in `errors`:

```graphql
mutation CreateUser($input: UserInput!) {
createUser(input: $input) {
id
}
}
```

Executing results in:

```json
{
"data": null,
"errors": [{
"message": "Field 'mutationErrors' required to be selected for 'createUser' mutation",
"path": "createUser"
}]
}
```

To resolve the GraphQL validation error select the `mutationErrors` field with any or all sub-fields:

```graphql
mutation CreateUser($input: UserInput!) {
createUser(input: $input) {
id
mutationErrors {
code
}
}
}
```

The use of a new top-level keyword `error` is to tell the validation system that the field is required to be selected and included in part of the response.

This may open a slippery slope that goes against a core ideal of GraphQL: the client selects what is to be returned. I think, though, in some instances having the server give some restrictions, especially around the case of user error handling, this could make sense. And, it's not required that you use the `error` keyword or even errors in your Mutations at all, in which case the client can keep behaving as it does today.

Note that adding an `error` type to an existing Mutation would be backwards-incompatible in the same way as making an input field non-nullable.

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.