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

There can be only one variable named \"$hasura_json_var_1\ #7170

Closed
jemink opened this issue Jul 2, 2021 · 10 comments
Closed

There can be only one variable named \"$hasura_json_var_1\ #7170

jemink opened this issue Jul 2, 2021 · 10 comments
Assignees
Labels
k/bug Something isn't working p/urgent Immediate action required

Comments

@jemink
Copy link

jemink commented Jul 2, 2021

I upgraded my hasura on below version
hasura/graphql-engine:v2.0.0-beta.2.cli-migrations-v3

And run below query

mutation updateLocation(
  $id: Int!
  $employees: [EmployeeType]
  $badges: [BadgeType]
) {
  updateLocation(
    id: $id
    employees: $employees
    badges: $badges
  ) {
    affected_row
  }
}

Variable:-

{
"badges": [],
"employees": [],
"id": 8157
}

Then I am getting below error

{
  "data": null,
  "errors": [
    {
      "originalError": {},
      "name": "GraphQLError",
      "positions": [
        96,
        149
      ],
      "source": {
        "body": [
          "mutation ($hasura_json_var_1: [EmployeeType],$hasura_json_var_1: [BadgeType],$id: Int!) { updateLocation(badges: $hasura_json_var_1,id: $id, employees: $hasura_json_var_1) { affected_row  } }"
        ],
        "locationOffset": {
          "line": 1,
          "column": 1
        },
        "name": "GraphQL request"
      },
      "message": "There can be only one variable named \"$hasura_json_var_1\".",
      "nodes": [
        {
          "kind": "Name",
          "value": "hasura_json_var_1",
          "loc": {
            "start": 96,
            "end": 113
          }
        },
        {
          "kind": "Name",
          "value": "hasura_json_var_1",
          "loc": {
            "start": 149,
            "end": 166
          }
        }
      ],
      "stack": [
        "GraphQLError: There can be only one variable named \"$hasura_json_var_1\".",
        "    at Object.VariableDefinition (/usr/app/node_modules/graphql/validation/rules/UniqueVariableNamesRule.js:25:29)",
        "    at Object.enter (/usr/app/node_modules/graphql/language/visitor.js:323:29)",
        "    at Object.enter (/usr/app/node_modules/graphql/utilities/TypeInfo.js:370:25)",
        "    at visit (/usr/app/node_modules/graphql/language/visitor.js:243:26)",
        "    at validate (/usr/app/node_modules/graphql/validation/validate.js:69:24)",
        "    at Object.validateDocument (/usr/app/node_modules/graphql-helix/dist/process-request.js:21:30)",
        "    at /usr/app/node_modules/graphql-helix/dist/process-request.js:58:21",
        "    at Object.processRequest (/usr/app/node_modules/graphql-helix/dist/process-request.js:220:7)",
        "    at /usr/app/node_modules/@graphql-mesh/cli/bin.js:426:47",
        "    at processTicksAndRejections (node:internal/process/task_queues:96:5)"
      ],
      "locations": [
        {
          "line": 1,
          "column": 97
        },
        {
          "line": 1,
          "column": 150
        }
      ]
    },
    {
      "originalError": {},
      "name": "GraphQLError",
      "positions": [
        148,
        817
      ],
      "source": {
        "body": [
          "mutation ($hasura_json_var_1: [EmployeeType],$hasura_json_var_1: [BadgeType],$id: Int!) { updateLocation(badges: $hasura_json_var_1,id: $id,employees: $hasura_json_var_1) { affected_row  } }"
        ],
        "locationOffset": {
          "line": 1,
          "column": 1
        },
        "name": "GraphQL request"
      },
      "message": "Variable \"$hasura_json_var_1\" of type \"[BadgeType]\" used in position expecting type \"[EmployeeType]\".",
      "nodes": [
        {
          "directives": [],
          "kind": "VariableDefinition",
          "variable": {
            "kind": "Variable",
            "name": {
              "kind": "Name",
              "value": "hasura_json_var_1",
              "loc": {
                "start": 149,
                "end": 166
              }
            },
            "loc": {
              "start": 148,
              "end": 166
            }
          },
          "loc": {
            "start": 148,
            "end": 179
          },
          "type": {
            "kind": "ListType",
            "loc": {
              "start": 168,
              "end": 179
            },
            "type": {
              "kind": "NamedType",
              "name": {
                "kind": "Name",
                "value": "BadgeType",
                "loc": {
                  "start": 169,
                  "end": 178
                }
              },
              "loc": {
                "start": 169,
                "end": 178
              }
            }
          }
        },
        {
          "kind": "Variable",
          "name": {
            "kind": "Name",
            "value": "hasura_json_var_1",
            "loc": {
              "start": 818,
              "end": 835
            }
          },
          "loc": {
            "start": 817,
            "end": 835
          }
        }
      ],
      "stack": [
        "GraphQLError: Variable \"$hasura_json_var_1\" of type \"[BadgeType]\" used in position expecting type \"[EmployeeType]\".",
        "    at Object.leave (/usr/app/node_modules/graphql/validation/rules/VariablesInAllowedPositionRule.js:55:35)",
        "    at Object.leave (/usr/app/node_modules/graphql/language/visitor.js:344:29)",
        "    at Object.leave (/usr/app/node_modules/graphql/utilities/TypeInfo.js:390:21)",
        "    at visit (/usr/app/node_modules/graphql/language/visitor.js:243:26)",
        "    at validate (/usr/app/node_modules/graphql/validation/validate.js:69:24)",
        "    at Object.validateDocument (/usr/app/node_modules/graphql-helix/dist/process-request.js:21:30)",
        "    at /usr/app/node_modules/graphql-helix/dist/process-request.js:58:21",
        "    at Object.processRequest (/usr/app/node_modules/graphql-helix/dist/process-request.js:220:7)",
        "    at /usr/app/node_modules/@graphql-mesh/cli/bin.js:426:47",
        "    at processTicksAndRejections (node:internal/process/task_queues:96:5)"
      ],
      "locations": [
        {
          "line": 1,
          "column": 149
        },
        {
          "line": 1,
          "column": 818
        }
      ]
    }
  ]
}

But it is working fine with alpha 10

hasura/graphql-engine:v2.0.0-alpha.10.cli-migrations-v3
@btodorce
Copy link

This is causing us a massive issue, since we have multiple mutations that previously worked that are suddenly broken and we "forced" to use the alpha v10 instead of the latest version.

@jemink
Copy link
Author

jemink commented Jul 15, 2021

EmployeeType

type EmployeeType {
id: Int
name: String
}

BadgeType

type BadgeType {
icon: String
name: String
}

Also I connected remote schema inside my hasura and created custom resolver. It is working fine on remote schema url

@0x777 0x777 added k/bug Something isn't working p/high candidate for being included in the upcoming sprint p/urgent Immediate action required and removed p/high candidate for being included in the upcoming sprint labels Jul 15, 2021
@abooij
Copy link
Contributor

abooij commented Jul 15, 2021

We've had some internal discussions about this today. There seems to be an issue with the way the GraphQL queries that are sent to remotes GraphQL servers are built. In particular, there is a phase that tries to come up with fresh GraphQL variables, so that presets for certain object fields can be incorporated. This is essentially what resolveRemoteVariable here is concerned with.

The philosophy of that RemoteJSONValue case of that code (starting on line 185) is that sometimes, we're kind of forced to come up with a new variable to represent the JSON value that we'd truly like to pass to the remote GraphQL server, and so it generates a fresh name for it, so that we can truly pass that value as JSON, without having to inline anything into the GraphQL query (which is not possible in general). The so-called variable cache keeps track of which JSON value is passed under which variable name.

In this case, the value [] gets passed twice. The code believes it can simply reuse the same variable name, since, from a JSON point of view, it's the same value. This is not quite right, however, as from a GraphQL point of view they are two values of different types - in this case [EmployeeBadge] and [BadgeType], and so we need to pass them separately.


The above explains why the remote GraphQL server chokes on the query it receives. However, for me there is one additional point of confusion: why did Hasura declare the variable hasura_json_var_1 twice? From the log:

          "mutation ($hasura_json_var_1: [EmployeeType],$hasura_json_var_1: [BadgeType],$id: Int!) { updateLocation(badges: $hasura_json_var_1,id: $id, employees: $hasura_json_var_1) { affected_row  } }"

I believe the answer can be found here. At that point, all we've generated for the GraphQL query we're building, is an (augmented) GraphQL selection set. That particular method generates the list of variable declarations. The particular line linked extracts the desired list of variable declarations from it as follows:

  1. it (intuitively speaking*) syntactically browses through that selection set, saving all (augmented) variables being used
  2. it deduplicates those usages into a HashSet,
  3. it converts that set into a list,
  4. and finally, for each element of the list, it converts the (augmented) variable usage into a variable declaration.

(* The reason this is at best an intuition is that in reality such things are represented in graphql-engine by abstract syntax trees.)

The reason that hasura_json_var_1 isn't being deduplicated by the HashSet is that the augmented variables indeed contain type information (in this case something representing [EmployeeType] resp. [BadgeType]) and are hence distinct.


@0x777 suggested that the so-called variable cache should index based on not just JSON values, but also on GraphQL types. I think that's necessary, but I'm not yet fully convinced it's sufficient. The augmented variable usages have a third identifying element, namely a VariableInfo, which indicates whether the variable is required or optional. I believe that, for the purpose of generating correct names, this is not relevant, since at this point if a variable wasn't assigned a value yet then we'd have no way to generate a valid GraphQL query. Additionally, in the current version of the code, when the RemoteJSONValue case of resolveRemoteVariable fires, it always marks the variable as VIRequired.

But the duplication problem above could potentially apply, especially if at some point in the future we would for some reason also want to generate optional variables here. So to avoid declaring a variable twice, I think we should also keep track of whether the variable is required or not.

Anyway, the last paragraph is not super important to get right, I think. If the so-called variable cache simply indexes both by value and by type, this issue should be avoided.

@nicuveo @jkachmar

@jemink
Copy link
Author

jemink commented Jul 16, 2021

I can't able to access this page https://github.com/hasura/graphql-engine-mono/blob/6417ffc9b24a1a2c8437f77cd8c3c7e742f9c5fc/server/src-lib/Hasura/GraphQL/Execute/Remote.hs#L78
Also my input is optional so If user doesn't select anything then I am passing empty array that is a expected scenarios

@abooij
Copy link
Contributor

abooij commented Jul 16, 2021

@jemink Thanks, I've updated the links, they should work for you now.

@abooij
Copy link
Contributor

abooij commented Jul 16, 2021

@jemink Ah, and I realize it perhaps wasn't clear from my phrasing, but indeed this is a bug, and we're fixing it. My message above was simply analyzing what's going on internally inside graphql-engine.

@jemink
Copy link
Author

jemink commented Jul 28, 2021

Hello @abooij
Is there any update regarding this issue ?

@abooij
Copy link
Contributor

abooij commented Jul 28, 2021

@jemink A fix is being developed and will be released ASAP.

@jemink
Copy link
Author

jemink commented Aug 2, 2021

Thank you. Please let me know when you released the new version

@jkachmar jkachmar self-assigned this Aug 4, 2021
@jkachmar
Copy link
Contributor

jkachmar commented Aug 4, 2021

Closed via c322af9.

@jkachmar jkachmar closed this as completed Aug 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
k/bug Something isn't working p/urgent Immediate action required
Projects
None yet
Development

No branches or pull requests

5 participants