Stream: implementers
Topic: GF#17409: GraphQL errors
Grahame Grieve (Jul 01 2018 at 22:17):
When I drafted the graphQL page (http://build.fhir.org/graphql) I said to use FHIR operation outcome in case of errors - it's a richer error regarding the underlying problem. but an implementer has pointed out that this isn't good for existing client libraries. Does anyone have any comment on this (see http://facebook.github.io/graphql/draft/#sec-Response-Format for graphql spec).
Robert Winterbottom (Jul 03 2018 at 13:41):
Hi @Grahame Grieve, I have some experience with client side libraries that use GraphQL. Two big ones are Relay Modern and Apollo Client and they both tend to use the method described in the GraphQL specification. While apollo-client does seem to have the ability to handle network errors simultaneously with graphql errors, relay modern does not do so very cleanly(if someone has more experience with this framework and can comment I would be curious to know). This could be problematic for developers using these libraries.
I am also actively developing a GraphQL server in Node.js which uses a library written by the team at Facebook. They do a lot to try to prevent you from doing things against the spec, so setting response status codes and reshaping the JSON response is not very straightforward. By default, they do not give you access to the response object on the server since they handle response codes and format for you. If you work around it you can cause unhandled promise rejection errors, which in future versions of node.js will terminate the node process. There is some discussion here, https://github.com/graphql/express-graphql/pull/118#issuecomment-258027222, from Lee Byron, one of the core maintainers of GraphQL, about adding a hook to reshape the response to something else (like an operation outcome).
GraphQL does have an extensions property that exists on errors and could contain an OperationOutcome. This would only require clients to check the response.errors property for any errors, which all GraphQL clients currently support. I think this may be the preferred way to go. There are some hacky ways to proxy all requests coming into GraphQL so you can handle the response in case of an error, but this could break if the GraphQL implementation changes.
Grahame Grieve (Jul 03 2018 at 20:10):
ok, so we could say that you SHALL return a valid graphQL error response, and SHOULD also return an OperationOutcome as an extension?
Robert Winterbottom (Jul 05 2018 at 14:12):
That sounds good to me. Here is an example error object in the spec with extensions being used, http://facebook.github.io/graphql/draft/#example-fce18. I am at the point with the server I am building where I am adding in error handling. If it would be helpful I can put something together explaining all the cases I am encountering and examples of each and post them back here (or wherever is appropriate). Most of them can be summed up with the two scenarios.
Someone makes a request that DOES NOT make it to a valid graphql endpoint, e.g. /[base]/SomeNonexistentProfile/2/$graphql. In this case I am not sure if it is better to return a normal operation outcome since we are not hitting graphql, or if it is better to format an operation outcome to look like a graphql error for consistency sake (putting it in an extension property under errors), so I am curious what you think about this.
Someone makes a request that DOES make it to a valid graphql endpoint. /[base]/Patient/2/$graphql. Here inside graphql we can handle authentication errors, patient does not exist (404 because of the id), internal server errors, and any other error we encounter. These can be returned as you described, as a valid graphql error which should also contain an operation outcome under extension.
Grahame Grieve (Jul 05 2018 at 19:08):
I think that the former case has to be a non-graphql error message - else you're saying, any appearance of the graphql parameter invokes global graphql error handling.... overriding the mime type... sounds like a really bad idea
Robert Winterbottom (Jul 05 2018 at 19:39):
Oh right, I see. If the response was a traditional OperationOutcome than the mime-type would be application/fhir+json, but if we reshaped it to match the GraphQL spec, then we would have to change the mime-type to application/json. I agree changing the mime-types would be a bad idea.
So for the former case, we should return a standard operation outcome indicating the issue with mime-type application/fhir+json and for the latter case, when a user reaches the GraphQL endpoint, we return something like the following with a mime-type of application/json ?
{ "data": null, "errors": [ { "extensions": { "resourceType": "OperationOutcome", "issue": [ { "severity": "error", "code": "exception", "diagnostics": "500: Internal server error" } ] }, "message": "500: Internal server error" } ] }
Grahame Grieve (Jul 05 2018 at 19:40):
yes though I thought it would be this:
Grahame Grieve (Jul 05 2018 at 19:40):
{
"data": null,
"errors": [
{
"extensions": {
"resource" : {
"resourceType": "OperationOutcome",
"issue": [
{
"severity": "error",
"code": "exception",
"diagnostics": "500: Internal server error"
}
]
},
"message": "500: Internal server error"
}
}
]
}
Robert Winterbottom (Jul 05 2018 at 19:44):
I like that. The GraphQL spec says we can put anything under the extensions property as long as it is a map, so placing the content under a resource property sounds like a great idea. This also allows developers to still put their own information in extensions without interfering with the structure of the operation outcome json.
Grahame Grieve (Jul 05 2018 at 19:49):
k I'll change the graphql page soon
Robert Winterbottom (Jul 05 2018 at 19:50):
Awesome, thanks @Grahame Grieve
Last updated: Apr 12 2022 at 19:14 UTC
