Use fresh path and ast_node values for errors of lazy evaluated queries

[juanrgon]

Closes #5020

I'm attempting to fix an issue with error paths and locations being duplicated with DataLoaders and lazy evaluation

[rmosolgo]

Hey, thanks for sharing this fix -- I'd love to include it! Would you mind including the script from #5020 in the test suite? You could add it to execution_error_spec.rb, for example.

[rmosolgo]

Thanks for your work on this! In reviewing the change, I realized that the source of the problem was Dataloader::Source's cache of fetch(...)'d values. Whenever that method returns a value for a key, it's used again for later requests for the same key. So in this case, it was returning the exact same GraphQL::ExecutionError for the requested key ("a").

I'm hesitant to change runtime.rb when I can help it -- that code affects a lot of behavior! So I pushed a change that handles it inside Dataloader::Source instead, by dup'ing any error from fetch.

[juanrgon]

@rmosolgo is it possible to get this fix back-ported to 2.1 and 2.2 in addition to 2.3? I can do the heavy-lifting for this, if possible, but will just need a little guidance

[rmosolgo]

Yes, no problem. Basically my approach is to:

• pull the 2.1.x branch locally
• create new branch, eg, 2.1-fix-dataloader-error-path
• cherry-pick each of these commits to that branch (or, somehow port the changes on to that branc)
• push the branch and open a PR onto the 2.1.x branch

Let me know if you have any trouble -- I'd be happy to help with the backports. Once the PR is ready, I can merge and release to rubygems 👍

[juanrgon]

Thanks for the quick response @rmosolgo! I opened two PRs with patches:

• 2.1 patch
• 2.2 patch