Current limitations in GraphQLError

[sibljon]

Thanks to all who have contributed to this fantastic library.

It would be great to be able to be able to have more flexibility when reading from a GraphQL response's errors array. Specifically:

1. The GraphQL spec calls out locations as an key in a GraphQL error in addition to message. It seems straight forward add a locations key to GraphQLError.
2. Although the spec doesn't specifically mention other error keys, it would be useful to be able to parse additional keys (ex: userMessage -- a message that's guaranteed to be presentable to the user). To do so, it seems like adding an additional type parameter to GraphQLResult resulting in GraphQLResult<Data, ErrorType> (sadly, plain Error already exists in the namespace), thus allowing consumers of apollo-ios to define their own error types. This is clearly a more involved change, but is crucial to my own usage of apollo-ios.

Lastly, an certainly least importantly, the aforelinked spec section also mentions:

If the data entry in the response is null or not present, the errors entry in the response must not be empty. It must contain at least one error. The errors it contains should indicate why no data was able to be returned.

Thus, it seems like an enum might be better suited for this task rather than data and errors both being optionals. Result is a good example of this, and on potential implementation would be for GraphQLError to have a property of type Result.

[martijnwalraven]

(Responding here instead of on the PR for context.)

Thanks for bringing this up! I agree adding locations to GraphQLError makes sense.

I also see why custom error keys would be useful, but forcing users to specify a custom error type seems like a very heavyweight solution. Swift doesn't support default type arguments, so your PR would force you to always fully specify the type parameters of GraphQLResult everywhere, e.g. for every fetch.

Instead of custom error types, an alternative might be to modify GraphQLError to allow storing additional keys as a dictionary. By default, it would make them available untyped through a subscript. And then you could add typed accessors in your own app as an extension on GraphQLError:

extension GraphQLError {
  var userMessage: String? {
    return self["userMessage"] as? String
  }
}

How does that sound?

[sibljon]

Good call on the light-weight solution you've proposed -- it seems like it'll be better for most consumers of this library. I hadn't noticed the inconvenience since I only use fetch and perform once in a wrapper, but you're right that it's there for those who don't wrap.

I'll go ahead and close #40. Implementing this is not something I've got steam for right now, but perhaps in the future.

[martijnwalraven]

I implemented the solution we discussed as part of f77286f.

[benrom]

@martijnwalraven I'm implementing the GraphQLError extension you suggested above, xcode complains on the self[ bit with:

Type GraphQLErrorhas no subscript members

Is there something I'm missing?

[martijnwalraven]

@benrom: This was only recently added and is not not in the patch 0.4.3 version yet.