From 0d1c2fbc480e130c6bdb430007970d19108d9afd Mon Sep 17 00:00:00 2001
From: Robert Mosolgo <rdmosolgo@gmail.com>
Date: Thu, 4 Feb 2021 19:30:57 -0500
Subject: [PATCH 01/12] Add failing test, start to support args with Dataloader

---
 lib/graphql.rb                               |  2 +-
 lib/graphql/execution/interpreter/runtime.rb |  5 +++
 lib/graphql/query/null_context.rb            |  5 ++-
 spec/graphql/dataloader_spec.rb              | 40 ++++++++++++++++++++
 4 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/lib/graphql.rb b/lib/graphql.rb
index 8725c94246..13723486b4 100644
--- a/lib/graphql.rb
+++ b/lib/graphql.rb
@@ -128,6 +128,7 @@ def match?(pattern)
 require "graphql/filter"
 require "graphql/internal_representation"
 require "graphql/static_validation"
+require "graphql/dataloader"
 require "graphql/introspection"
 
 require "graphql/analysis_error"
@@ -148,7 +149,6 @@ def match?(pattern)
 require "graphql/unauthorized_error"
 require "graphql/unauthorized_field_error"
 require "graphql/load_application_object_failed_error"
-require "graphql/dataloader"
 require "graphql/deprecation"
 
 module GraphQL
diff --git a/lib/graphql/execution/interpreter/runtime.rb b/lib/graphql/execution/interpreter/runtime.rb
index de570d9fd0..e8ff640c7f 100644
--- a/lib/graphql/execution/interpreter/runtime.rb
+++ b/lib/graphql/execution/interpreter/runtime.rb
@@ -198,6 +198,11 @@ def evaluate_selection(path, result_name, field_ast_nodes_or_ast_node, scoped_co
 
           begin
             kwarg_arguments = arguments(object, field_defn, ast_node)
+            # TODO: I think the thing to do here is, for each argument:
+            #  - enqueue a job to coerce _that_ argument and add it to the result set
+            #  - then check the result set: if it's complete, then call a method to continue execution
+            #  - I guess that'd be for each argument _definition_ -- maybe the counter would
+            #    be to check that each _definition_ has finished being loaded.
           rescue GraphQL::ExecutionError, GraphQL::UnauthorizedError => e
             continue_value(next_path, e, owner_type, field_defn, return_type.non_null?, ast_node)
             return
diff --git a/lib/graphql/query/null_context.rb b/lib/graphql/query/null_context.rb
index f2f33ecefa..ec21e1ec46 100644
--- a/lib/graphql/query/null_context.rb
+++ b/lib/graphql/query/null_context.rb
@@ -9,10 +9,11 @@ def visible_field?(t); true; end
         def visible_type?(t); true; end
       end
 
-      attr_reader :schema, :query, :warden
+      attr_reader :schema, :query, :warden, :dataloader
 
       def initialize
         @query = nil
+        @dataloader = GraphQL::Dataloader::NullDataloader.new
         @schema = GraphQL::Schema.new
         @warden = NullWarden.new(
           GraphQL::Filter.new,
@@ -36,7 +37,7 @@ def instance
           @instance = self.new
         end
 
-        def_delegators :instance, :query, :schema, :warden, :interpreter?
+        def_delegators :instance, :query, :schema, :warden, :interpreter?, :dataloader
       end
     end
   end
diff --git a/spec/graphql/dataloader_spec.rb b/spec/graphql/dataloader_spec.rb
index 1b4ee23f95..9e355d569d 100644
--- a/spec/graphql/dataloader_spec.rb
+++ b/spec/graphql/dataloader_spec.rb
@@ -173,6 +173,16 @@ def common_ingredients(recipe_1_id:, recipe_2_id:)
         common_ids = recipe1[:ingredient_ids] & recipe2[:ingredient_ids]
         dataloader.with(DataObject).load_all(common_ids)
       end
+
+      field :common_ingredients_with_load, [Ingredient], null: false do
+        argument :recipe_1_id, ID, required: true, loads: Recipe
+        argument :recipe_2_id, ID, required: true, loads: Recipe
+      end
+
+      def common_ingredients_with_load(recipe_1:, recipe_2:)
+        common_ids = recipe_1[:ingredient_ids] & recipe_2[:ingredient_ids]
+        dataloader.with(DataObject).load_all(common_ids)
+      end
     end
 
     query(Query)
@@ -394,4 +404,34 @@ def database_log
     ]
     assert_equal expected_log, database_log
   end
+
+  it "loads arguments in batches" do
+    query_str = <<-GRAPHQL
+    {
+      commonIngredientsWithLoad(recipe1Id: 5, recipe2Id: 6) {
+        name
+      }
+    }
+    GRAPHQL
+
+    res = FiberSchema.execute(query_str)
+    expected_data = {
+      "commonIngredientsWithLoad" => [
+        {"name"=>"Corn"},
+        {"name"=>"Butter"},
+      ]
+    }
+    assert_equal expected_data, res["data"]
+
+    expected_log = [
+      [:mget, ["5", "6"]],
+      [:mget, ["2", "3"]],
+    ]
+    assert_equal expected_log, database_log
+  end
+
+
+  it "Works with analyzing arguments with `loads:`"
+
+  it "Works when `loads:` returns `request ...`"
 end

From 95b9cfb83992196395b3d48332620b60005a6302 Mon Sep 17 00:00:00 2001
From: Robert Mosolgo <rdmosolgo@gmail.com>
Date: Fri, 5 Feb 2021 07:35:05 -0500
Subject: [PATCH 02/12] Add basic dataloader support for arguments

---
 .../execution/interpreter/arguments_cache.rb  | 16 ++---
 lib/graphql/execution/interpreter/runtime.rb  | 36 +++++++----
 lib/graphql/schema/argument.rb                | 59 +++++++++++++++++++
 3 files changed, 93 insertions(+), 18 deletions(-)

diff --git a/lib/graphql/execution/interpreter/arguments_cache.rb b/lib/graphql/execution/interpreter/arguments_cache.rb
index d5ad1d63d4..455da62ef5 100644
--- a/lib/graphql/execution/interpreter/arguments_cache.rb
+++ b/lib/graphql/execution/interpreter/arguments_cache.rb
@@ -10,7 +10,7 @@ def initialize(query)
             h[ast_node] = Hash.new do |h2, arg_owner|
               h2[arg_owner] = Hash.new do |h3, parent_object|
                 # First, normalize all AST or Ruby values to a plain Ruby hash
-                args_hash = prepare_args_hash(ast_node)
+                args_hash = self.class.prepare_args_hash(@query, ast_node)
                 # Then call into the schema to coerce those incoming values
                 args = arg_owner.coerce_arguments(parent_object, args_hash, query.context)
 
@@ -33,7 +33,7 @@ def fetch(ast_node, argument_owner, parent_object)
 
         NO_VALUE_GIVEN = Object.new
 
-        def prepare_args_hash(ast_arg_or_hash_or_value)
+        def self.prepare_args_hash(query, ast_arg_or_hash_or_value)
           case ast_arg_or_hash_or_value
           when Hash
             if ast_arg_or_hash_or_value.empty?
@@ -41,27 +41,27 @@ def prepare_args_hash(ast_arg_or_hash_or_value)
             end
             args_hash = {}
             ast_arg_or_hash_or_value.each do |k, v|
-              args_hash[k] = prepare_args_hash(v)
+              args_hash[k] = prepare_args_hash(query, v)
             end
             args_hash
           when Array
-            ast_arg_or_hash_or_value.map { |v| prepare_args_hash(v) }
+            ast_arg_or_hash_or_value.map { |v| prepare_args_hash(query, v) }
           when GraphQL::Language::Nodes::Field, GraphQL::Language::Nodes::InputObject, GraphQL::Language::Nodes::Directive
             if ast_arg_or_hash_or_value.arguments.empty?
               return NO_ARGUMENTS
             end
             args_hash = {}
             ast_arg_or_hash_or_value.arguments.each do |arg|
-              v = prepare_args_hash(arg.value)
+              v = prepare_args_hash(query, arg.value)
               if v != NO_VALUE_GIVEN
                 args_hash[arg.name] = v
               end
             end
             args_hash
           when GraphQL::Language::Nodes::VariableIdentifier
-            if @query.variables.key?(ast_arg_or_hash_or_value.name)
-              variable_value = @query.variables[ast_arg_or_hash_or_value.name]
-              prepare_args_hash(variable_value)
+            if query.variables.key?(ast_arg_or_hash_or_value.name)
+              variable_value = query.variables[ast_arg_or_hash_or_value.name]
+              prepare_args_hash(query, variable_value)
             else
               NO_VALUE_GIVEN
             end
diff --git a/lib/graphql/execution/interpreter/runtime.rb b/lib/graphql/execution/interpreter/runtime.rb
index e8ff640c7f..4a55cc8ba8 100644
--- a/lib/graphql/execution/interpreter/runtime.rb
+++ b/lib/graphql/execution/interpreter/runtime.rb
@@ -196,18 +196,34 @@ def evaluate_selection(path, result_name, field_ast_nodes_or_ast_node, scoped_co
             object = authorized_new(field_defn.owner, object, context, next_path)
           end
 
-          begin
-            kwarg_arguments = arguments(object, field_defn, ast_node)
-            # TODO: I think the thing to do here is, for each argument:
-            #  - enqueue a job to coerce _that_ argument and add it to the result set
-            #  - then check the result set: if it's complete, then call a method to continue execution
-            #  - I guess that'd be for each argument _definition_ -- maybe the counter would
-            #    be to check that each _definition_ has finished being loaded.
-          rescue GraphQL::ExecutionError, GraphQL::UnauthorizedError => e
-            continue_value(next_path, e, owner_type, field_defn, return_type.non_null?, ast_node)
-            return
+          total_args_count = field_defn.arguments.size
+          if total_args_count == 0
+            kwarg_arguments = GraphQL::Execution::Interpreter::Arguments::EMPTY
+            evaluate_selection_with_args(kwarg_arguments, field_defn, next_path, ast_node, field_ast_nodes, scoped_context, owner_type, object, is_eager_field, return_type)
+          else
+            # TODO: still use the cache somehow for this
+            # TODO: identify other uses of .arguments_for and use this instead
+            args_hash = ArgumentsCache.prepare_args_hash(@query, ast_node)
+            resolved_args_count = 0
+            argument_values = {}
+            field_defn.arguments.each do |arg_name, arg_defn|
+              @dataloader.append_job do
+                arg_defn.coerce_into_values(object, args_hash, context, argument_values)
+                resolved_args_count += 1
+                if resolved_args_count == total_args_count
+                  # Should be after any lazies
+                  kwarg_arguments = GraphQL::Execution::Interpreter::Arguments.new(
+                    argument_values: argument_values,
+                  )
+                  evaluate_selection_with_args(kwarg_arguments, field_defn, next_path, ast_node, field_ast_nodes, scoped_context, owner_type, object, is_eager_field, return_type)
+                end
+              end
+            end
           end
+        end
 
+        def evaluate_selection_with_args(kwarg_arguments, field_defn, next_path, ast_node, field_ast_nodes, scoped_context, owner_type, object, is_eager_field, return_type)
+          context.scoped_context = scoped_context
           after_lazy(kwarg_arguments, owner: owner_type, field: field_defn, path: next_path, ast_node: ast_node, scoped_context: context.scoped_context, owner_object: object, arguments: kwarg_arguments) do |resolved_arguments|
             if resolved_arguments.is_a?(GraphQL::ExecutionError) || resolved_arguments.is_a?(GraphQL::UnauthorizedError)
               continue_value(next_path, resolved_arguments, owner_type, field_defn, return_type.non_null?, ast_node)
diff --git a/lib/graphql/schema/argument.rb b/lib/graphql/schema/argument.rb
index d2de381be7..ff5a61eea5 100644
--- a/lib/graphql/schema/argument.rb
+++ b/lib/graphql/schema/argument.rb
@@ -236,6 +236,65 @@ def prepare_value(obj, value, context: nil)
         end
       end
 
+      # @api private
+      def coerce_into_values(parent_object, values, context, argument_values)
+        arg_name = graphql_name
+        arg_key = keyword
+        has_value = false
+        default_used = false
+        if values.key?(arg_name)
+          has_value = true
+          value = values[arg_name]
+        elsif values.key?(arg_key)
+          has_value = true
+          value = values[arg_key]
+        elsif default_value?
+          has_value = true
+          value = default_value
+          default_used = true
+        end
+
+        if has_value
+          loaded_value = nil
+          coerced_value = context.schema.error_handler.with_error_handling(context) do
+            type.coerce_input(value, context)
+          end
+
+          # TODO this should probably be inside after_lazy
+          if loads && !from_resolver?
+            loaded_value = if type.list?
+              loaded_values = coerced_value.map { |val| owner.load_application_object(self, loads, val, context) }
+              context.schema.after_any_lazies(loaded_values) { |result| result }
+            else
+              owner.load_application_object(self, loads, coerced_value, context)
+            end
+          end
+
+          coerced_value = if loaded_value
+            loaded_value
+          else
+            coerced_value
+          end
+
+          context.schema.after_lazy(coerced_value) do |coerced_value|
+            owner.validate_directive_argument(self, coerced_value)
+            prepared_value = context.schema.error_handler.with_error_handling(context) do
+              prepare_value(parent_object, coerced_value, context: context)
+            end
+
+            # TODO code smell to access such a deeply-nested constant in a distant module
+            argument_values[arg_key] = GraphQL::Execution::Interpreter::ArgumentValue.new(
+              value: prepared_value,
+              definition: self,
+              default_used: default_used,
+            )
+          end
+        else
+          # has_value is false
+          owner.validate_directive_argument(self, nil)
+        end
+      end
+
       private
 
       def validate_input_type(input_type)

From 1285317563f8e98dcc66bb1219321d1cd6b93f56 Mon Sep 17 00:00:00 2001
From: Robert Mosolgo <rdmosolgo@gmail.com>
Date: Fri, 5 Feb 2021 07:46:11 -0500
Subject: [PATCH 03/12] Support when loading argument returns .request

---
 lib/graphql/execution/interpreter/runtime.rb |  9 +++++----
 lib/graphql/schema/argument.rb               |  4 +++-
 spec/graphql/dataloader_spec.rb              | 16 ++++++++++++----
 3 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/lib/graphql/execution/interpreter/runtime.rb b/lib/graphql/execution/interpreter/runtime.rb
index 4a55cc8ba8..a6acc3eb98 100644
--- a/lib/graphql/execution/interpreter/runtime.rb
+++ b/lib/graphql/execution/interpreter/runtime.rb
@@ -211,10 +211,11 @@ def evaluate_selection(path, result_name, field_ast_nodes_or_ast_node, scoped_co
                 arg_defn.coerce_into_values(object, args_hash, context, argument_values)
                 resolved_args_count += 1
                 if resolved_args_count == total_args_count
-                  # Should be after any lazies
-                  kwarg_arguments = GraphQL::Execution::Interpreter::Arguments.new(
-                    argument_values: argument_values,
-                  )
+                  kwarg_arguments = schema.after_any_lazies(argument_values.values) {
+                    GraphQL::Execution::Interpreter::Arguments.new(
+                      argument_values: argument_values,
+                    )
+                  }
                   evaluate_selection_with_args(kwarg_arguments, field_defn, next_path, ast_node, field_ast_nodes, scoped_context, owner_type, object, is_eager_field, return_type)
                 end
               end
diff --git a/lib/graphql/schema/argument.rb b/lib/graphql/schema/argument.rb
index ff5a61eea5..68f9afbe41 100644
--- a/lib/graphql/schema/argument.rb
+++ b/lib/graphql/schema/argument.rb
@@ -276,7 +276,9 @@ def coerce_into_values(parent_object, values, context, argument_values)
             coerced_value
           end
 
-          context.schema.after_lazy(coerced_value) do |coerced_value|
+          # If this isn't lazy, then the block returns eagerly and assigns the result here
+          # If it _is_ lazy, then we write the lazy to the hash, then update it later
+          argument_values[arg_key] = context.schema.after_lazy(coerced_value) do |coerced_value|
             owner.validate_directive_argument(self, coerced_value)
             prepared_value = context.schema.error_handler.with_error_handling(context) do
               prepare_value(parent_object, coerced_value, context: context)
diff --git a/spec/graphql/dataloader_spec.rb b/spec/graphql/dataloader_spec.rb
index 9e355d569d..1588687009 100644
--- a/spec/graphql/dataloader_spec.rb
+++ b/spec/graphql/dataloader_spec.rb
@@ -188,7 +188,11 @@ def common_ingredients_with_load(recipe_1:, recipe_2:)
     query(Query)
 
     def self.object_from_id(id, ctx)
-      ctx.dataloader.with(DataObject).load(id)
+      if ctx[:use_request]
+        ctx.dataloader.with(DataObject).request(id)
+      else
+        ctx.dataloader.with(DataObject).load(id)
+      end
     end
 
     def self.resolve_type(type, obj, ctx)
@@ -405,7 +409,7 @@ def database_log
     assert_equal expected_log, database_log
   end
 
-  it "loads arguments in batches" do
+  it "loads arguments in batches, even with request" do
     query_str = <<-GRAPHQL
     {
       commonIngredientsWithLoad(recipe1Id: 5, recipe2Id: 6) {
@@ -428,10 +432,14 @@ def database_log
       [:mget, ["2", "3"]],
     ]
     assert_equal expected_log, database_log
+
+    # Run the same test, but using `.request` from object_from_id
+    database_log.clear
+    res2 = FiberSchema.execute(query_str, context: { use_request: true })
+    assert_equal expected_data, res2["data"]
+    assert_equal expected_log, database_log
   end
 
 
   it "Works with analyzing arguments with `loads:`"
-
-  it "Works when `loads:` returns `request ...`"
 end

From cf55589424111b5e5a667da7a01faa9589b1f022 Mon Sep 17 00:00:00 2001
From: Robert Mosolgo <rdmosolgo@gmail.com>
Date: Fri, 5 Feb 2021 10:08:41 -0500
Subject: [PATCH 04/12] Improve lazy / dataloader compatibility

---
 .../execution/interpreter/arguments_cache.rb  |  45 ++++-
 lib/graphql/execution/interpreter/resolve.rb  |   6 +
 lib/graphql/execution/interpreter/runtime.rb  |  15 +-
 lib/graphql/query/context.rb                  |   2 +-
 lib/graphql/schema/field.rb                   |   4 +
 spec/graphql/dataloader_spec.rb               |  39 +++-
 spec/graphql/execution_error_spec.rb          | 172 ++++++++++--------
 spec/graphql/schema/field_spec.rb             |   6 +
 8 files changed, 207 insertions(+), 82 deletions(-)

diff --git a/lib/graphql/execution/interpreter/arguments_cache.rb b/lib/graphql/execution/interpreter/arguments_cache.rb
index 455da62ef5..1ca3131120 100644
--- a/lib/graphql/execution/interpreter/arguments_cache.rb
+++ b/lib/graphql/execution/interpreter/arguments_cache.rb
@@ -6,18 +6,44 @@ class Interpreter
       class ArgumentsCache
         def initialize(query)
           @query = query
+          @dataloader = query.context.dataloader
           @storage = Hash.new do |h, ast_node|
             h[ast_node] = Hash.new do |h2, arg_owner|
               h2[arg_owner] = Hash.new do |h3, parent_object|
                 # First, normalize all AST or Ruby values to a plain Ruby hash
                 args_hash = self.class.prepare_args_hash(@query, ast_node)
                 # Then call into the schema to coerce those incoming values
-                args = arg_owner.coerce_arguments(parent_object, args_hash, query.context)
+                total_args_count = arg_owner.arguments.size
+                if total_args_count == 0
+                  h3[parent_object] = NO_ARGUMENTS
+                else
+                  resolved_args_count = 0
+                  argument_values = {}
+                  # TODO how to make sure calls into this cache _while the data is pending_
+                  # work properly?
+                  h3[parent_object] = argument_values
 
-                h3[parent_object] = @query.schema.after_lazy(args) do |resolved_args|
-                  # when this promise is resolved, update the cache with the resolved value
-                  h3[parent_object] = resolved_args
+                  arg_owner.arguments.each do |arg_name, arg_defn|
+                    @dataloader.append_job do
+                      arg_defn.coerce_into_values(parent_object, args_hash, @query.context, argument_values)
+                      resolved_args_count += 1
+                      if resolved_args_count == total_args_count
+                        kwarg_arguments = @query.schema.after_any_lazies(argument_values.values) {
+                          GraphQL::Execution::Interpreter::Arguments.new(
+                            argument_values: argument_values,
+                          )
+                        }
+
+                        h3[parent_object] = @query.schema.after_lazy(kwarg_arguments) do |resolved_args|
+                          # when this promise is resolved, update the cache with the resolved value
+                          h3[parent_object] = resolved_args
+                        end
+                      end
+                    end
+                  end
                 end
+
+                h3[parent_object]
               end
             end
           end
@@ -25,6 +51,17 @@ def initialize(query)
 
         def fetch(ast_node, argument_owner, parent_object)
           @storage[ast_node][argument_owner][parent_object]
+          # If any jobs were enqueued, run them now,
+          # since this might have been called outside of execution.
+          # (The jobs are responsible for updating `result` in-place.)
+          @dataloader.run
+          # Ack, the _hash_ is updated, but the key is eventually
+          # overridden with an immutable arguments instance.
+          # The first call queues up the job,
+          # then this call fetches the result.
+          # TODO this should be better, find a solution
+          # that works with merging the runtime.rb code
+          @storage[ast_node][argument_owner][parent_object]
         end
 
         private
diff --git a/lib/graphql/execution/interpreter/resolve.rb b/lib/graphql/execution/interpreter/resolve.rb
index e89072c724..133b50e3f9 100644
--- a/lib/graphql/execution/interpreter/resolve.rb
+++ b/lib/graphql/execution/interpreter/resolve.rb
@@ -25,6 +25,12 @@ def self.resolve_all(results, dataloader)
         #
         # @return [void]
         def self.resolve(results, dataloader)
+          # There might be pending jobs here that _will_ write lazies
+          # into the result hash. We should run them out, so we
+          # can be sure that all lazies will be present in the result hashes.
+          # A better implementation would somehow interleave (or unify)
+          # these approaches.
+          dataloader.run
           next_results = []
           while results.any?
             result_value = results.shift
diff --git a/lib/graphql/execution/interpreter/runtime.rb b/lib/graphql/execution/interpreter/runtime.rb
index a6acc3eb98..39f2a81452 100644
--- a/lib/graphql/execution/interpreter/runtime.rb
+++ b/lib/graphql/execution/interpreter/runtime.rb
@@ -201,16 +201,23 @@ def evaluate_selection(path, result_name, field_ast_nodes_or_ast_node, scoped_co
             kwarg_arguments = GraphQL::Execution::Interpreter::Arguments::EMPTY
             evaluate_selection_with_args(kwarg_arguments, field_defn, next_path, ast_node, field_ast_nodes, scoped_context, owner_type, object, is_eager_field, return_type)
           else
-            # TODO: still use the cache somehow for this
-            # TODO: identify other uses of .arguments_for and use this instead
+            # TODO: DRY this with ArgumentsCache
             args_hash = ArgumentsCache.prepare_args_hash(@query, ast_node)
             resolved_args_count = 0
             argument_values = {}
+            raised_error = false
             field_defn.arguments.each do |arg_name, arg_defn|
               @dataloader.append_job do
-                arg_defn.coerce_into_values(object, args_hash, context, argument_values)
+                # TODO put error handling in args cache, too
+                begin
+                  arg_defn.coerce_into_values(object, args_hash, context, argument_values)
+                rescue GraphQL::ExecutionError, GraphQL::UnauthorizedError => e
+                  raised_error = true
+                  continue_value(next_path, e, owner_type, field_defn, return_type.non_null?, ast_node)
+                end
+
                 resolved_args_count += 1
-                if resolved_args_count == total_args_count
+                if resolved_args_count == total_args_count && !raised_error
                   kwarg_arguments = schema.after_any_lazies(argument_values.values) {
                     GraphQL::Execution::Interpreter::Arguments.new(
                       argument_values: argument_values,
diff --git a/lib/graphql/query/context.rb b/lib/graphql/query/context.rb
index a174838ca0..a4bc8fe16a 100644
--- a/lib/graphql/query/context.rb
+++ b/lib/graphql/query/context.rb
@@ -157,7 +157,7 @@ def initialize(query:, schema: query.schema, values:, object:)
       end
 
       def dataloader
-        @dataloader ||= query.multiplex.dataloader
+        @dataloader ||= query.multiplex ? query.multiplex.dataloader : schema.dataloader_class.new
       end
 
       # @api private
diff --git a/lib/graphql/schema/field.rb b/lib/graphql/schema/field.rb
index 8f5363cbd6..15b2cd3887 100644
--- a/lib/graphql/schema/field.rb
+++ b/lib/graphql/schema/field.rb
@@ -60,6 +60,10 @@ def introspection?
         @introspection
       end
 
+      def inspect
+        "#<#{self.class} #{path}#{arguments.any? ? "(...)" : ""}: #{type.to_type_signature}>"
+      end
+
       alias :mutation :resolver
 
       # @return [Boolean] Apply tracing to this field? (Default: skip scalars, this is the override value)
diff --git a/spec/graphql/dataloader_spec.rb b/spec/graphql/dataloader_spec.rb
index 1588687009..04ef29939e 100644
--- a/spec/graphql/dataloader_spec.rb
+++ b/spec/graphql/dataloader_spec.rb
@@ -440,6 +440,43 @@ def database_log
     assert_equal expected_log, database_log
   end
 
+  class UsageAnalyzer < GraphQL::Analysis::AST::Analyzer
+    def initialize(query)
+      @query = query
+      @fields = Set.new
+    end
 
-  it "Works with analyzing arguments with `loads:`"
+    def on_enter_field(node, parent, visitor)
+      args = @query.arguments_for(node, visitor.field_definition)
+      if args.is_a?(GraphQL::Execution::Lazy)
+        args = args.value
+      end
+      @fields << [node.name, args.keys]
+    end
+
+    def result
+      @fields
+    end
+  end
+
+  it "Works with analyzing arguments with `loads:`, even with .request" do
+    query_str = <<-GRAPHQL
+    {
+      commonIngredientsWithLoad(recipe1Id: 5, recipe2Id: 6) {
+        name
+      }
+    }
+    GRAPHQL
+    query = GraphQL::Query.new(FiberSchema, query_str)
+    results = GraphQL::Analysis::AST.analyze_query(query, [UsageAnalyzer])
+    expected_results = [
+      ["commonIngredientsWithLoad", [:recipe_1, :recipe_2]],
+      ["name", []],
+    ]
+    assert_equal expected_results, results.first.to_a
+
+    query2 = GraphQL::Query.new(FiberSchema, query_str, context: { use_request: true })
+    result2 = GraphQL::Analysis::AST.analyze_query(query2, [UsageAnalyzer])
+    assert_equal expected_results, result2.first.to_a
+  end
 end
diff --git a/spec/graphql/execution_error_spec.rb b/spec/graphql/execution_error_spec.rb
index e181bbe613..5b730c7127 100644
--- a/spec/graphql/execution_error_spec.rb
+++ b/spec/graphql/execution_error_spec.rb
@@ -65,79 +65,79 @@
               "flavor" => "Manchego",
             },
             "flavor" => "Brie",
-            },
-            "allDairy" => [
-              { "flavor" => "Brie" },
-              { "flavor" => "Gouda" },
-              { "flavor" => "Manchego" },
-              { "source" => "COW", "executionError" => nil }
-            ],
-            "dairyErrors" => [
-              { "__typename" => "Cheese" },
-              nil,
-              { "__typename" => "Cheese" },
-              { "__typename" => "Milk" }
-            ],
-            "dairy" => {
-              "milks" => [
-                {
-                  "source" => "COW",
-                  "executionError" => nil,
-                  "allDairy" => [
-                    { "__typename" => "Cheese" },
-                    { "__typename" => "Cheese" },
-                    { "__typename" => "Cheese" },
-                    { "__typename" => "Milk", "origin" => "Antiquity", "executionError" => nil }
-                  ]
-                }
-              ]
-            },
-            "executionError" => nil,
-            "valueWithExecutionError" => 0
           },
-          "errors"=>[
-            {
-              "message"=>"missing dairy",
-              "locations"=>[{"line"=>25, "column"=>5}],
-              "path"=>["dairyErrors", 1]
-            },
-            {
-              "message"=>"There was an execution error",
-              "locations"=>[{"line"=>41, "column"=>5}],
-              "path"=>["executionError"]
-            },
-            {
-              "message"=>"Could not fetch latest value",
-              "locations"=>[{"line"=>42, "column"=>5}],
-              "path"=>["valueWithExecutionError"]
-            },
-            {
-              "message"=>"There was an execution error",
-              "locations"=>[{"line"=>31, "column"=>9}],
-              "path"=>["dairy", "milks", 0, "executionError"]
-            },
-            {
-              "message"=>"No cheeses are made from Yak milk!",
-              "locations"=>[{"line"=>5, "column"=>7}],
-              "path"=>["cheese", "error1"]
-            },
-            {
-              "message"=>"No cheeses are made from Yak milk!",
-              "locations"=>[{"line"=>8, "column"=>7}],
-              "path"=>["cheese", "error2"]
-            },
-            {
-              "message"=>"There was an execution error",
-              "locations"=>[{"line"=>22, "column"=>9}],
-              "path"=>["allDairy", 3, "executionError"]
-            },
-            {
-              "message"=>"There was an execution error",
-              "locations"=>[{"line"=>36, "column"=>13}],
-              "path"=>["dairy", "milks", 0, "allDairy", 3, "executionError"]
-            },
-          ]
-        }
+          "allDairy" => [
+            { "flavor" => "Brie" },
+            { "flavor" => "Gouda" },
+            { "flavor" => "Manchego" },
+            { "source" => "COW", "executionError" => nil }
+          ],
+          "dairyErrors" => [
+            { "__typename" => "Cheese" },
+            nil,
+            { "__typename" => "Cheese" },
+            { "__typename" => "Milk" }
+          ],
+          "dairy" => {
+            "milks" => [
+              {
+                "source" => "COW",
+                "executionError" => nil,
+                "allDairy" => [
+                  { "__typename" => "Cheese" },
+                  { "__typename" => "Cheese" },
+                  { "__typename" => "Cheese" },
+                  { "__typename" => "Milk", "origin" => "Antiquity", "executionError" => nil }
+                ]
+              }
+            ]
+          },
+          "executionError" => nil,
+          "valueWithExecutionError" => 0
+        },
+        "errors"=>[
+          {
+            "message"=>"There was an execution error",
+            "locations"=>[{"line"=>41, "column"=>5}],
+            "path"=>["executionError"]
+          },
+          {
+            "message"=>"Could not fetch latest value",
+            "locations"=>[{"line"=>42, "column"=>5}],
+            "path"=>["valueWithExecutionError"]
+          },
+          {
+            "message"=>"missing dairy",
+            "locations"=>[{"line"=>25, "column"=>5}],
+            "path"=>["dairyErrors", 1]
+          },
+          {
+            "message"=>"There was an execution error",
+            "locations"=>[{"line"=>31, "column"=>9}],
+            "path"=>["dairy", "milks", 0, "executionError"]
+          },
+          {
+            "message"=>"There was an execution error",
+            "locations"=>[{"line"=>22, "column"=>9}],
+            "path"=>["allDairy", 3, "executionError"]
+          },
+          {
+            "message"=>"There was an execution error",
+            "locations"=>[{"line"=>36, "column"=>13}],
+            "path"=>["dairy", "milks", 0, "allDairy", 3, "executionError"]
+          },
+          {
+            "message"=>"No cheeses are made from Yak milk!",
+            "locations"=>[{"line"=>5, "column"=>7}],
+            "path"=>["cheese", "error1"]
+          },
+          {
+            "message"=>"No cheeses are made from Yak milk!",
+            "locations"=>[{"line"=>8, "column"=>7}],
+            "path"=>["cheese", "error2"]
+          },
+        ]
+      }
       assert_equal(expected_result, result.to_h)
     end
   end
@@ -195,6 +195,34 @@
     end
   end
 
+  describe "minimal lazy non-error case" do
+    let(:query_string) {%|
+  {
+    cheese(id: 1) {
+      nonError: similarCheese(source: [SHEEP]) {
+        id
+      }
+    }
+  }
+    |}
+
+    it "does lazy non-errors right" do
+      # This is extracted from the test above -- it kept breaking
+      # when working on dataloader, so I isolated it to keep an eye
+      # on the minimal reproduction
+      expected_result = {
+        "data"=>{
+          "cheese"=>{
+            "nonError"=> {
+              "id" => 3,
+            },
+          },
+        }
+      }
+      assert_equal(expected_result, result.to_h)
+    end
+  end
+
   describe "fragment query when returned from a field" do
     let(:query_string) {%|
     query MilkQuery {
diff --git a/spec/graphql/schema/field_spec.rb b/spec/graphql/schema/field_spec.rb
index 8426805bf9..707de6a28e 100644
--- a/spec/graphql/schema/field_spec.rb
+++ b/spec/graphql/schema/field_spec.rb
@@ -13,6 +13,12 @@
       end
     end
 
+    describe "inspect" do
+      it "includes the path and return type" do
+        assert_equal "#<Jazz::BaseField Query.inspectInput(...): [String!]!>", field.inspect
+      end
+    end
+
     it "uses the argument class" do
       arg_defn = field.graphql_definition.arguments.values.first
       assert_equal :ok, arg_defn.metadata[:custom]

From 0c1b4aaebb665d00ede94e213e2a9733dca8d3e3 Mon Sep 17 00:00:00 2001
From: Robert Mosolgo <rdmosolgo@gmail.com>
Date: Fri, 5 Feb 2021 10:16:55 -0500
Subject: [PATCH 05/12] skip linter on continuation method

---
 lib/graphql/execution/interpreter/runtime.rb | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/graphql/execution/interpreter/runtime.rb b/lib/graphql/execution/interpreter/runtime.rb
index 39f2a81452..dfbff45d32 100644
--- a/lib/graphql/execution/interpreter/runtime.rb
+++ b/lib/graphql/execution/interpreter/runtime.rb
@@ -199,7 +199,7 @@ def evaluate_selection(path, result_name, field_ast_nodes_or_ast_node, scoped_co
           total_args_count = field_defn.arguments.size
           if total_args_count == 0
             kwarg_arguments = GraphQL::Execution::Interpreter::Arguments::EMPTY
-            evaluate_selection_with_args(kwarg_arguments, field_defn, next_path, ast_node, field_ast_nodes, scoped_context, owner_type, object, is_eager_field, return_type)
+            evaluate_selection_with_args(kwarg_arguments, field_defn, next_path, ast_node, field_ast_nodes, scoped_context, owner_type, object, is_eager_field)
           else
             # TODO: DRY this with ArgumentsCache
             args_hash = ArgumentsCache.prepare_args_hash(@query, ast_node)
@@ -223,15 +223,16 @@ def evaluate_selection(path, result_name, field_ast_nodes_or_ast_node, scoped_co
                       argument_values: argument_values,
                     )
                   }
-                  evaluate_selection_with_args(kwarg_arguments, field_defn, next_path, ast_node, field_ast_nodes, scoped_context, owner_type, object, is_eager_field, return_type)
+                  evaluate_selection_with_args(kwarg_arguments, field_defn, next_path, ast_node, field_ast_nodes, scoped_context, owner_type, object, is_eager_field)
                 end
               end
             end
           end
         end
 
-        def evaluate_selection_with_args(kwarg_arguments, field_defn, next_path, ast_node, field_ast_nodes, scoped_context, owner_type, object, is_eager_field, return_type)
+        def evaluate_selection_with_args(kwarg_arguments, field_defn, next_path, ast_node, field_ast_nodes, scoped_context, owner_type, object, is_eager_field)  # rubocop:disable Metrics/ParameterLists
           context.scoped_context = scoped_context
+          return_type = field_defn.type
           after_lazy(kwarg_arguments, owner: owner_type, field: field_defn, path: next_path, ast_node: ast_node, scoped_context: context.scoped_context, owner_object: object, arguments: kwarg_arguments) do |resolved_arguments|
             if resolved_arguments.is_a?(GraphQL::ExecutionError) || resolved_arguments.is_a?(GraphQL::UnauthorizedError)
               continue_value(next_path, resolved_arguments, owner_type, field_defn, return_type.non_null?, ast_node)

From 48058cfdbebfc0ea7450e0f4099fdee6c1f5038a Mon Sep 17 00:00:00 2001
From: Robert Mosolgo <rdmosolgo@gmail.com>
Date: Fri, 5 Feb 2021 11:42:28 -0500
Subject: [PATCH 06/12] Reimplement #coerce_arguments, call it from
 ArgumentsCache (again)

---
 guides/dataloader/overview.md                 |  2 -
 .../execution/interpreter/arguments_cache.rb  | 48 ++++-----
 lib/graphql/query.rb                          | 10 +-
 lib/graphql/schema/member/has_arguments.rb    | 98 +++++++------------
 spec/graphql/dataloader_spec.rb               |  2 +
 5 files changed, 64 insertions(+), 96 deletions(-)

diff --git a/guides/dataloader/overview.md b/guides/dataloader/overview.md
index 3fcdeec77d..d559df06a2 100644
--- a/guides/dataloader/overview.md
+++ b/guides/dataloader/overview.md
@@ -127,8 +127,6 @@ class FollowUser < GraphQL::Schema::Mutation
 end
 ```
 
-__Caveat:__ The current implementation loads all `loads:` objects in the _same fiber_, so data will be fetched sequentially.
-
 ## Data Sources
 
 To implement batch-loading data sources, see the {% internal_link "Sources guide", "/dataloader/sources" %}.
diff --git a/lib/graphql/execution/interpreter/arguments_cache.rb b/lib/graphql/execution/interpreter/arguments_cache.rb
index 1ca3131120..b0464b839d 100644
--- a/lib/graphql/execution/interpreter/arguments_cache.rb
+++ b/lib/graphql/execution/interpreter/arguments_cache.rb
@@ -10,40 +10,18 @@ def initialize(query)
           @storage = Hash.new do |h, ast_node|
             h[ast_node] = Hash.new do |h2, arg_owner|
               h2[arg_owner] = Hash.new do |h3, parent_object|
-                # First, normalize all AST or Ruby values to a plain Ruby hash
-                args_hash = self.class.prepare_args_hash(@query, ast_node)
-                # Then call into the schema to coerce those incoming values
-                total_args_count = arg_owner.arguments.size
-                if total_args_count == 0
-                  h3[parent_object] = NO_ARGUMENTS
-                else
-                  resolved_args_count = 0
-                  argument_values = {}
-                  # TODO how to make sure calls into this cache _while the data is pending_
-                  # work properly?
-                  h3[parent_object] = argument_values
-
-                  arg_owner.arguments.each do |arg_name, arg_defn|
-                    @dataloader.append_job do
-                      arg_defn.coerce_into_values(parent_object, args_hash, @query.context, argument_values)
-                      resolved_args_count += 1
-                      if resolved_args_count == total_args_count
-                        kwarg_arguments = @query.schema.after_any_lazies(argument_values.values) {
-                          GraphQL::Execution::Interpreter::Arguments.new(
-                            argument_values: argument_values,
-                          )
-                        }
-
-                        h3[parent_object] = @query.schema.after_lazy(kwarg_arguments) do |resolved_args|
-                          # when this promise is resolved, update the cache with the resolved value
-                          h3[parent_object] = resolved_args
-                        end
-                      end
-                    end
+                dataload_for(ast_node, arg_owner, parent_object) do |kwarg_arguments|
+                  h3[parent_object] = @query.schema.after_lazy(kwarg_arguments) do |resolved_args|
+                    h3[parent_object] = resolved_args
                   end
                 end
 
-                h3[parent_object]
+                if !h3.key?(parent_object)
+                  # TODO should i bother putting anything here?
+                  h3[parent_object] = NO_ARGUMENTS
+                else
+                  h3[parent_object]
+                end
               end
             end
           end
@@ -64,6 +42,14 @@ def fetch(ast_node, argument_owner, parent_object)
           @storage[ast_node][argument_owner][parent_object]
         end
 
+        # @yield [Interpreter::Arguments, Lazy<Interpreter::Arguments>] The finally-loaded arguments
+        def dataload_for(ast_node, argument_owner, parent_object, &block)
+          # First, normalize all AST or Ruby values to a plain Ruby hash
+          args_hash = self.class.prepare_args_hash(@query, ast_node)
+          argument_owner.coerce_arguments(parent_object, args_hash, @query.context, &block)
+          nil
+        end
+
         private
 
         NO_ARGUMENTS = {}.freeze
diff --git a/lib/graphql/query.rb b/lib/graphql/query.rb
index 773cac3156..e70b20fe50 100644
--- a/lib/graphql/query.rb
+++ b/lib/graphql/query.rb
@@ -251,12 +251,18 @@ def irep_selection
     # @param parent_object [GraphQL::Schema::Object]
     # @return Hash{Symbol => Object}
     def arguments_for(ast_node, definition, parent_object: nil)
+      if interpreter?
+        arguments_cache.fetch(ast_node, definition, parent_object)
+      else
+        arguments_cache[ast_node][definition]
+      end
+    end
+
+    def arguments_cache
       if interpreter?
         @arguments_cache ||= Execution::Interpreter::ArgumentsCache.new(self)
-        @arguments_cache.fetch(ast_node, definition, parent_object)
       else
         @arguments_cache ||= ArgumentsCache.build(self)
-        @arguments_cache[ast_node][definition]
       end
     end
 
diff --git a/lib/graphql/schema/member/has_arguments.rb b/lib/graphql/schema/member/has_arguments.rb
index 3a300078cf..0a86dff4da 100644
--- a/lib/graphql/schema/member/has_arguments.rb
+++ b/lib/graphql/schema/member/has_arguments.rb
@@ -81,79 +81,55 @@ def argument_class(new_arg_class = nil)
         end
 
         # @api private
+        # If given a block, it will eventually yield the loaded args to the block.
+        #
+        # If no block is given, it will immediately dataload (but might return a Lazy).
+        #
         # @param values [Hash<String, Object>]
         # @param context [GraphQL::Query::Context]
-        # @return [Hash<Symbol, Object>, Execution::Lazy<Hash>]
-        def coerce_arguments(parent_object, values, context)
+        # @yield [Interpreter::Arguments, Execution::Lazy<Interpeter::Arguments>]
+        # @return [Interpreter::Arguments, Execution::Lazy<Interpeter::Arguments>]
+        def coerce_arguments(parent_object, values, context, &block)
           # Cache this hash to avoid re-merging it
           arg_defns = self.arguments
+          total_args_count = arg_defns.size
 
-          if arg_defns.empty?
-            GraphQL::Execution::Interpreter::Arguments::EMPTY
+          if total_args_count == 0
+            final_args = GraphQL::Execution::Interpreter::Arguments::EMPTY
+            if block_given?
+              block.call(final_args)
+              nil
+            else
+              final_args
+            end
           else
+            finished_args = nil
             argument_values = {}
-            arg_lazies = arg_defns.map do |arg_name, arg_defn|
-              arg_key = arg_defn.keyword
-              has_value = false
-              default_used = false
-              if values.key?(arg_name)
-                has_value = true
-                value = values[arg_name]
-              elsif values.key?(arg_key)
-                has_value = true
-                value = values[arg_key]
-              elsif arg_defn.default_value?
-                has_value = true
-                value = arg_defn.default_value
-                default_used = true
-              end
-
-              if has_value
-                loads = arg_defn.loads
-                loaded_value = nil
-                coerced_value = context.schema.error_handler.with_error_handling(context) do
-                  arg_defn.type.coerce_input(value, context)
-                end
-
-                # TODO this should probably be inside after_lazy
-                if loads && !arg_defn.from_resolver?
-                  loaded_value = if arg_defn.type.list?
-                    loaded_values = coerced_value.map { |val| load_application_object(arg_defn, loads, val, context) }
-                    context.schema.after_any_lazies(loaded_values) { |result| result }
-                  else
-                    load_application_object(arg_defn, loads, coerced_value, context)
+            resolved_args_count = 0
+            arg_defns.each do |arg_name, arg_defn|
+              context.dataloader.append_job do
+                arg_defn.coerce_into_values(parent_object, values, context, argument_values)
+                resolved_args_count += 1
+                if resolved_args_count == total_args_count
+                  finished_args = context.schema.after_any_lazies(argument_values.values) {
+                    GraphQL::Execution::Interpreter::Arguments.new(
+                      argument_values: argument_values,
+                    )
+                  }
+
+                  if block_given?
+                    block.call(finished_args)
                   end
                 end
-
-                coerced_value = if loaded_value
-                  loaded_value
-                else
-                  coerced_value
-                end
-
-                context.schema.after_lazy(coerced_value) do |coerced_value|
-                  validate_directive_argument(arg_defn, coerced_value)
-                  prepared_value = context.schema.error_handler.with_error_handling(context) do
-                    arg_defn.prepare_value(parent_object, coerced_value, context: context)
-                  end
-
-                  # TODO code smell to access such a deeply-nested constant in a distant module
-                  argument_values[arg_key] = GraphQL::Execution::Interpreter::ArgumentValue.new(
-                    value: prepared_value,
-                    definition: arg_defn,
-                    default_used: default_used,
-                  )
-                end
-              else
-                # has_value is false
-                validate_directive_argument(arg_defn, nil)
               end
             end
 
-            context.schema.after_any_lazies(arg_lazies) do
-              GraphQL::Execution::Interpreter::Arguments.new(
-                argument_values: argument_values,
-              )
+            if block_given?
+              nil
+            else
+              # This API returns eagerly, gotta run it now
+              context.dataloader.run
+              finished_args
             end
           end
         end
diff --git a/spec/graphql/dataloader_spec.rb b/spec/graphql/dataloader_spec.rb
index 04ef29939e..ccef257fbd 100644
--- a/spec/graphql/dataloader_spec.rb
+++ b/spec/graphql/dataloader_spec.rb
@@ -448,6 +448,8 @@ def initialize(query)
 
     def on_enter_field(node, parent, visitor)
       args = @query.arguments_for(node, visitor.field_definition)
+      # This bug has been around for a while,
+      # see https://github.com/rmosolgo/graphql-ruby/issues/3321
       if args.is_a?(GraphQL::Execution::Lazy)
         args = args.value
       end

From 66d6277c92ddd0531d4e022605f1d0b3194f9021 Mon Sep 17 00:00:00 2001
From: Robert Mosolgo <rdmosolgo@gmail.com>
Date: Fri, 5 Feb 2021 11:52:40 -0500
Subject: [PATCH 07/12] Move error handling into coerce_arguments

---
 lib/graphql/execution/interpreter/runtime.rb | 28 +++-----------------
 lib/graphql/schema/input_object.rb           |  8 ++++--
 lib/graphql/schema/member/has_arguments.rb   | 15 +++++++++--
 3 files changed, 22 insertions(+), 29 deletions(-)

diff --git a/lib/graphql/execution/interpreter/runtime.rb b/lib/graphql/execution/interpreter/runtime.rb
index dfbff45d32..87d2e845ea 100644
--- a/lib/graphql/execution/interpreter/runtime.rb
+++ b/lib/graphql/execution/interpreter/runtime.rb
@@ -201,31 +201,9 @@ def evaluate_selection(path, result_name, field_ast_nodes_or_ast_node, scoped_co
             kwarg_arguments = GraphQL::Execution::Interpreter::Arguments::EMPTY
             evaluate_selection_with_args(kwarg_arguments, field_defn, next_path, ast_node, field_ast_nodes, scoped_context, owner_type, object, is_eager_field)
           else
-            # TODO: DRY this with ArgumentsCache
-            args_hash = ArgumentsCache.prepare_args_hash(@query, ast_node)
-            resolved_args_count = 0
-            argument_values = {}
-            raised_error = false
-            field_defn.arguments.each do |arg_name, arg_defn|
-              @dataloader.append_job do
-                # TODO put error handling in args cache, too
-                begin
-                  arg_defn.coerce_into_values(object, args_hash, context, argument_values)
-                rescue GraphQL::ExecutionError, GraphQL::UnauthorizedError => e
-                  raised_error = true
-                  continue_value(next_path, e, owner_type, field_defn, return_type.non_null?, ast_node)
-                end
-
-                resolved_args_count += 1
-                if resolved_args_count == total_args_count && !raised_error
-                  kwarg_arguments = schema.after_any_lazies(argument_values.values) {
-                    GraphQL::Execution::Interpreter::Arguments.new(
-                      argument_values: argument_values,
-                    )
-                  }
-                  evaluate_selection_with_args(kwarg_arguments, field_defn, next_path, ast_node, field_ast_nodes, scoped_context, owner_type, object, is_eager_field)
-                end
-              end
+            # TODO remove all arguments(...) usages?
+            @query.arguments_cache.dataload_for(ast_node, field_defn, object) do |resolved_arguments|
+              evaluate_selection_with_args(resolved_arguments, field_defn, next_path, ast_node, field_ast_nodes, scoped_context, owner_type, object, is_eager_field)
             end
           end
         end
diff --git a/lib/graphql/schema/input_object.rb b/lib/graphql/schema/input_object.rb
index 9c0e774ccb..ca0e3ee56e 100644
--- a/lib/graphql/schema/input_object.rb
+++ b/lib/graphql/schema/input_object.rb
@@ -214,8 +214,12 @@ def coerce_input(value, ctx)
           arguments = coerce_arguments(nil, value, ctx)
 
           ctx.schema.after_lazy(arguments) do |resolved_arguments|
-            input_obj_instance = self.new(resolved_arguments, ruby_kwargs: resolved_arguments.keyword_arguments, context: ctx, defaults_used: nil)
-            input_obj_instance.prepare
+            if resolved_arguments.is_a?(GraphQL::Error)
+              raise resolved_arguments
+            else
+              input_obj_instance = self.new(resolved_arguments, ruby_kwargs: resolved_arguments.keyword_arguments, context: ctx, defaults_used: nil)
+              input_obj_instance.prepare
+            end
           end
         end
 
diff --git a/lib/graphql/schema/member/has_arguments.rb b/lib/graphql/schema/member/has_arguments.rb
index 0a86dff4da..2d8f39571c 100644
--- a/lib/graphql/schema/member/has_arguments.rb
+++ b/lib/graphql/schema/member/has_arguments.rb
@@ -106,11 +106,22 @@ def coerce_arguments(parent_object, values, context, &block)
             finished_args = nil
             argument_values = {}
             resolved_args_count = 0
+            raised_error = false
             arg_defns.each do |arg_name, arg_defn|
               context.dataloader.append_job do
-                arg_defn.coerce_into_values(parent_object, values, context, argument_values)
+                begin
+                  arg_defn.coerce_into_values(parent_object, values, context, argument_values)
+                rescue GraphQL::ExecutionError, GraphQL::UnauthorizedError => err
+                  raised_error = true
+                  if block_given?
+                    block.call(err)
+                  else
+                    finished_args = err
+                  end
+                end
+
                 resolved_args_count += 1
-                if resolved_args_count == total_args_count
+                if resolved_args_count == total_args_count && !raised_error
                   finished_args = context.schema.after_any_lazies(argument_values.values) {
                     GraphQL::Execution::Interpreter::Arguments.new(
                       argument_values: argument_values,

From 076c78e9188df18ec0f4f065dcdbbb676e4371ba Mon Sep 17 00:00:00 2001
From: Robert Mosolgo <rdmosolgo@gmail.com>
Date: Fri, 5 Feb 2021 15:42:00 -0500
Subject: [PATCH 08/12] Add more args tests, update variables_spec

---
 spec/graphql/dataloader_spec.rb               | 78 +++++++++++++++++++
 .../rails/graphql/query/variables_spec.rb     | 19 ++---
 2 files changed, 88 insertions(+), 9 deletions(-)

diff --git a/spec/graphql/dataloader_spec.rb b/spec/graphql/dataloader_spec.rb
index ccef257fbd..ee32f6b107 100644
--- a/spec/graphql/dataloader_spec.rb
+++ b/spec/graphql/dataloader_spec.rb
@@ -183,6 +183,22 @@ def common_ingredients_with_load(recipe_1:, recipe_2:)
         common_ids = recipe_1[:ingredient_ids] & recipe_2[:ingredient_ids]
         dataloader.with(DataObject).load_all(common_ids)
       end
+
+      field :common_ingredients_from_input_object, [Ingredient], null: false do
+        class CommonIngredientsInput < GraphQL::Schema::InputObject
+          argument :recipe_1_id, ID, required: true, loads: Recipe
+          argument :recipe_2_id, ID, required: true, loads: Recipe
+        end
+        argument :input, CommonIngredientsInput, required: true
+      end
+
+
+      def common_ingredients_from_input_object(input:)
+        recipe_1 = input[:recipe_1]
+        recipe_2 = input[:recipe_2]
+        common_ids = recipe_1[:ingredient_ids] & recipe_2[:ingredient_ids]
+        dataloader.with(DataObject).load_all(common_ids)
+      end
     end
 
     query(Query)
@@ -481,4 +497,66 @@ def result
     result2 = GraphQL::Analysis::AST.analyze_query(query2, [UsageAnalyzer])
     assert_equal expected_results, result2.first.to_a
   end
+
+  it "Works with input objects, load and request" do
+    query_str = <<-GRAPHQL
+    {
+      commonIngredientsFromInputObject(input: { recipe1Id: 5, recipe2Id: 6 }) {
+        name
+      }
+    }
+    GRAPHQL
+    res = FiberSchema.execute(query_str)
+    expected_data = {
+      "commonIngredientsFromInputObject" => [
+        {"name"=>"Corn"},
+        {"name"=>"Butter"},
+      ]
+    }
+    assert_equal expected_data, res["data"]
+
+    expected_log = [
+      [:mget, ["5", "6"]],
+      [:mget, ["2", "3"]],
+    ]
+    assert_equal expected_log, database_log
+
+
+    # Run the same test, but using `.request` from object_from_id
+    database_log.clear
+    res2 = FiberSchema.execute(query_str, context: { use_request: true })
+    assert_equal expected_data, res2["data"]
+    assert_equal expected_log, database_log
+  end
+
+  it "Works with input objects using variables, load and request" do
+    query_str = <<-GRAPHQL
+    query($input: CommonIngredientsInput!) {
+      commonIngredientsFromInputObject(input: $input) {
+        name
+      }
+    }
+    GRAPHQL
+    res = FiberSchema.execute(query_str, variables: { input: { recipe1Id: 5, recipe2Id: 6 }})
+    expected_data = {
+      "commonIngredientsFromInputObject" => [
+        {"name"=>"Corn"},
+        {"name"=>"Butter"},
+      ]
+    }
+    assert_equal expected_data, res["data"]
+
+    expected_log = [
+      [:mget, ["5", "6"]],
+      [:mget, ["2", "3"]],
+    ]
+    assert_equal expected_log, database_log
+
+
+    # Run the same test, but using `.request` from object_from_id
+    database_log.clear
+    res2 = FiberSchema.execute(query_str, context: { use_request: true }, variables: { input: { recipe1Id: 5, recipe2Id: 6 }})
+    assert_equal expected_data, res2["data"]
+    assert_equal expected_log, database_log
+  end
 end
diff --git a/spec/integration/rails/graphql/query/variables_spec.rb b/spec/integration/rails/graphql/query/variables_spec.rb
index 0f73ed63ff..3e736dbd35 100644
--- a/spec/integration/rails/graphql/query/variables_spec.rb
+++ b/spec/integration/rails/graphql/query/variables_spec.rb
@@ -15,13 +15,17 @@
   }
   |}
   let(:ast_variables) { GraphQL.parse(query_string).definitions.first.variables }
-  let(:schema) { Dummy::Schema }
+  let(:schema) {
+    Class.new(Dummy::Schema) {
+      # This tests coercion behavior which only happens when `.interpreter?` is false
+      use GraphQL::Execution::Execute
+      use GraphQL::Analysis
+    }
+  }
+  let(:query_context) { GraphQL::Query.new(schema, "{ __typename }").context }
   let(:variables) {
     GraphQL::Query::Variables.new(
-    OpenStruct.new({
-      schema: schema,
-      warden: GraphQL::Schema::Warden.new(schema.default_filter, schema: schema, context: nil),
-    }),
+    query_context,
     ast_variables,
     provided_variables)
   }
@@ -272,10 +276,7 @@ class << self
       let(:run_query) { schema.execute(query_string, variables: provided_variables) }
 
       let(:variables) { GraphQL::Query::Variables.new(
-        OpenStruct.new({
-          schema: schema,
-          warden: GraphQL::Schema::Warden.new(schema.default_filter, schema: schema, context: nil),
-        }),
+        query_context,
         ast_variables,
         provided_variables)
       }

From 7511684d6337a64b38e332d931c7f2dbdf59836a Mon Sep 17 00:00:00 2001
From: Robert Mosolgo <rdmosolgo@gmail.com>
Date: Fri, 5 Feb 2021 16:39:03 -0500
Subject: [PATCH 09/12] Update variables spec for new dependency on
 context.dataloader.append_job

---
 .../rails/graphql/query/variables_spec.rb          | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/spec/integration/rails/graphql/query/variables_spec.rb b/spec/integration/rails/graphql/query/variables_spec.rb
index 3e736dbd35..9a58883874 100644
--- a/spec/integration/rails/graphql/query/variables_spec.rb
+++ b/spec/integration/rails/graphql/query/variables_spec.rb
@@ -225,8 +225,22 @@ def variables_test(ast_node:, **args)
           class << self
             attr_accessor :args_cache
           end
+          # Work around the fact that:
+          # - These tests check for `!intepreter?` behavior
+          # - Previously, these tests used an OpenStruct instead of a real context
+          # - But, now, the context requires `.dataloader.append_job`
+          # - At first I used NullContext, but that doesn't have the `.schema` reference that Variables needs.
+          # - So I built a _real_ context, but that started returning `.interpreter? => true`
+          # - So, update the schema to _really_ be `.interpreter? => false`
+          # When the legacy runtime is removed, so can the tests for that behavior (and the behavior itself.)
+          def args_cache
+            self.class.args_cache
+          end
 
           self.args_cache = args_cache
+          # This tests some coercion behavior that only applies to the legacy runtime:
+          use GraphQL::Execution::Execute
+          use GraphQL::Analysis
         end
       }
 

From 30b98eac89278bf70377bdbebfac57d7a2ea54d3 Mon Sep 17 00:00:00 2001
From: Robert Mosolgo <rdmosolgo@gmail.com>
Date: Fri, 5 Feb 2021 22:57:31 -0500
Subject: [PATCH 10/12] return a nested lazy if there is one, fixes #3314

---
 lib/graphql/execution/interpreter/resolve.rb | 21 ++++---
 lib/graphql/execution/interpreter/runtime.rb |  5 +-
 spec/graphql/dataloader_spec.rb              | 63 ++++++++++++++++++++
 3 files changed, 76 insertions(+), 13 deletions(-)

diff --git a/lib/graphql/execution/interpreter/resolve.rb b/lib/graphql/execution/interpreter/resolve.rb
index 133b50e3f9..4544a9b29c 100644
--- a/lib/graphql/execution/interpreter/resolve.rb
+++ b/lib/graphql/execution/interpreter/resolve.rb
@@ -41,17 +41,16 @@ def self.resolve(results, dataloader)
               results.concat(result_value)
               next
             elsif result_value.is_a?(Lazy)
-              result_value = result_value.value
-            end
-
-            if result_value.is_a?(Lazy)
-              # Since this field returned another lazy,
-              # add it to the same queue
-              results << result_value
-            elsif result_value.is_a?(Hash) || result_value.is_a?(Array)
-              # Add these values in wholesale --
-              # they might be modified by later work in the dataloader.
-              next_results << result_value
+              loaded_value = result_value.value
+              if loaded_value.is_a?(Lazy)
+                # Since this field returned another lazy,
+                # add it to the same queue
+                results << loaded_value
+              elsif loaded_value.is_a?(Hash) || loaded_value.is_a?(Array)
+                # Add these values in wholesale --
+                # they might be modified by later work in the dataloader.
+                next_results << loaded_value
+              end
             end
           end
 
diff --git a/lib/graphql/execution/interpreter/runtime.rb b/lib/graphql/execution/interpreter/runtime.rb
index 87d2e845ea..44f0d904a4 100644
--- a/lib/graphql/execution/interpreter/runtime.rb
+++ b/lib/graphql/execution/interpreter/runtime.rb
@@ -291,9 +291,10 @@ def evaluate_selection_with_args(kwarg_arguments, field_defn, next_path, ast_nod
             # (Subselections of this mutation will still be resolved level-by-level.)
             if is_eager_field
               Interpreter::Resolve.resolve_all([field_result], @dataloader)
+            else
+              # Return this from `after_lazy` because it might be another lazy that needs to be resolved
+              field_result
             end
-
-            nil
           end
         end
 
diff --git a/spec/graphql/dataloader_spec.rb b/spec/graphql/dataloader_spec.rb
index ee32f6b107..4ad73a2b4f 100644
--- a/spec/graphql/dataloader_spec.rb
+++ b/spec/graphql/dataloader_spec.rb
@@ -559,4 +559,67 @@ def result
     assert_equal expected_data, res2["data"]
     assert_equal expected_log, database_log
   end
+
+
+  describe "example from #3314" do
+    module Example
+      class FooType < GraphQL::Schema::Object
+        field :id, ID, null: false
+      end
+
+      class FooSource < GraphQL::Dataloader::Source
+        def fetch(ids)
+          ids.map { OpenStruct.new(id: _1) }
+        end
+      end
+
+      class QueryType < GraphQL::Schema::Object
+        field :foo, Example::FooType, null: true do
+          argument :foo_id, GraphQL::Types::ID, required: false, loads: Example::FooType
+          argument :use_load, GraphQL::Types::Boolean, required: false, default_value: false
+        end
+
+        def foo(use_load: false, foo: nil)
+          if use_load
+            dataloader.with(Example::FooSource).load("load")
+          else
+            dataloader.with(Example::FooSource).request("request")
+          end
+        end
+      end
+
+      class Schema < GraphQL::Schema
+        query Example::QueryType
+        use GraphQL::Dataloader
+
+        def self.object_from_id(id, ctx)
+          ctx.dataloader.with(Example::FooSource).request(id)
+        end
+      end
+    end
+
+    it "loads properly" do
+      result = Example::Schema.execute(<<~GRAPHQL)
+      {
+        foo(useLoad: false, fooId: "Other") {
+          __typename
+          id
+        }
+        fooWithLoad: foo(useLoad: true, fooId: "Other") {
+          __typename
+          id
+        }
+      }
+      GRAPHQL
+      # This should not have a Lazy in it
+      expected_result = {
+        "data" => {
+          "foo" => { "id" => "request", "__typename" => "Foo" },
+          "fooWithLoad" => { "id" => "load", "__typename" => "Foo" },
+        }
+      }
+
+      assert_equal expected_result, result.to_h
+    end
+  end
 end

From e990af989c23a15efa87ab93366e7312470677ed Mon Sep 17 00:00:00 2001
From: Robert Mosolgo <rdmosolgo@gmail.com>
Date: Fri, 5 Feb 2021 22:59:18 -0500
Subject: [PATCH 11/12] fix example for old rubies

---
 spec/graphql/dataloader_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/graphql/dataloader_spec.rb b/spec/graphql/dataloader_spec.rb
index 4ad73a2b4f..c1ce7b8a3d 100644
--- a/spec/graphql/dataloader_spec.rb
+++ b/spec/graphql/dataloader_spec.rb
@@ -569,7 +569,7 @@ class FooType < GraphQL::Schema::Object
 
       class FooSource < GraphQL::Dataloader::Source
         def fetch(ids)
-          ids.map { OpenStruct.new(id: _1) }
+          ids.map { |id| OpenStruct.new(id: id) }
         end
       end
 

From bfdac33b031ff5cb3587eaa4193e5c05545a3f24 Mon Sep 17 00:00:00 2001
From: Robert Mosolgo <rdmosolgo@gmail.com>
Date: Sat, 6 Feb 2021 09:05:01 -0500
Subject: [PATCH 12/12] Fix lint error

---
 spec/graphql/dataloader_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/graphql/dataloader_spec.rb b/spec/graphql/dataloader_spec.rb
index d8738d19bc..0548515801 100644
--- a/spec/graphql/dataloader_spec.rb
+++ b/spec/graphql/dataloader_spec.rb
@@ -599,7 +599,7 @@ def self.object_from_id(id, ctx)
     end
 
     it "loads properly" do
-      result = Example::Schema.execute(<<~GRAPHQL)
+      result = Example::Schema.execute(<<-GRAPHQL)
       {
         foo(useLoad: false, fooId: "Other") {
           __typename