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

Spec clarification: errors in list elements #259

Closed
dllx opened this issue Jan 25, 2017 · 7 comments
Closed

Spec clarification: errors in list elements #259

dllx opened this issue Jan 25, 2017 · 7 comments

Comments

@dllx
Copy link

dllx commented Jan 25, 2017

I think the relevant parts of the spec are "6.4.4 Errors and Non-Nullability" and "3.1.7 Lists, result coercion", which in my opinion do not clarify the following:

If a field of type [String] is queried, and an error occurs during determining the value of the second item, would the result of the field be null or would it be ["elA", null, "elC"]?

Section 3.1.7 says

Each item in the list must be the result of a result coercion of the item type. If a reasonable coercion is not possible they must raise a field error.

The question is the "granularity" of the field error and the returned null value: is it constrained to the field as a whole or can it apply to list items?

In the second case, would there be one "field error" listing all single item errors or would there be multiple field errors? Also, in this case the null propagation of section 6.4.4 should be applied: in a [String!] field, the whole list should be set to null. This would then extend to nested lists: [[String!]] should return a null item in the outer list.

An argument against the first case is:
Returning null for the whole field would lose the items that could be determined.

Could please the specification be extended to include a clarification on what GraphQL is expected to do in this case?

@tonyghita
Copy link

tonyghita commented Mar 21, 2017

If a field of type [String] is queried, and an error occurs during determining the value of the second item, would the result of the field be null or would it be ["elA", null, "elC"]?

The way I've understood it, the result would contain the list items that did resolve, and an error for the ones that didn't.

{
  "data": { "listField": ["elA", "elC"] },
   "errors": [{"message": "unable to resolve listField[1]"}]
}

Or something to that effect. @leebyron is it intentional that the specification is vague on this point?
@dllx I'm curious to hear how you've handled this case.

@stubailo
Copy link
Contributor

I think it depends on if the items of the list are non null or not.

@tonyghita
Copy link

Right. I think it's ambiguous in the first case where we have a field type that has null-able elements. For example [String].

@dllx
Copy link
Author

dllx commented Mar 22, 2017

@tonyghita I allowed the list to contain null values and took the freedom to set the erroneous fields to null and the successful fields to their normal values. I also included enhanced error information to indicate the error path (see last section in this post).

I did not think of simply omitting the erroneous fields from the list (and the spec didn't give me a hint this might be intended).

In the non-nullable case, with the solution proposed in your example, it is even possible to infer the original indices of the items in the result list if desired. It can be done by examining the path in the errors like you wrote in your example, but would be a bit awkward. Anyway, it's better than returning the complete list as null.

I even patched graphql-java we use as server to produce error structures that contain the path of the error in the result data structure like you did in your example error messages above. (By the way, this is a very useful addition to GraphQL errors as it allows machine interpretation of the error, and I'd recommend adding it to the GraphQL spec---but that would be another issue. Has there been some work done in this direction?)

@tonyghita
Copy link

I'd recommend adding it to the GraphQL spec---but that would be another issue. Has there been some work done in this direction?

@dllx, yeah there is an RFC for this exact specification on #230

The more I think about it, the less omitting the result makes sense to me. I think I'll follow your lead and return a null value with a corresponding error in the case that a nullable element's resolution errors.

@dllx
Copy link
Author

dllx commented Mar 23, 2017

#230 is very nice and useful, thanks for the pointer!

I think this discussion shows that spec clarification is necessary, preferrably with examples for both the nullable and the non-nullable list items case.

@leebyron
Copy link
Collaborator

leebyron commented May 1, 2018

Added copy for this in #436 about errors applying to each item in a list

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

No branches or pull requests

4 participants