Add more details to static errors raised after the query run

[sobrinho]

Is your feature request related to a problem? Please describe.

Static errors that makes the query invalid points straight to the error location, i.e.:

{
  "errors": [
    {
      "message": "Field 'documents' is missing required arguments: kind",
      "locations": [
        {
          "line": 7,
          "column": 7
        }
      ],
      "path": [
        "query",
        "employees",
        "documents",
        "kind"
      ]
    }
  ]
}

But static errors such as non-nullable field does not, i.e.:

{
  "errors": [
    {
      "message": "Cannot return null for non-nullable field Employee.status"
    }
  ]
}

The problem is that it makes really hard to debug since you can have a query that return multiple nodes such as:

query {
  employees {
    id
    fullName
    status
  }
}

If you have 100 employees, which one did return an empty status?

Describe the solution you'd like

Would be nice to have the error wrapped pointing to the specific object that failed the validation, i.e.:

{
  "errors": [
    {
      "message": "Cannot return null for non-nullable field Employee.status",
      "object": {
        "wrapping_type": "EmployeeType",
        "type": "Employee",
        "gid": "..."
      }
    }
  ]
}

Describe alternatives you've considered

We built a GraphQL::Schema::FieldExtension to add this context to unhandled exceptions but it doesn't trigger for static errors.

Be aware of the two kinds of static errors: on the query (missing argument, undefined field) and on the result (non-nullable fields).

While the first one doesn't have a backtrace, it does show the line and column on the query and the path.

The second one doesn't have a backtrace and points to the type/field that failed but not the object or a backtrace.

Could be interesting to have the backtrace as well like:

{
  "errors": [
    {
      "message": "Cannot return null for non-nullable field Employee.status",
      "object": {
        "wrapping_type": "EmployeeType",
        "type": "Employee",
        "gid": "..."
      },
      "location": ["/app/types/employee_type.rb", 15]
    }
  ]
}

Probably the location is not trackable in all scenarios like when the object is a Hash, but we could have something designed for the other kinds where the field is not implemented on the type:

{
  "errors": [
    {
      "message": "Cannot return null for non-nullable field Employee.status",
      "object": {
        "wrapping_type": "EmployeeType",
        "type": "Hash",
        "value": { "id": "...", "status": "" }
      },
      "location": null
    }
  ]
}

Additional context

While this extremely useful for debugging, it might leak important data if misused and rendered to the user.

What I'm thinking of is that those information would be added to data.query.static_errors and data.context.errors or conditionally rendered if the Rails.env is development or test.

Alternatively, it could be a feature flag or a use GraphQL::DetailedStaticErrors unless Rails.env.production?.

[rmosolgo]

Hey @sobrinho, we can probably improve this along those lines, or at least support a setup like you're suggesting. Here's where those errors are created:

graphql-ruby/lib/graphql/execution/interpreter/runtime.rb

Lines 475 to 477 in a5451ab

	# This block is called if `result_name` is not dead. (Maybe a previous invalid nil caused it be marked dead.)
	err = parent_type::InvalidNullError.new(parent_type, field, value)
	schema.type_error(err, context)

A couple options right off the bat:

• Implement def self.type_error(err, ctx) to identify InvalidNullErrors and handle them by raising a GraphQL::ExecutionError with the extra context you want. (You could create a subclass of GraphQL::ExecutionError and implement to_h to include whatever metadata you need.) You could use ctx[:current_path] to get the runtime path.
• Improve InvalidNullError.new(...) to also receive the parent object (eg the Employee in your example) so that you can produce better debugging outputs. (You could get the parent object in that code with selection_result.graphql_application_value.)

I'll try to take a look at these soon but I thought I'd go ahead and mention them in case you want to give either one a try.

[swalkinshaw]

Coincidentally this is related to #5239

[ravangen]

As noted in #5239, there is some nuance, but sounds like InvalidNullError could behave more like ExecutionError. Might be able to accept ast_node and path here such that the information is available for to_h as part of the output.