Fix: Incorrect/incomplete error message

[joaoGabriel55]

Fix issue #5101

Actual error message:

Expected a number, but it was malformed ("-")

Expected error message:

Argument 'arg' on Field 'hello' has an invalid value (-foo). Expected type 'MyEnum!'.

[rmosolgo]

Hey, thanks for taking a crack at this! I agree that it would be good to improve this error message. I have put it off because I think it will be very tricky do it in a robust way 😅

I pushed a few more tests which should pass before we merge this PR -- different cases where -foo may appear and should be handled gracefully. Want to see if you can make them pass, too?

Personally, I think re-parsing GraphQL with a regular expression won't work ... there's a ton of variety that can appear in a valid GraphQL string. I don't have a great idea for an alternative (which is I why I initially chose the current implementation).

One possibility might be to change the control flow. Instead of raising from Lexer#advance, we could have that method return a different token name, for example, :INVALID_NUMBER. Then, we could update the parser to handle that kind of token by raising an error. The parser probably has more information about the current parsing context and could therefore produce a better error message. I haven't tried that approach, but I would investigate it if I was trying to improve this!

Thanks again and let me know if I can help with anything else along the way.

[joaoGabriel55]

Hello @rmosolgo
I think the main goal would be to find a way to reuse this logic on our "new" error validation. The main problem is to convert the Query String passed on the GraphQL.parse method to an AST. I tried a lot of solutions using Regex but any of them covered complex cases like Query lists. Any ideas about it?

lib/graphql/static_validation/rules/argument_literals_are_compatible.rb:L52

[rmosolgo]

Hey, sorry I didn't respond sooner. I don't really have a vision for how to build an AST from an invalid query string. There are error-tolerant parsers out there, but I haven't studied them and don't know how to improve GraphQL-Ruby's parser to become error-tolerant.

For now, I added some regex-foo that attempts to grab the part after the - and include it in the error message. If you'd like to propose further improvements to this code, please feel free to open another PR! Thanks for getting this started.