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

Weird error when running mutations with invalid inputs #69

Closed
adamors opened this issue Dec 10, 2021 · 6 comments
Closed

Weird error when running mutations with invalid inputs #69

adamors opened this issue Dec 10, 2021 · 6 comments

Comments

@adamors
Copy link

adamors commented Dec 10, 2021

Hey, I've encountered a bug when running mutations and providing an invalid input.

Given the following schema

type Query {
  hello: String
}

input AddFavoriteFoodInput {
  id: ID!
  comment: String!
  foodNames: [String!]!
}

type Mutation {
  addFavoriteFood(input: AddFavoriteFoodInput!): Boolean!
}

if you run the mutation

mutation AddFavoriteFood($input: AddFavoriteFoodInput!) {
  addFavoriteFood(input: $input)
}

with variables

{
  "input": {
    "id": "123",
    "comment": "Hello",
    "foodNames": [
      "Pizza",
      "Apple",
       null,
      "Pineapple"
    ]
  }
}

the expected error message is:

"Variable \"$input\" got invalid value null at \"input.foodNames[2]\";
Expected non-nullable type \"String!\" not to be null."

However with GraphQL Complexity enabled, this turns into

"Argument \"input\" of required type \"AddFavoriteFoodInput!\" was provided the
variable \"$input\" which was not provided a runtime value."

Here's a repo reproducing the issue: https://github.com/adamors/graphql-complexity-bug

@ahrbil
Copy link

ahrbil commented Dec 21, 2021

Hello @adamors, I'm getting the same error as well.
have you found any workarounds?
thank you.

@adamors
Copy link
Author

adamors commented Dec 21, 2021

@ahrbil not really, currently any code that calls the graphql complexity methods is wrapped in a try/catch and the error is discarded.

@ivome
Copy link
Collaborator

ivome commented Jan 21, 2022

This library does not implement query and input validation to not duplicate the functionality and increase execution costs. So the error messages can vary. One option would be to validate the query and input arguments using the functions from the graphql-js library before running the complexity calculation.

@jjm340
Copy link

jjm340 commented Feb 3, 2022

I'm actually seeing this when valid input is given. Feels like a bug in the library itself.

Taking this back - any error is obscured by this cryptic error when using this library. In development it'd be nice to have the actual errors sent to the client.

@jjm340
Copy link

jjm340 commented Feb 3, 2022

This library does not implement query and input validation to not duplicate the functionality and increase execution costs. So the error messages can vary. One option would be to validate the query and input arguments using the functions from the graphql-js library before running the complexity calculation.

@ivome I don't understand your work-around - can you elaborate?

Vince-Smith pushed a commit to Vince-Smith/graphql-query-complexity that referenced this issue Jul 15, 2022
Currently unused variables are ignored for the purposes of calculating complexity.
However, because graphql-js's `getVariableValues` method returns either a map of coersed
values, OR an array of errors, if directives are present in conjuection with
unused variables then strange errors will be raised.

In particular errors will state that directive variables were not present
when they may well have been.

This change adds fallback measures to ensure that complexity will stil be
calculated if unused variables are present alonside directives.

Fixes slicknode#69
Vince-Smith pushed a commit to Vince-Smith/graphql-query-complexity that referenced this issue Jul 15, 2022
Currently unused variables are ignored for the purposes of calculating complexity.
However, because graphql-js's `getVariableValues` method returns either a map of coersed
values, OR an array of errors, if directives are present in conjuection with
unused variables then strange errors will be raised.

In particular errors will state that directive variables were not present
when they may well have been.

This change adds fallback measures to ensure that complexity will stil be
calculated if unused variables are present alonside directives.

Fixes slicknode#69
@ivome
Copy link
Collaborator

ivome commented Aug 27, 2022

This should be fixed by the changes from #78

@ivome ivome closed this as completed Aug 27, 2022
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