Exec-next: improve null propagation errors

lib/graphql/execution/field_resolve_step.rb

@@ -49,6 +49,7 @@ def append_selection(ast_node)
       end
 
       def value
+        return nil if @selections_step.killed
         query = @selections_step.query
         set_current_field
         query.current_trace.begin_execute_field(@field_definition, @arguments, @field_results, query)
@@ -79,7 +80,9 @@ def sync(lazy)
       end
 
       def call
+        return nil if @selections_step.killed
         set_current_field if @field_definition
+
         if @enqueued_authorization
           enqueue_next_steps
         elsif @finish_extension_idx
@@ -102,24 +105,64 @@ def call
         set_current_field(nil)
       end
 
-      def add_graphql_error(err)
+      def add_graphql_error(result, key, err, return_type: @field_definition.type)
         err.path = path
         if err.ast_node.nil?
           err.ast_nodes = ast_nodes
         end
-        @selections_step.query.context.add_error(err)
+        @runner.add_finalizer(@selections_step.query, result, key, err)
+        if !err.is_a?(GraphQL::Execution::Skip)
+          field_type = return_type
+          should_propagate_null = field_type.non_null?
+          while (should_propagate_null == false && field_type.kind.wraps?)
+            field_type = field_type.of_type
+            should_propagate_null = field_type.non_null?
+          end
+          if should_propagate_null
+            propagate_nulls
+          end
+        end
         err
       end
 
+      def propagate_nulls
+        propagating_null = true
+        highest_nulled_depth = path.size
+        highest_list_depth = nil
+        current_field_step = self
+        while current_field_step
+          return_type = current_field_step.field_definition.type
+          if propagating_null && return_type.non_null?
+            highest_nulled_depth = current_field_step.path.size
+          else
+            propagating_null = false
+          end
+
+          if return_type.list?
+            highest_list_depth = current_field_step.path.size
+          end
+
+          current_field_step = current_field_step.selections_step.field_resolve_step
+        end
+
+        if highest_list_depth.nil? || highest_nulled_depth <= highest_list_depth
+          kill_field_step = self
+          while kill_field_step && highest_nulled_depth <= kill_field_step.path.size
+            kill_field_step.selections_step.killed = true
+            kill_field_step = kill_field_step.selections_step.field_resolve_step
+          end
+        end
+      end
+
       def build_errors_result(errors, single_error)
         first_error = errors.nil? ? single_error : errors.pop
-        @field_results = error_instance_array(@selections_step.objects.size, first_error)
+        @field_results = [first_error]
+        @results = [@selections_step.results.first]
         if errors
           errors.each do |e|
-            add_graphql_error(e)
+            add_graphql_error(@results.first, key, e)
           end
         end
-        @results ||= @selections_step.results
         build_results
       end
 
@@ -226,7 +269,7 @@ def execute_field
                   authorized_results << @results[i]
                 end
               rescue GraphQL::ExecutionError => exec_err
-                add_graphql_error(exec_err)
+                add_graphql_error(@results[i], key, exec_err)
               end
             end
             i += 1
@@ -331,12 +374,12 @@ def execute_field
           end
         end
       rescue GraphQL::ExecutionError => err
-        add_graphql_error(err)
+        build_errors_result(nil, err)
       rescue StandardError => stderr
         begin
           @selections_step.query.handle_or_reraise(stderr, field: @field_definition, arguments: @arguments, object: nil)
         rescue GraphQL::ExecutionError => err
-          add_graphql_error(err)
+          add_graphql_error(@results[0], key, err)
         end
       end
 
@@ -458,13 +501,13 @@ def build_results
       end
 
       def finish_leaf_result(result_h, key, field_result, return_type, ctx)
-        final_field_result = build_leaf_result(field_result, return_type, ctx, false)
+        final_field_result = build_leaf_result(result_h, key, field_result, return_type, ctx, false)
 
         @directive_finalizers&.each { |f| @runner.add_finalizer(ctx.query, result_h, key, f) }
         result_h[@key] = final_field_result
       end
 
-      def build_leaf_result(field_result, return_type, ctx, is_from_array)
+      def build_leaf_result(result_h, result_key, field_result, return_type, ctx, is_from_array)
         if field_result.nil?
           if return_type.non_null?
             add_non_null_error(is_from_array)
@@ -473,7 +516,7 @@ def build_leaf_result(field_result, return_type, ctx, is_from_array)
           end
         elsif field_result.is_a?(Finalizer)
           if field_result.is_a?(GraphQL::RuntimeError)
-            add_graphql_error(field_result)
+            add_graphql_error(result_h, result_key, field_result, return_type: return_type)
           else
             field_result.path = path
             @runner.add_finalizer(ctx.query, result_h, key, field_result)
@@ -484,7 +527,11 @@ def build_leaf_result(field_result, return_type, ctx, is_from_array)
           end
 
           inner_type = return_type.of_type
-          field_result.map { |item| build_leaf_result(item, inner_type, ctx, true) }
+          result_a = Array.new(field_result.size)
+          field_result.each_with_index do |item, idx|
+            result_a[idx] = build_leaf_result(result_a, idx, item, inner_type, ctx, true)
+          end
+          result_a
         else
           return_type.coerce_result(field_result, ctx)
         end
@@ -526,6 +573,7 @@ def enqueue_next_steps
               query.current_trace.objects(obj_type, next_objects, ctx)
               @runner.add_step(SelectionsStep.new(
                 path: path,
+                field_resolve_step: self,
                 parent_type: obj_type,
                 selections: @next_selections,
                 objects: next_objects,
@@ -538,6 +586,7 @@ def enqueue_next_steps
             query.current_trace.objects(@static_type, @all_next_objects, ctx)
             @runner.add_step(SelectionsStep.new(
               path: path,
+              field_resolve_step: self,
               parent_type: @static_type,
               selections: @next_selections,
               objects: @all_next_objects,
@@ -558,7 +607,11 @@ def authorized_finished(step)
 
       def add_non_null_error(is_from_array)
         err = @parent_type::InvalidNullError.new(@parent_type, @field_definition, ast_nodes, is_from_array: is_from_array, path: path)
-        @runner.schema.type_error(err, @selections_step.query.context)
+        nn_result = @runner.schema.type_error(err, @selections_step.query.context)
+        if nn_result.nil?
+          propagate_nulls
+        end
+        nn_result
       end
 
       def set_current_field(new_value = @field_definition)
@@ -576,7 +629,7 @@ def build_graphql_result(graphql_result, key, field_result, return_type, is_nn,
           end
         elsif field_result.is_a?(Finalizer)
           graphql_result[key] = if field_result.is_a?(GraphQL::RuntimeError)
-            add_graphql_error(field_result)
+            add_graphql_error(graphql_result, key, field_result)
           else
             field_result.path = path
             @runner.add_finalizer(@selections_step.query, graphql_result, key, field_result)

lib/graphql/execution/finalize.rb

@@ -33,14 +33,15 @@ def initialize(query, data, runner)
 
           targets.each_with_index do |target, idx|
             if target.is_a?(Hash)
-              if target[key].equal?(err)
+              value_at_key = target[key]
+              if value_at_key.equal?(err)
                 tf = @finalizers[target] ||= {}.compare_by_identity
                 tf[key] = err
                 @finalizers_count += 1
-              elsif (arr = target[key]).is_a?(Array)
-                arr.each_with_index do |el, idx|
+              elsif value_at_key.is_a?(Array)
+                value_at_key.each_with_index do |el, idx|
                   if el.equal?(err)
-                    tf = @finalizers[arr] ||= {}.compare_by_identity
+                    tf = @finalizers[value_at_key] ||= {}.compare_by_identity
                     tf[idx] = err
                     @finalizers_count += 1
                   end
@@ -170,9 +171,7 @@ def check_object_result(result_h, parent_type, ast_selections)
       end
 
       def check_list_result(result_arr, inner_type, ast_selections)
-        inner_type_non_null = false
-        if inner_type.non_null?
-          inner_type_non_null = true
+        if (inner_type_non_null = inner_type.non_null?)
           inner_type = inner_type.of_type
         end
 

lib/graphql/execution/prepare_object_step.rb

@@ -73,6 +73,10 @@ def call
         @field_resolve_step.set_current_field(nil)
       end
 
+      def add_field_error(err)
+        @field_resolve_step.add_graphql_error(@graphql_result, @key, err)
+      end
+
       def authorize
         if @field_resolve_step.was_scoped && !@resolved_type.reauthorize_scoped_objects
           @authorized_value = @object
@@ -96,13 +100,13 @@ def authorize
           create_result
         end
       rescue GraphQL::RuntimeError => err
-        @graphql_result[@key] = @field_resolve_step.add_graphql_error(err)
+        @graphql_result[@key] = add_field_error(err)
       rescue StandardError => err
         query ||= @field_resolve_step.selections_step.query
         begin
           query.handle_or_reraise(err, field: @field_resolve_step.field_definition, arguments: @field_resolve_step.arguments, object: @object) # rubocop:disable Development/ContextIsPassedCop
         rescue GraphQL::RuntimeError => err
-          @graphql_result[@key] = @field_resolve_step.add_graphql_error(err)
+          @graphql_result[@key] = add_field_error(err)
         end
       end
 
@@ -120,13 +124,13 @@ def create_result
             elsif @is_non_null
               @graphql_result[@key] = @field_resolve_step.add_non_null_error(@is_from_array)
             else
-              @graphql_result[@key] = @field_resolve_step.add_graphql_error(@authorization_error)
+              @graphql_result[@key] = add_field_error(@authorization_error)
             end
           rescue GraphQL::RuntimeError => err
             if @is_non_null
               @graphql_result[@key] = @field_resolve_step.add_non_null_error(@is_from_array)
             else
-              @graphql_result[@key] = @field_resolve_step.add_graphql_error(err)
+              @graphql_result[@key] = add_field_error(err)
             end
           end
         end

lib/graphql/execution/runner.rb

@@ -298,6 +298,7 @@ def begin_execute(isolated_steps, results, query, root_type, root_value)
           if query.query?
             isolated_steps[0] << SelectionsStep.new(
               parent_type: root_type,
+              field_resolve_step: nil,
               selections: selected_operation.selections,
               objects: objects,
               results: [data],
@@ -317,6 +318,7 @@ def begin_execute(isolated_steps, results, query, root_type, root_value)
               isolated_steps << [SelectionsStep.new(
                 clobber: false, # `data` is being shared among several selections steps
                 parent_type: root_type,
+                field_resolve_step: field_resolve_step,
                 selections: field_resolve_step.ast_nodes || Array(field_resolve_step.ast_node),
                 objects: objects,
                 results: [data],
@@ -332,6 +334,7 @@ def begin_execute(isolated_steps, results, query, root_type, root_value)
             end
             isolated_steps[0] << SelectionsStep.new(
               parent_type: root_type,
+              field_resolve_step: nil,
               selections: selected_operation.selections,
               objects: objects,
               results: [data],
@@ -355,6 +358,7 @@ def begin_execute(isolated_steps, results, query, root_type, root_value)
           results << { "data" => data }
           isolated_steps[0] << SelectionsStep.new(
             parent_type: resolved_type,
+            field_resolve_step: nil,
             selections: selected_operation.selections,
             objects: objects,
             results: [data],
@@ -372,6 +376,7 @@ def begin_execute(isolated_steps, results, query, root_type, root_value)
             results << { "data" => list_result }
             isolated_steps[0] << SelectionsStep.new(
               parent_type: inner_type,
+              field_resolve_step: nil,
               selections: selected_operation.selections,
               objects: root_value,
               results: list_result,
@@ -420,6 +425,7 @@ def run_isolated_scalar(type, partial)
         selections = partial.ast_nodes
         dummy_ss = SelectionsStep.new(
           parent_type: nil,
+          field_resolve_step: nil,
           selections: selections,
           objects: nil,
           results: nil,

lib/graphql/execution/selections_step.rb

@@ -2,8 +2,9 @@
 module GraphQL
   module Execution
     class SelectionsStep
-      def initialize(parent_type:, selections:, objects:, results:, runner:, query:, path:, clobber: true)
+      def initialize(parent_type:, field_resolve_step:, selections:, objects:, results:, runner:, query:, path:, clobber: true)
         @path = path
+        @field_resolve_step = field_resolve_step
         @parent_type = parent_type
         @selections = selections
         @runner = runner
@@ -12,10 +13,13 @@ def initialize(parent_type:, selections:, objects:, results:, runner:, query:, p
         @query = query
         @graphql_objects = nil
         @all_selections = nil
+        @killed = false
         @clobber = clobber
       end
 
-      attr_reader :path, :query, :objects, :results
+      attr_reader :path, :query, :objects, :results, :field_resolve_step
+
+      attr_accessor :killed
 
       def graphql_objects
         @graphql_objects ||= @objects.map do |obj|

lib/graphql/execution_error.rb

@@ -26,6 +26,10 @@ def initialize(message, ast_node: nil, ast_nodes: nil, options: nil, extensions:
     end
 
     def finalize_graphql_result(query, result_data, key)
+      add_this_error = !query.context.errors.any? { |e| e.eql?(self) }
+      if add_this_error
+        query.context.add_error(self)
+      end
       if ast_node.is_a?(GraphQL::Language::Nodes::Directive)
         # This is for backwards compatibility ... what does the spec say?
         result_data.delete(key)

spec/graphql/authorization_spec.rb

@@ -620,11 +620,7 @@ def auth_execute(*args, **kwargs)
     it "halts on unauthorized mutations" do
       query = "mutation { doUnauthorizedStuff(input: {}) { __typename } }"
       res = auth_execute(query, context: { unauthorized_mutation: true })
-      if TESTING_EXEC_NEXT
-        assert_equal({}, res["data"])
-      else
-        assert_nil res["data"].fetch("doUnauthorizedStuff")
-      end
+      assert_nil res["data"].fetch("doUnauthorizedStuff")
       assert_raises GraphQL::RequiredImplementationMissingError do
         auth_execute(query)
       end