Scalar ParseValue not validating variable inputs about graphql HOT 2 CLOSED

Hi @pyros2097

I just got a chance to look at the code you posted and ran it through both graphql-go and graphql-js to verify the output, and you are right, at first glance, the behaviour is suspect.

Hang on tight cos this issue has multiple layers to it:

1.validator not implemented yet in graphql-go

Validation of arg inputs (for e.g. is the input matches the expected input type, etc) are supposed to be done through the validator before it gets passed to the executor.

But currently, validator has not been implemented yet.

2. panic() is not expected to be used in definition of scalars at the moment.

I see that currently you try to throw a panic() if the scalar value is unexpected.

If you study how built-in scalars are defined (both in graphql-go and graphql-js, you would notice that if a scalar value/literal can't be parsed/serialized, it will return nil.

Currently, if you throw a panic in ParseValue/ParseLiteral/Serialize (unlike Resolve in ObjectConfig), it won't be caught by graphql-go.

Side note: according to GraphQL spec, regarding Scalars:

• "The only requirement is that the server must yield values which adhere to the expected Scalar type"
• "raise a field error" if it can't parse the given value.

3. Query string in params2 does not get validated because it did not specify the correct type

The current query in params2 of your example:

      mutation M($input: String!) {
        ObjectCreate(id: $input) {
          id
        }
      }

But ObjectCreate field expected a nullable value of type ID, as specified here:

...
Mutation: graphql.NewObject(graphql.ObjectConfig{
            Name: "Mutation",
            Fields: graphql.FieldConfigMap{
                "ObjectCreate": &graphql.FieldConfig{
                    Args: graphql.FieldConfigArgument{
                        "id": &graphql.ArgumentConfig{
                            Type: ID, // <-- nullable value of type `ID`
                        },
                    },
                },
            },
        }),

Try to update it to:

      mutation M($input: ID) { // <--- Update it to nullable `ID`, as specified in `ObjectCreate` field
        ObjectCreate(id: $input) {
          id
        }
      }

After updating the query, now your ParseValue function will be executed and should throw fits all over ;)

The reason why your original didn't throw error when you specified the wrong type for the input argument is because validator has not been implemented. (see point 1).

If validator was doing its job, it should have an return error similar to this:

Variable "$input" of type "String!" used in position expecting type "ID".

So to address your original question, the reasons why your custom scalar does not seem to validate input are because:

• the query that used variables specified the wrong input type.
• and validator is not there to do its job to let you know in the first place.

This issue highlights at least two things that are incomplete in graphql-go implementation:

• validator, to validate queries
• how to handle errors in user-defined functions (e.g ParseValue, ParseLiteral, Serialize, Resolve etc)
  • Related issues #28 and #44
  • At least Resolve currently allows user to panic() and it would be recovered by executor.
  • Others are left uncaught I believe.
  • One suggestion in #28 is to discourage panic, be go-idiomatic and let user return error.

I'll create issues to highlight these flaws in implementation.

Cheers!

from graphql.

Hi @sogko,
Your'e exactly right. It wasn't validating my type because my query was wrong. Anyway it seems if we pass the input directly like in the first query it validates it correctly and the panic in the ParseLiteral is triggered and returned as a graphql error. Thanks for the reply. I'll be closing this issue.

from graphql.