rmosolgo / graphql-ruby

Ruby implementation of GraphQL
http://graphql-ruby.org
MIT License
5.37k stars 1.38k forks source link

Incorrect `path` attribute on query validation error #4711

Open utay opened 9 months ago

utay commented 9 months ago

Describe the bug

With this query:

query TestQuery($slug: String! = "utay") {
  user(slug: $slug) {
    id
  }
}

This error is returned:

{
  "errors": [
    {
      "message": "Non-null variable $slug can't have a default value",
      "locations": [
        {
          "line": 1,
          "column": 17
        }
      ],
      "path": [
        "query TestQuery"
      ],
      "extensions": {
        "code": "defaultValueInvalidOnNonNullVariable",
        "variableName": "slug"
      }
    }
  ]
}

The issue here is path; it's equal to "query TestQuery" and I think it is not exactly compliant with the graphql spec which expects path to be an array of fields. It is an issue for us because we use a federation router in front of the graphql server and path is invalid for the router in this case.

Versions

graphql version: 2.1 rails version: 7.0.7

Expected behavior

Maybe path shouldn't be set when there's a validation error in the query arguments as some other graphql implementations seem to do that.

(It's a separate topic but some implementations accept default values for non-null variables and it's not clear to me what is the right behavior per the spec.)

rmosolgo commented 7 months ago

Hey, sorry I didn't get back to you on this earlier! I agree that the path on this error should match the spec.

Also, looking back at the spec, I can see that this query doesn't violate it. It's allowed within the given grammar and not expressly prohibited elsewhere.

I'll keep this open until I've checked the paths and removed this error from the project.

rmosolgo commented 1 month ago

šŸ‘‹ Just one update on this, GraphQL-Ruby doesn't add this error to queries like this anymore (since https://github.com/rmosolgo/graphql-ruby/pull/5030). I'm hoping to clean up these errors soon.