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

What if there's a "field error" during serial execution? #277

Closed
rmosolgo opened this issue Mar 6, 2017 · 6 comments
Closed

What if there's a "field error" during serial execution? #277

rmosolgo opened this issue Mar 6, 2017 · 6 comments

Comments

@rmosolgo
Copy link

rmosolgo commented Mar 6, 2017

Let's say you have a mutation type like:

type Mutation {
  push(val: Int!): Int!
}

And you send a query like:

mutation {
  one: push(val: 1)
  thirteen: push(val: 13)
  two: push(val: 2)
}

And, given the unluckiness of 13, push(val: 13) causes an internal error, resulting a null value for that field. What should happen? Specifically:

@stubailo
Copy link
Contributor

stubailo commented Mar 7, 2017

I think not executing push(val: 2) would be a really valuable property - that would make it meaningful to include multiple mutations in one document, rather than sending a bunch of separate mutations.

@stubailo
Copy link
Contributor

stubailo commented Mar 7, 2017

On the other hand, if #252 (nested serial fields) was introduced, you could specify dependencies via:

mutation {
  one: push(val: 1) {
    then { # proxy for Mutation
      thirteen: push(val: 13)
    }
  }
  two: push(val: 2)
}

Actually; the above might already be spec compliant. HMMM!!!

@OlegIlyenko
Copy link
Contributor

OlegIlyenko commented Mar 7, 2017

I also always had an assumption that push(val: 2) should not be executed because of the error handling rules. Now that you pointed this out, it is indeed unclear how this should be handled according to the spec.

I would agree with @stubailo on this one. It is quite valuable property to have. If you would like to give user a choice, you can provide two mutation field variations: nullable and not-null.

@rmosolgo
Copy link
Author

rmosolgo commented Mar 7, 2017

Thanks for shedding some light on this one! I've merged that behavior into GraphQL Ruby. Do you think it should be clarified in the spec, or is the spec sufficient already?

@OlegIlyenko
Copy link
Contributor

yes, I think it needs to be explicitly stated un the spec. From what I can tell, It's not clear how execution should behave in this situation.

Even if this behavior can be inferred from other parts of the spec, I think it would be beneficial if this aspect is explicitly described.

@leebyron
Copy link
Collaborator

leebyron commented May 1, 2018

Should the result be { data: null, errors: [ /* 1 error */] }? (Judging by "Errors and Non-Nullability", yes.)

This is the most important question, since the result is what's observable to the request, and this is absolutely correct.

Should push(val: 2) be executed?

This has been intentionally ambiguous since the requestor cannot observe the results, however I think it's totally acceptable to not execute it in the face of errors. I've added some additional text in #438 which specifically points out that execution of sibling fields may be cancelled in the case that an error occurs on a non-null field.

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