This is a failing test case covering an introspection query erro when using a variable in fragment

[madsflensted]

This PR is a failing test case, (no fix included) that demonstrates an error I came across when sending an introspection query using a variable and using that variable in a fragment.

The background is that I was trying out an Elm GraphQl package together with your tutorial server and getting this error:

$ graphqelm http://localhost:17290
{ errors: 
   [ { message: '{error,#{key =>\n            {input_coercion,<<"Bool">>,\n                             {var,{name,19,<<"includeDeprecated">>}},\n                             not_bool},\n         message =>\n             <<"General uncategorized error: {input_coercion,<<\\"Bool\\">>,\\n                                 {var,{name,19,<<\\"includeDeprecated\\">>}},\\n                                 not_bool}">>,\n         path => [<<"document">>,<<"fields">>,<<"includeDeprecated">>]}}',
       type: 'error' } ],
  status: 400 }

The test data file in this PR includes a cut down version of the query that still exhibits the error.
As far as I have been able to tell it is provoked when using a query variable in an introspection query, and then using this query variable in a fragment. If I try to use the query variable directly in the query part it is ok, and if I use a hard coded parameter in the fragment part it is ok. From the error message it seems that the graphql engine tries to pass the AST of the variable (or variable listed in the fragment?) to the graphql_scalar_bool_coerce.erl functions, instead of the resolved value?

[jlouis]

Hi Mads!

You've hit #107 by accident, which is a thing I've wanted to support, but haven't had the time to attack yet. The problem is that the current type checker and by extension execution doesn't understand how a variable propagates between a query and its fragments.

While the #107 case contains the details, the quick overview is that one has to regard a fragment ... { ... } as a "function" (lambda) of sorts in the type system. It will have some escaping variables; your $includeDeprecated is one of those. Now when you "call" a fragment (by writing a fragment spread in a query or another query) you have to type check the signature of the fragment with the signature in the call site of currently available types in the variable environment. Since we don't do that, we can't safely know when a fragment expansion is valid and when it is not. There is also a point where $includeDeprecated can be a Bool in one fragment and an Int (say) in another fragment. You have to guard against this as well.

The current type checker doesn't account for this. So when it encounters the unknown $includeDeprecated it assumes there is no bound variable and it fails. The change isn't hard, but takes some effort since you have to invert some of the type checking passes to "infer" what the type signature is.

The execution engine is a bit more simple since it will always have a valid context of variables which it can pull from. If the type checker manages to prove that this is a well typed program, then it will always be the case that the variable will be valid and thus we can probably get away with no changes to the execution engine at all.

I don't think the problem is hidden in introspection, since that just follows orders in this case :)

I think we should add this test case, since it demonstrates the problem. We may want to add a skip into common test on the guy for now (since we won't pass it), but it will document that we don't support this particular feature yet in a better way. And once we solve #107 it will be natural to add this test case to the system as a real test case.

[madsflensted]

Hi Jesper.

Thank you for clarifying. I had looked at #107 but I had not understood how exactly these two issues were linked.

I have pushed a test setting that skips this test case for now.

[jlouis]

Perfect! Let us get this merged so we can track it.