Mutation errors mechanism seems sub-optimal

[rocketraman]

I reviewed https://graphql-rules.com/rules/mutation-payload-errors closely. There seems to be a better approach.

The rule suggests:

type LikePostPayload {
  recordId: Int
  # `record` is nullable! If there is an error we may return null for Post
  record: Post
  errors: [LikePostProblems!]
}

Note the statement:

record is nullable! If there is an error we may return null for Post

Of course, the downside of this is that making record: Post nullable solely for this reason conflicts with rule https://graphql-rules.com/rules/output-non-null. Typed languages on the client side that handle nullability (Kotlin, Elm, Typescript, Rescript, etc.) now lose the field nullability information for the success case.

Instead, why not define the payload as a union type of both success and error conditions:

union LikePostPayload = LikePostSuccess | SpikeProtectionProblem | PostDoesNotExistsProblem

type LikePostSuccess {
  recordId: Int!
  record: Post!
}

The LikePostPayload union very nicely documents in one place all the possible return values for the mutation, and typed client languages with pattern matching or equivalent functionality deal very nicely with this. Furthermore, the LikePostSuccess type correctly models the nullability of the recordId and record fields for the success case.

The mutation is cleaner because it is now symmetric and __typename can now be a top-level field rather than being buried underneath errors. In other words, there is a single top-level discriminator for clients, rather than the client having to inspect the errors field first:

mutation {
  likePost(id: 666) {
    __typename
    ... on LikePostSuccess {
      recordId
      record {
        title
        likes
      }
    }
    ... on ProblemInterface {
      message
    }
    ... on SpikeProtectionProblem {
      wait
    }
    ... on PostDoesNotExistsProblem {
      postId
    }
  }
}

NOTE: As an aside, I also removed message from ... on SpikeProtectionProblem and ... on PostDoesNotExistsProblem. It is not necessary because message will already be returned in both those cases via ... on ProblemInterface.

[rocketraman]

One more point: after just now reviewing the linked video about errors, she actually uses the pattern I describe above, not the pattern in the text of the rule.

[RIP21]

@rocketraman I agree with you, I'm on that side too.
The only problem with that approach is that sometimes you may have multiple problems returned and with this approach, you kinda locked into one. It's fine if there is usually only 1 problem at a time guaranteed.
It's easy to fix tho, having a LikePostFail type that will have problems field that will be an array of Problems of different types..
Then you'll just handle Success or Fail case and in case of fail you'll iterate over problems, again, fully typed.

[RIP21]

I think it will be valuable to contribute that discussion/issue to the actual rules.

[rocketraman]

The only problem with that approach is that sometimes you may have multiple problems returned and with this approach, you kinda locked into one. It's fine if there is usually only 1 problem at a time guaranteed.

Not at all. The payload type is a union of the success type (or types) and all the relevant problem types i.e. in the example above:

union LikePostPayload = LikePostSuccess | SpikeProtectionProblem | PostDoesNotExistsProblem

both the SpikeProtectionProblem and the PostDoesNotExistsProblem are possible problems.

It's easy to fix tho, having a LikePostFail type that will have problems field that will be an array of Problems of different types..

With this model, a common per-mutation problem union type doesn't add much value. There is already a common interface (ProblemInterface) for error types, and if the client doesn't care about the specific error types it can use that instead e.g.:

when(response) {
  is LikePostSuccess -> ...
  is ProblemInterface -> ...
}

or if discrimination of one or more problem types is necessary:

when(response) {
  is LikePostSuccess -> ...
  is SpikeProtectionProblem -> ...
  is PostDoesNotExistsProblem -> ...
}

Adding a LikePostFail union type doesn't enable any additional useful client-side patterns. It may have value as an interface type in specific cases.