Ignore Missing Ignore/Skip Directives

[Vince-Smith]

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, whenever directives are present in conjunction 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.

Similar to #69 (falling back to use the non-coersed variables should also fix this issue in most cases).

[Vince-Smith]

@ivome just fyi, it seems like the CI failures are unrelated to the changes in this PR. Not sure what needs done to resolve that 😄

[mengledowl]

+1 - we ran into an issue which would be resolved by having this merged. Specifically, if you pass in a bad value for an enum in a query variable, the error which states that the value was bad gets swallowed and replaced by an error stating that the argument was not passed in at all.

[ivome]

Thanks for the PR and sorry for the slow response. I wanted to take some time to take a closer look at this. I am not sure it was good idea to swallow the errors that are caused by invalid input values. We started doing that a while ago to prevent some other issues that popped up, but that lead to some misleading error messages, like mentioned by @mengledowl or reported in #69.

I implemented in change in #78 that reports those errors instead of swallowing them and running the complexity estimation on a query that is not valid to begin with. I hope this solves the issue or at least provides a meaningful error message?

I don't think the queries in the tests of this PR here are valid, so we should raise an error instead of ignoring the error and doing some calculation which might have unexpected behavior.

[Vince-Smith]

Thanks for the PR and sorry for the slow response. I wanted to take some time to take a closer look at this. I am not sure it was good idea to swallow the errors that are caused by invalid input values. We started doing that a while ago to prevent some other issues that popped up, but that lead to some misleading error messages, like mentioned by @mengledowl or reported in #69.

I implemented in change in #78 that reports those errors instead of swallowing them and running the complexity estimation on a query that is not valid to begin with. I hope this solves the issue or at least provides a meaningful error message?

I don't think the queries in the tests of this PR here are valid, so we should raise an error instead of ignoring the error and doing some calculation which might have unexpected behavior.

Thanks @ivome for the feedback! Yeah I wasn't sure if there was a reason for swallowing the error so didn't want to update it to raise one.

At a glance, what you're proposing seems like it should work. So this PR can just be closed 😄 in favor of #78

[ivome]

I just published #78 with v0.12.0.