aPureBase / KGraphQL

Pure Kotlin GraphQL implementation
https://kgraphql.io
MIT License
305 stars 58 forks source link

Error handling for mutations #68

Open FabioGimmillaro opened 4 years ago

FabioGimmillaro commented 4 years ago

Hi,

is there any support for handling errors like specified in the latest graphQL spec? https://graphql.github.io/graphql-spec/June2018/#sec-Errors

Currently we try to delete multiple entries in our database through one bulk mutation. The problem is that we are uncertain how the graphql-response should look like, when 1 or more entries could not be deleted because, e.g. the current user has the wrong permissions.

Thanks in advance for your answer :)

jeggy commented 4 years ago

The error format defined by the specification currently isn't fully supported. We do plan to follow the specification.

I think I would split this issue into 2 different issues:

  1. Correctly catch errors thrown within your resolver and add them to the errors list.
  2. Be able to throw multiple errors from a single resolver, this would require you(the developer) to throw some specific exception that would be extending some exception that we would define within KGraphQL.
FabioGimmillaro commented 4 years ago

Hi jeggy. Thanks for your answer.

Which errors list do you mean? I'm using kgraphql 0.9.1 and there are no errors lists within my resolvers.

jeggy commented 4 years ago

it's not supported currently, so there is nothing. it should add it to the response object, so it would look something like this:

{
  "data": {...}
  "errors": [...]
}

For now, you can catch the exception yourself with something like this:

try {
  schema.execute(query)
} catch (e: GraphQLError) {
  println(e.prettyPrint())
}

I will make this a high priority to be added, as this is important to have working.

FabioGimmillaro commented 4 years ago

Hi jeggy,

thanks. I did actually something similar to your first suggestion (data + error) Cool. I'm looking forward to the new feature :)

NathanPB commented 3 years ago

@jeggy I don't feel like opening another issue. I've been doing some work about this one, and I reached the point where you are using the DeferredJsonArray/Map class, which I don't understand its purpose and I would like to know to be able to continue trying to build something useful.

Why are you using those classes instead of the simple kotlinx.serialization builders? They seem overcomplicated :flushed: https://github.com/Kotlin/kotlinx.serialization/blob/master/docs/json.md#json-element-builders

jeggy commented 3 years ago

I introduced the DeferredJson builders because I wanted to be able to solve the N+1 problem while also provide a suspendable lambda function to the resolvers and ontop of all that execute the resolvers in parallel :laughing: Which is what makes this possible: https://kgraphql.io/Reference/Type%20System/objects-and-interfaces/#data-loaded-properties

I fully agree the DeferredJson structure is a mess, but that was the best I could come up with to solve all of this in one solution.

PssbleTrngle commented 2 years ago

Are there any updates on this?