Spec compliance: Errors do not propagate to the closest nullable field

[felixfbecker]

How to reproduce:

• Define a schema with a nullable field, and a non-nullable sub-field
• Have the schema for the non-nullable field return an error

Expected:

The nullable field should be null and the error should point to that field. Specifically, the non-nullable field should not appear in the response because it must never be null - the nullable parent field must be null instead.

From the spec:

Since Non-Null type fields cannot be null, field errors are propagated to be handled by the parent field. If the parent field may be null then it resolves to null, otherwise if it is a Non-Null type, the field error is further propagated to it’s parent field.

http://facebook.github.io/graphql/draft/#sec-Errors-and-Non-Nullability

Actual:

Both the nullable and the non-nullable field will be present, with the non-nullable field being null and the error in the errors array pointing to that error. This is not spec-compliant.

---

Version: b46637030579abd312c5eea21d36845b6e9e7ca4 (latest)

[tonyghita]

Thanks for the issue report! We definitely want this functionality implemented. I've added a TODO item to track this work on the roadmap.

I think the reason why this is not yet implemented is because the response is written as field execution finishes, and this functionality would require re-writing the response in the current implementation.

It may be easier to reason about this functionality if we deferred writing the response until execution has finished 🤔

[gracenoah]

I'm interested in this feature. I think it wouldn't be as hard as it seems because there's already some reasonable buffer management going on.

The general rule that needs to be enforced is that if the current field has any non-nullable sub-fields, it can't flush its output until after its non-nullable sub-fields have succeeded. To avoid premature optimization, I plan to make every field with non-nullable sub-fields wait for all sub-fields to finish, buffer the output, and then decide to: 1. flush it on success, 2. flush a null if it's nullable or 3. do nothing and expect the parent to flush a null eventually

The parent will inspect Request.Errs after executing the child fields to detect if any non-nullable children failed and react accordingly.

Does this make sense? Any objections before I start?

[felixfbecker]

Was this fixed in https://github.com/graph-gophers/graphql-go/pull/296? @tinnywang

[tinnywang]

@felixfbecker yes, #296 addresses this issue.