graphql-js
graphql-js copied to clipboard
Allow separation of variable value coercion from the rest of execution
graphql-js lets you run the entire GraphQL operation with a single graphql function, or break it apart into different steps (parse, validate, execute) if you want a bit more control (for example, handling errors at different stages differently). However, the execute step is actually three steps combined into one:
- Selecting the required operation from the document (
executeis the first step that looks at the explicitly-providedoperationNamefrom the request) - Coercing variable values
- Actual execution
Servers should be able to separate out these three steps and run them individually if they want. Specifically, it's likely that error handling should work differently for errors in the first two steps vs in the third. Apollo Server tries to provide a 400 HTTP status code for errors in the first two steps and a 200 for errors at execution time; this is also what the current GraphQL over HTTP spec suggests via a SHOULD, for response content-type application/graphql+json.
It's not too hard for a server to implement the first step manually, but differentiating between variable coercion errors and field errors is challenging. For example, Apollo Server has this hacky error-matching code.
Ideally, you would be able to choose between running execute and running three resolveOperation, coerceVariableValues, and executeResolvedOperation functions (passing return values from the previous functions to the next ones). This should be fully backwards-compatible since it's just adding a new API.
(I thought there was already an issue for this but I had trouble finding it.)
See #3644 and #3658
these PRs came from different directions to solve this issue but both include different major refactors
#3658 was started to address breaking up execution into different stages and ended up with one type of major refactor
#3644 was started to integrate execute and subscribe but also ended up exporting “helpers” to address above issue.
the basic thing is that in a pipeline that includes subscriptions, you have to find the operation to figure out how to execute, so users of graphql-js may have to peak inside the operation to figure out how to execute, which is another reason to give users greater control over the pipeline.
this may have been mitigated somewhat for setups that use different endpoints or protocols for subscriptions (or perhaps some other factor?)
anyway, thanks for raising a dedicated issue to track this
Also see #3215
(note that you can manage the 400 by checking for lack of data field on the return, but if you want to differentiate "couldn't select operation" from "couldn't coerce variables" it's harder.)
Thanks Yaacov, I knew there had been some recent work here but I wanted something to link in a GraphQL-over-HTTP discussion.
Sure. And thank u again @glasser for creating this dedicated issue to track these efforts. Just a note on #3215 that’s kind of a least change approach just to export the tools surrounding ExecutionContext. We should probably make sure to separate the internal errors array from the rest of the context, which is basically the sum total of my approach to this in #3644.
This issue may now be a great place for you @glasser or @IvanGoncharov or anybody interested to compare the different approaches in the PRs above, or suggest a different approach or just generally chime in on what the best way to move forward e.g. if we should solve this/refactor in stages, etc, etc.