Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nulls in required fields should cause errors and bubble #298

Merged
merged 1 commit into from
Aug 22, 2018

Conversation

vektah
Copy link
Collaborator

@vektah vektah commented Aug 21, 2018

Fixes #275

From the spec:

If an error is thrown while resolving a field, it should be treated as though the field returned null, and an error must be added to the "errors" list in the response.

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.

If the field returns null because of an error which has already been added to the "errors" list in the response, the "errors" list must not be further affected. That is, only one error should be added to the errors list per field.

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.

If a List type wraps a Non-Null type, and one of the elements of that list resolves to null, then the entire list must resolve to null. If the List type is also wrapped in a Non-Null, the field error continues to propagate upwards.

If all fields from the root of the request to the source of the field error return Non-Null types, then the "data" entry in the response should be null.

@@ -87,6 +88,7 @@ func (n NamedTypes) getType(t *ast.Type) *Type {
res := &Type{
NamedType: n[t.NamedType],
Modifiers: modifiers,
ASTType: orig,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't great, I think there is a bigger refactor that needs to happen to align the types used by codegen and the types in the ast.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 I guess this can't just be a boolean since you need to recurse to Elem.

@vektah vektah merged commit de75064 into master Aug 22, 2018
@vektah vektah deleted the handle-response-nulls branch August 22, 2018 03:47
cgxxv pushed a commit to cgxxv/gqlgen that referenced this pull request Mar 25, 2022
Nulls in required fields should cause errors and bubble
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Response returns null on a non-nullable type
2 participants