Relay: Validating inputs for mutations

[F21]

I have been searching for ways to perform field validations for mutations.

For example, let's save we have a form for registering a user:

Username: someuser
Email: [email protected]
Password: somepassword

When performing the mutation, we need to check on the server that the username and email have not already been registered and signal that to the client and display the appropriate error messages.

As far as I can see from the graphql spec, the errors object (when a business rule fails and we throw an error) looks like so:

{
  "data": null,
  "errors": [
    {
      "message": "The username is already taken!",
      "locations": []
    }
  ]
}

This does not allow us to associate which error message with an input field. In addition, it would be nice if an error code is returned as well, for example (USERNAME_TAKEN), so that its machine readable and the client code and produce an internationalized string out of it.

What do you guys think about adding a validation spec for relay? For example, relay could expect the following structure if an error has occurred:

{
  "data": null,
  "errors": [
    {
      "field": "username",
      "code": "USERNAME_TAKEN",
      "message": "The username is already taken!",
      "locations": []
    }
  ]
}

[ghost]

+1!
I've been banging my head over this the whole day.

As far as I'm concerned, I don't quite need a full blown validation feature. I would simply like to be able to return a machine-readable error. Internationalization is a good example, but also simply adding extra information so that the UI can react with something more useful than a plain string.

[josephsavona]

Relay returns the full mutation error from the server as transaction.getError().source, so you should be able to implement any custom scheme for errors that you like, and access it in the mutation's onFailure callback. Does that solve your use case?

[ghost]

If I pass onFailure to Relay.Store.commitUpdate I get passed a transaction and so I can access the whole GraphQL return data, true. But the question is: how do I generate (server-side) something more interesting than this?

"errors": [ { "message": "foo", "locations": [ { "line": 1, "column": 56 } ] } ]

[dylanahsmith]

According to the Errors section of the GraphQL spec, the top-level errors entry is meant for developers rather than to display to users:

Every error must contain an entry with the key message with a string description of the error intended for the developer as a guide to understand and correct the error.

A similar issue was closed on the GraphQL spec repo (https://github.com/facebook/graphql/issues/117#issuecomment-170180628) where it was suggested that errors for users should be provided as fields on the payload rather than using the top-level errors entry.

[ryancole]

@dylanahsmith if errors are represented on the payload, wouldn't that imply to Relay that the mutation was successful, though? it seems as though if Relay sees that errors array, in the response, it knows that the mutation has failed, and triggers all the expected fail-case actions, such as the onFailure callback. if we have "user errors" as part of a _successful_ payload then Relay will proceed through it's succeed-case actions, such as onSuccess and whatever else it does (run Relay configs, adjust client graph, etc?)

The GraphQL spec says that the errors entries may be extended however needed. Despite that one seemingly confusing comment about errors being for developer-focused messages, it seems like extending an entry in errors, and adding a possible field property or something, would be a pretty good approach because it already works so well with Relay.

I'm not opposed to errors as part of the response payload. In fact, I'm currently implementing my errors this way. I just want to be sure that if I do it this way then Relay won't have any unintended side effects thinking that these mutations actually succeeded. @leebyron @josephsavona

[dylanahsmith]

Relay should be able to handle the configuration changes properly with that design as long as the server responds with the current state of the nodes without the changes applied that were considered invalid:

• FIELDS_CHANGE: Would just refresh those fields with the current state
• RANGE_ADD: The edgeName field should be null and cause it to not be added
• RANGE_DELETE: deletedIDFieldName field should not include the id if the deletion didn't happen
• REQUIRED_CHILDREN: Your onSuccess callback should check for a field that indicates that there were errors then handle the response appropriately

[ryancole]

That's basically how I've proceeded. I do see why this seems a bit strange to some, though. There's already a mechanism for literally failing a query/mutation, but we're going to by pass that and instead opt to abide by these undefined rules so that our client side store doesn't get out of whack. I do think a more deliberate approach to these UI errors would be nice, and would make plenty of sense as part of Relay + GraphQL. It'd take coordination between the two, because I do see that UI errors don't really make sense as a first class concept in GraphQL, but they do somewhere in Relay perhaps.

[wincent]

Thanks for the discussion everybody. I am going to close this now due to inactivity. I think there are a few ways forward here:

1. Work within the existing framework to provide user-visible validation messages.
2. Formally encode the notion of mutation validation errors at the level of the "Relay spec".
3. Formally encode the notion of mutation validation errors at the level of the GraphQL spec.

As pointed out above, the GraphQL spec doesn't (yet) say much about user-visible errors, although the scope for future extensions is explicitly mentioned:

however future versions of the spec may describe additional entries to errors.

I think I'd want to at least discuss the options there before proceeding down the path of any Relay-specific extensions, modifications or workarounds. I know that there have been informal discussions about error-reporting internally at FB, so the team is definitely thinking about it; if you have opinions on the subject, now is a good time to make yourself heard.