Conversation
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A custom resolver can successfully return
Value::Nullfor a field declared as non-null. Juniper propagates that null through the parent selection, but it does not record the field error required by GraphQL when no lower-level error caused the null.Example
Given this schema, where the custom
criticalresolver returnsValue::Null:and this query:
Juniper currently bubbles the null to the operation root without recording why:
{"data":null}The response must also contain the corresponding field error:
{ "data": null, "errors": [ { "message": "Cannot return null for non-nullable field Query.critical.", "path": ["critical"] } ] }The focused regression is
direct_null_from_non_null_field_is_reportedinjuniper/src/executor_tests/executor.rs. It runs the operation through bothexecute_syncandexecute, asserting the root null and error path in both modes.Root Cause
The sync and async object executors treat
Ok(Value::Null)at a non-null field only as a propagation signal. They stop the current selection without adding a field error, assuming that some lower-level completion already recorded one.Proposed Fix
Add an internal executor check for an existing error at or below the current field path. If a non-null field produces
Ok(Value::Null)without such an error, record one non-null violation before preserving the existing bubbling behavior.Related Reports
No exact duplicate was found. Issue #84 and issue #1287, fixed by PR #1318, concern propagating an error that already exists. This bug occurs one step earlier: resolution succeeds with
Ok(Value::Null), so Juniper has no error to propagate unless the executor creates the required non-null field error.Open PR #1256 refactors the same async execution branch but preserves the
Ok(Value::Null) if is_non_null => Nonebehavior. It overlaps at the code level but does not fix this issue.