Return GraphQL compliant errors #3728
Conversation
There was a problem hiding this comment.
LGTM
Regarding the 1.1 release, my understanding is that master would be released as 1.1 at some point. I'll have to confirm with @danielmai about that. Semver says that breaking API changes require a major update but I guess we are far from v2.0.
Would be nice to populate locations as well. Looks like its mandatory as well. Do we have that information yet?
|
Doesn't semver say that breaking changes go into minor releases, so from 1.0.16->1.1.0 is the right spot? i.e. just not in patch releases. I think there's been some changes to Dgraph query parsing to report error locations. Would make for nicer error messages to have that info. |
|
P.S. GraphQL layer has the locations info and already reports that - e.g. on type errors or parse errors etc. It does it atm using a different struct, will be using this, once this merges. |
There was a problem hiding this comment.
Reviewed 2 of 2 files at r1.
Reviewable status:complete! all files reviewed, all discussions resolved
(cherry picked from commit efb9387)
Add GraphQL spec compliant errors to x package. That means alpha http endpoint should return GraphQL compliant errors.
Change from current structure is where
codesits. With this PR it moves toextensions. Should we call this breaking and move into 1.1 release?This changes means Alpha and GraphQL layer can use the same sorts of errors and responses and moves Dgraph closer to GraphQL compliant.
Maybe we should consider using GQL Locations structure for reporting locations of Dgraph query parsing errors as well?
This change is