Discussion: Generic error codes in the spec

[pranshuchittora]

Abstract
I am researching and planning to build a generic test suite to check the compliance of the various GraphQL libraries The project idea is given by the GraphQL Foundation for the Google Summer of Code 2020. There are various implementations of GraphQL and all of them are throwing error in various ways.
As all errors are thrown in plain text(string). There is no way to test these implementations for such errors.

Say we have given an incorrect query (with circular fragment) to a GraphQL server and we are expecting an error. If the error comes with a specific error code then it would be great to test the libraries and provides and assuring the actual cause of the error, therefore, improving the DX as well.

For example imagine a well known error place that can hold a "spec rule number" that is indicative of what validation rule you have caused an error message to fail on

errors : [ { message : "any old message", extensions : [ __graphqlspec : { reason : "March2018-5.1.2.4" }}}}

Follow this discussion -> graphql-java/graphql-java#1826 (comment)

Motivation
I am really impressed by the standardisation of the error codes in the following projects.

• React error codes -> https://github.com/facebook/react/blob/master/scripts/error-codes/codes.json
• Rust lang -> https://doc.rust-lang.org/error-index.html
• TypeScript -> https://github.com/microsoft/TypeScript/blob/master/src/compiler/diagnosticMessages.json

What can be done
It would be great if we can come up with such standard error codes in the GraphQL spec.

[sungam3r]

OK. My thoughts:

1. No breaking changes. Good.
2. Easy to implement.
3. It should be explicitly stated in spec (if we decide to supplement the spec) that attaching the error information about code is optional for the server.
4. I propose to use __graphqlspec as a conventional prefix for all such things and not as full property name. Full property name for error code may be __graphqlspecErrorCode

[shobhitchittora]

Well @sungam3r IMO the whole point of adding such error codes is to enforce / streamline error codes across implementations. Having it optional would defeat the whole purpose. What are your thoughts on this. What effort do you think would go in this route?

[sungam3r]

That is simple. It is impossible to do such error codes as required properties since

1. Breaking changes to existing implementations.
2. Extensions property by design has no structure defined.

[sungam3r]

See https://github.com/graphql/graphql-spec/blob/master/CONTRIBUTING.md#guiding-principles

Future changes should not change the meaning of existing schema or queries or in any other way cause an existing compliant GraphQL service to become non-compliant for prior versions of the spec.

[markokr]

How about a optional "catagory" key, so clients can make broad decision and servers are testable whether right check triggered?

Enumerating individual errors seems lot of work, for dubious benefit.

Main disadvantage would be that some implementations opt out from it if it's too bothersome.

[sungam3r]

Well, category or code are the same things. GraphQL-dotnet for example already has error codes in response https://github.com/graphql-dotnet/graphql-dotnet/blob/master/src/GraphQL.NewtonsoftJson/ExecutionResultJsonConverter.cs#L98

[markokr]

code is fine as a key. The question is can other implementations reproduce them.

Main goal is spec regression tests that can be run over several implementations.

It's fine to say that this testsuite requires support for code.

[pranshuchittora]

Well, category or code are the same things. GraphQL-dotnet for example already has error codes in response https://github.com/graphql-dotnet/graphql-dotnet/blob/master/src/GraphQL.NewtonsoftJson/ExecutionResultJsonConverter.cs#L98

That looks interesting, can you point me to the exact listing of these error codes.

[michaelstaib]

Code is already used in many implementations so I would go with that. We also use code in Hot Chocolate. The more important issue is to specify error codes for spec errors.

[sungam3r]

That looks interesting, can you point me to the exact listing of these error codes.

Errors codes are aligned with paragraph numbers from the spec. 5.4.2.1 for http://spec.graphql.org/June2018/#sec-Required-Arguments for example.

[markokr]

Hardwiring chapter numbers as errors seems wrong, both implementations side and also spec side: one-chapter-per-error may be annoying to maintain, especially if you are not allowed to refactor.

Named errors seems good enough, or if you really want numbers, then OID-like tree maintained in spec.

[michaelstaib]

I also would not bind them to the chapters. I personally also prefer named errors. But that might be a personal preference I would also be fine with something else.

[dariuszkuc]

Similar to "error code", we are using system events for all our exception handling but we expose it in error extension block (as per spec).

When following code first approach, schema generation process should not allow generation of invalid schema. Your runtime should already run valid schema that produces correct responses so I am unsure how often you would end up with something non-compliant with the spec. I believe you need this sort of validation when creating the schema but if that is the case then I am unsure why you would need this error information in GraphQL response as the server should not start with invalid schema.

[sungam3r]

@dariuszkuc The talk is about query validation errors as well.

[sungam3r]

@markokr We can use different properties together- code like UNIQUE_ARGUMENT, number like 1.2.3.4 , even helpUrl to spec like http://spec.graphql.org/June2018/#sec-Argument-Names