graphene-django: Let graphene handle serializer errors

I have just bumped into the same issue and first thought this is a bug before I have encountered this issue.

I think this decision should be reconsidered, let me outline why:

1. When executing a mutation with a validation error it returns 200 Success status code even though there is a failure and nothing has been mutated
2. Client always has to request the errors in its query even though in general it is simply interested in the success results
3. when a graphql error occurs graphene currently returns a 400 Bad Request error which indicates that request is invalid and can be fixed. If it were a programmer error it would need to return 500 but this is not the case
4. The specification doesn’t state that those are not fixable by the users (a user for me is the api client). It actually now also allows extensions to give more information to the client.

Reference for status codes: https://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html

What do you think?

I am happy to work on a PR if we can agree that this should be adjusted.

GraphQL errors encode exceptional scenarios - like a service being down or some other internal failure. Errors which are part of the API domain should be captured within that domain.

I am a bit confused by this statement in the related issue as the specification doesn’t explicitly state that this is the case (at least that’s how I read it).

The specification rather states:

If the result of resolving a field is null (either because the function to resolve the field returned null or because an error occurred), and that field is of a Non-Null type, then a field error is thrown. The error must be added to the “errors” list in the response.

Trying to rephrase this for our case: Each field returned by mutation which is null because of an error should have an error in errors.

Hence I suggest Graphene Django adds one error for each validation exception. This might need changes on graphql-core as well? The way how you have suggested would also be a good first step to move forward.

Also such a change depends on a fix for issue https://github.com/graphql-python/graphql-core/issues/203

All in all changing this won’t be trivial as changes in graphql-core are most likely necessary. But could we still reopen this issue? This way users with the same issue will find it better as the discussion continues. And as stated when we agree on a solution I am open to help with a PR if needed.

I am facing an issue maybe similar to this one?

I am using model serializer in serializermutation which relies on models’ field attributes null=True to determine whether a field is required. This is normally fine for most operation except delete, in which requires me to enter data for all required field when only id is needed.

We really need a way to fix this.

`{ “errors”: [ { “message”: “Argument "input" has invalid value {id: "V_cX1txRTVaMI0jDu5uoaQ"}.\nIn field "type": Expected "String!", found null.\nIn field "suffix": Expected "String!", found null.\nIn field "mime": Expected "String!", found null.”,

}

] }`

@patrick91 Thanks for your feedback and working on this. Providing all three options might be a way forward.

Personally I think though this is rather a matter of what the specification states and not what we prefer to do in graphene. Maybe this needs to be raised again within context of graphql specification as it seems the specification is not clear at this point.