From 3773787b26c588d8d391c5599b2bd1d9aa3020aa Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Tue, 26 May 2026 14:14:30 -0400 Subject: [PATCH 1/8] Update tests for exec-next --- spec/graphql/execution/interpreter_spec.rb | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/spec/graphql/execution/interpreter_spec.rb b/spec/graphql/execution/interpreter_spec.rb index 0a3c629d08..75db7c881b 100644 --- a/spec/graphql/execution/interpreter_spec.rb +++ b/spec/graphql/execution/interpreter_spec.rb @@ -747,6 +747,7 @@ class Thing < GraphQL::Schema::Object class Query < GraphQL::Schema::Object field :things, Thing.connection_type, null: false, resolve_static: true + field :other_things, Thing.connection_type, null: false, resolve_static: :things def self.things(context) [{title: "a"}, {title: "b"}, {title: "c"}] @@ -756,6 +757,10 @@ def things self.class.things(context) end + def other_things + self.class.things(context) + end + field :thing, Thing, null: false, resolve_static: true def self.thing(context) @@ -775,18 +780,24 @@ class Schema < GraphQL::Schema end end + it "works on different branches" do + res = ConnectionErrorTest::Schema.execute("{ things { nodes { title } } otherThings { nodes { title } } }") + assert_equal [["things", "nodes", 0, "title"], ["otherThings", "nodes", 0, "title"]], res["errors"].map { |e| e["path"] } + assert_equal 2, res.context[:authorized_calls] + end + + focus it "returns only 1 error and stops resolving fields after that" do res = ConnectionErrorTest::Schema.execute("{ things { nodes { title } } }") - assert_equal 1, res["errors"].size + assert_equal [["things", "nodes", 0, "title"]], res["errors"].map { |e| e["path"] } assert_equal 1, res.context[:authorized_calls] res = ConnectionErrorTest::Schema.execute("{ things { edges { node { title } } } }") - assert_equal 1, res["errors"].size + assert_equal [["things", "edges", 0, "node", "title"]], res["errors"].map { |e| e["path"] } assert_equal 1, res.context[:authorized_calls] res = ConnectionErrorTest::Schema.execute("{ thing { title body } }") - exec_next_TODO "should abort other branches in this case" - assert_equal 1, res["errors"].size + assert_equal [["thing", "title"]], res["errors"].map { |e| e["path"] } assert_equal 1, res.context[:authorized_calls] end end From 787eb7908eb00401e956efb780bf861f7275cdb7 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Mon, 22 Jun 2026 12:09:07 -0400 Subject: [PATCH 2/8] Improve result -> error mapping, add eager null propagation termination --- lib/graphql/execution/field_resolve_step.rb | 84 +++++++++++++++++--- lib/graphql/execution/finalize.rb | 18 +++-- lib/graphql/execution/prepare_object_step.rb | 12 ++- lib/graphql/execution/runner.rb | 12 ++- lib/graphql/execution/selections_step.rb | 8 +- lib/graphql/execution_error.rb | 4 + spec/graphql/execution/interpreter_spec.rb | 1 - 7 files changed, 110 insertions(+), 29 deletions(-) diff --git a/lib/graphql/execution/field_resolve_step.rb b/lib/graphql/execution/field_resolve_step.rb index eef8e8db85..96756b5e2b 100644 --- a/lib/graphql/execution/field_resolve_step.rb +++ b/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,69 @@ def call set_current_field(nil) end - def add_graphql_error(err) + def add_graphql_error(result, key, err) err.path = path if err.ast_node.nil? err.ast_nodes = ast_nodes end - @selections_step.query.context.add_error(err) + errs = @runner.error_results[result] ||= {} + errs[key] = err + if !err.is_a?(GraphQL::Execution::Skip) + field_type = @field_definition.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_nulled_depth == 0 + # Actually everything should be killed here + raise "TODO depth of zero" + elsif 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] if errors + raise "TODO this is broken" errors.each do |e| add_graphql_error(e) end end - @results ||= @selections_step.results + @results = [@selections_step.results.first] build_results end @@ -226,7 +274,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 +379,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 +506,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 +521,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) else field_result.path = path @runner.add_finalizer(ctx.query, result_h, key, field_result) @@ -484,7 +532,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 +578,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 +591,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 +612,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 +634,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) diff --git a/lib/graphql/execution/finalize.rb b/lib/graphql/execution/finalize.rb index f9e3d607b0..dde4a059ce 100644 --- a/lib/graphql/execution/finalize.rb +++ b/lib/graphql/execution/finalize.rb @@ -22,6 +22,11 @@ def initialize(query, data, runner) end end + runner.error_results.each do |result, error_h| + @finalizers[result] = error_h + @finalizers_count += error_h.size + end + query.context.errors.each do |err| err_path = err.path - @current_exec_path key = err_path.pop @@ -33,14 +38,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 +176,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 diff --git a/lib/graphql/execution/prepare_object_step.rb b/lib/graphql/execution/prepare_object_step.rb index c566fdb7ee..59ac39fef4 100644 --- a/lib/graphql/execution/prepare_object_step.rb +++ b/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 diff --git a/lib/graphql/execution/runner.rb b/lib/graphql/execution/runner.rb index bdb5078779..5f6eecc5ca 100644 --- a/lib/graphql/execution/runner.rb +++ b/lib/graphql/execution/runner.rb @@ -35,6 +35,8 @@ def initialize(multiplex) @authorizes_cache = Hash.new do |h, query_context| h[query_context] = {}.compare_by_identity end.compare_by_identity + + @error_results = {}.compare_by_identity end attr_reader :runtime_directives, :uses_runtime_directives, :finalizer_keys @@ -53,7 +55,7 @@ def add_step(step) @dataloader.append_job(step) end - attr_reader :steps_queue, :schema, :variables, :dataloader, :resolves_lazies, :authorizes, :static_type_at, :runtime_type_at, :finalizers, :input_values + attr_reader :steps_queue, :schema, :variables, :dataloader, :resolves_lazies, :authorizes, :static_type_at, :runtime_type_at, :finalizers, :input_values, :error_results # @return [void] def add_finalizer(query, result_value, key, finalizer) @@ -128,7 +130,7 @@ def execute queries.each_with_index.map do |query, idx| result = results[idx] - fin_result = if (!@finalizers&.key?(query) && query.context.errors.empty?) || !query.valid? + fin_result = if (!@finalizers&.key?(query) && query.context.errors.empty? && @error_results.empty?) || !query.valid? result else if result @@ -298,6 +300,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 +320,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 +336,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 +360,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 +378,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 +427,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, diff --git a/lib/graphql/execution/selections_step.rb b/lib/graphql/execution/selections_step.rb index 800b413227..4999593818 100644 --- a/lib/graphql/execution/selections_step.rb +++ b/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| diff --git a/lib/graphql/execution_error.rb b/lib/graphql/execution_error.rb index 4ccbb001ef..b79d05fc94 100644 --- a/lib/graphql/execution_error.rb +++ b/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) diff --git a/spec/graphql/execution/interpreter_spec.rb b/spec/graphql/execution/interpreter_spec.rb index 75db7c881b..80eba984b8 100644 --- a/spec/graphql/execution/interpreter_spec.rb +++ b/spec/graphql/execution/interpreter_spec.rb @@ -786,7 +786,6 @@ class Schema < GraphQL::Schema assert_equal 2, res.context[:authorized_calls] end - focus it "returns only 1 error and stops resolving fields after that" do res = ConnectionErrorTest::Schema.execute("{ things { nodes { title } } }") assert_equal [["things", "nodes", 0, "title"]], res["errors"].map { |e| e["path"] } From f6f853dc1717718104bed534178cca64feb3063a Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Mon, 22 Jun 2026 13:24:24 -0400 Subject: [PATCH 3/8] Fix multiple error codepath --- lib/graphql/execution/field_resolve_step.rb | 5 ++--- spec/graphql/schema/validator_spec.rb | 6 ++++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/graphql/execution/field_resolve_step.rb b/lib/graphql/execution/field_resolve_step.rb index 96756b5e2b..776f050175 100644 --- a/lib/graphql/execution/field_resolve_step.rb +++ b/lib/graphql/execution/field_resolve_step.rb @@ -161,13 +161,12 @@ def propagate_nulls def build_errors_result(errors, single_error) first_error = errors.nil? ? single_error : errors.pop @field_results = [first_error] + @results = [@selections_step.results.first] if errors - raise "TODO this is broken" errors.each do |e| - add_graphql_error(e) + add_graphql_error(@results.first, key, e) end end - @results = [@selections_step.results.first] build_results end diff --git a/spec/graphql/schema/validator_spec.rb b/spec/graphql/schema/validator_spec.rb index 9d5385401a..be8d9c57d5 100644 --- a/spec/graphql/schema/validator_spec.rb +++ b/spec/graphql/schema/validator_spec.rb @@ -141,11 +141,13 @@ def validate(obj, ctx, value) { "validated" => nil }, ] }, - "errors" => [ + "errors" => if_exec_next([ + {"message"=>"value must be greater than 5", "locations"=>[{"line"=>1, "column"=>10}], "path"=>["list", 0, "validated"]}, + ],[ {"message"=>"value must be greater than 5", "locations"=>[{"line"=>1, "column"=>10}], "path"=>["list", 0, "validated"]}, {"message"=>"value must be greater than 5", "locations"=>[{"line"=>1, "column"=>10}], "path"=>["list", 1, "validated"]}, {"message"=>"value must be greater than 5", "locations"=>[{"line"=>1, "column"=>10}], "path"=>["list", 2, "validated"]}, - ] + ]) } assert_equal expected_response, res end From 5d025725cdbbc37f9ef8aadab565723cb388fb9e Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Mon, 22 Jun 2026 13:39:43 -0400 Subject: [PATCH 4/8] Better support for multiple errors, per-query lists of error responses --- lib/graphql/execution/field_resolve_step.rb | 18 +++++++++++++----- lib/graphql/execution/finalize.rb | 8 +++++--- lib/graphql/execution/runner.rb | 6 ++++-- spec/graphql/authorization_spec.rb | 6 +----- 4 files changed, 23 insertions(+), 15 deletions(-) diff --git a/lib/graphql/execution/field_resolve_step.rb b/lib/graphql/execution/field_resolve_step.rb index 776f050175..cc9f6f6475 100644 --- a/lib/graphql/execution/field_resolve_step.rb +++ b/lib/graphql/execution/field_resolve_step.rb @@ -105,15 +105,23 @@ def call set_current_field(nil) end - def add_graphql_error(result, key, 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 - errs = @runner.error_results[result] ||= {} - errs[key] = err + errs = @runner.error_results[@selections_step.query][result] ||= {}.compare_by_identity + if (existing_error = errs[key]) + if existing_error.is_a?(Array) + existing_error << err + else + errs[key] = [existing_error, err] + end + else + errs[key] = err + end if !err.is_a?(GraphQL::Execution::Skip) - field_type = @field_definition.type + 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 @@ -520,7 +528,7 @@ def build_leaf_result(result_h, result_key, field_result, return_type, ctx, is_f end elsif field_result.is_a?(Finalizer) if field_result.is_a?(GraphQL::RuntimeError) - add_graphql_error(result_h, result_key, 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) diff --git a/lib/graphql/execution/finalize.rb b/lib/graphql/execution/finalize.rb index dde4a059ce..e3410b06d1 100644 --- a/lib/graphql/execution/finalize.rb +++ b/lib/graphql/execution/finalize.rb @@ -22,9 +22,11 @@ def initialize(query, data, runner) end end - runner.error_results.each do |result, error_h| - @finalizers[result] = error_h - @finalizers_count += error_h.size + if !(error_results = runner.error_results[query]).empty? + error_results.each do |result, error_h| + @finalizers[result] = error_h + @finalizers_count += error_h.size + end end query.context.errors.each do |err| diff --git a/lib/graphql/execution/runner.rb b/lib/graphql/execution/runner.rb index 5f6eecc5ca..dfef856b3e 100644 --- a/lib/graphql/execution/runner.rb +++ b/lib/graphql/execution/runner.rb @@ -36,7 +36,9 @@ def initialize(multiplex) h[query_context] = {}.compare_by_identity end.compare_by_identity - @error_results = {}.compare_by_identity + @error_results = Hash.new do |h, query| + h[query] = {}.compare_by_identity + end.compare_by_identity end attr_reader :runtime_directives, :uses_runtime_directives, :finalizer_keys @@ -130,7 +132,7 @@ def execute queries.each_with_index.map do |query, idx| result = results[idx] - fin_result = if (!@finalizers&.key?(query) && query.context.errors.empty? && @error_results.empty?) || !query.valid? + fin_result = if (!@finalizers&.key?(query) && query.context.errors.empty? && @error_results[query].empty?) || !query.valid? result else if result diff --git a/spec/graphql/authorization_spec.rb b/spec/graphql/authorization_spec.rb index 138d693500..985fd00d21 100644 --- a/spec/graphql/authorization_spec.rb +++ b/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 From c0078bc10cee7ae9c0f643c23b9599c37e16f72b Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Mon, 22 Jun 2026 13:55:00 -0400 Subject: [PATCH 5/8] Merge finalizers at key --- lib/graphql/execution/finalize.rb | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/lib/graphql/execution/finalize.rb b/lib/graphql/execution/finalize.rb index e3410b06d1..9100a47b8c 100644 --- a/lib/graphql/execution/finalize.rb +++ b/lib/graphql/execution/finalize.rb @@ -24,7 +24,21 @@ def initialize(query, data, runner) if !(error_results = runner.error_results[query]).empty? error_results.each do |result, error_h| - @finalizers[result] = error_h + if (prev_f = @finalizers[result]) + error_h.each do |k, v| + if (prev_k_f = prev_f[k]) + if prev_k_f.is_a?(Array) + prev_k_f << v + else + prev_f[k] = [prev_k_f, v] + end + else + prev_f[k] = v + end + end + else + @finalizers[result] = error_h + end @finalizers_count += error_h.size end end From a73bba5ddb9336ec97f7aa9f0f7a0de47c97f6b6 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Mon, 22 Jun 2026 14:26:23 -0400 Subject: [PATCH 6/8] Update tests for non-null branches with null propagation --- spec/graphql/execution/interpreter_spec.rb | 26 ++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/spec/graphql/execution/interpreter_spec.rb b/spec/graphql/execution/interpreter_spec.rb index 80eba984b8..2cb13b359e 100644 --- a/spec/graphql/execution/interpreter_spec.rb +++ b/spec/graphql/execution/interpreter_spec.rb @@ -746,8 +746,10 @@ class Thing < GraphQL::Schema::Object end class Query < GraphQL::Schema::Object - field :things, Thing.connection_type, null: false, resolve_static: true - field :other_things, Thing.connection_type, null: false, resolve_static: :things + field :things, Thing.connection_type, resolve_static: true + field :other_things, Thing.connection_type, resolve_static: :things + field :non_null_things, Thing.connection_type, null: false, resolve_static: :things + field :non_null_other_things, Thing.connection_type, null: false, resolve_static: :things def self.things(context) [{title: "a"}, {title: "b"}, {title: "c"}] @@ -761,6 +763,14 @@ def other_things self.class.things(context) end + def non_null_other_things + self.class.things(context) + end + + def non_null_things + self.class.things(context) + end + field :thing, Thing, null: false, resolve_static: true def self.thing(context) @@ -786,6 +796,18 @@ class Schema < GraphQL::Schema assert_equal 2, res.context[:authorized_calls] end + it "Does non-null propagation across branches" do + res = ConnectionErrorTest::Schema.execute("{ nonNullThings { nodes { title } } nonNullOtherThings { nodes { title } } }") + expected_error_paths = if_exec_next([ + ["nonNullThings", "nodes", 0, "title"] + ], [ + ["nonNullThings", "nodes", 0, "title"], + ["nonNullOtherThings", "nodes", 0, "title"] + ]) + assert_equal expected_error_paths, res["errors"].map { |e| e["path"] } + assert_equal if_exec_next(1, 2), res.context[:authorized_calls] + end + it "returns only 1 error and stops resolving fields after that" do res = ConnectionErrorTest::Schema.execute("{ things { nodes { title } } }") assert_equal [["things", "nodes", 0, "title"]], res["errors"].map { |e| e["path"] } From def44a40f8bc698e092a8ee132d96c72f4820c39 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Tue, 23 Jun 2026 06:08:14 -0400 Subject: [PATCH 7/8] use finalizers instead of error_results --- lib/graphql/execution/field_resolve_step.rb | 11 +---------- lib/graphql/execution/finalize.rb | 21 --------------------- lib/graphql/execution/runner.rb | 8 ++------ 3 files changed, 3 insertions(+), 37 deletions(-) diff --git a/lib/graphql/execution/field_resolve_step.rb b/lib/graphql/execution/field_resolve_step.rb index cc9f6f6475..8b17e688ab 100644 --- a/lib/graphql/execution/field_resolve_step.rb +++ b/lib/graphql/execution/field_resolve_step.rb @@ -110,16 +110,7 @@ def add_graphql_error(result, key, err, return_type: @field_definition.type) if err.ast_node.nil? err.ast_nodes = ast_nodes end - errs = @runner.error_results[@selections_step.query][result] ||= {}.compare_by_identity - if (existing_error = errs[key]) - if existing_error.is_a?(Array) - existing_error << err - else - errs[key] = [existing_error, err] - end - else - errs[key] = err - end + @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? diff --git a/lib/graphql/execution/finalize.rb b/lib/graphql/execution/finalize.rb index 9100a47b8c..850b000168 100644 --- a/lib/graphql/execution/finalize.rb +++ b/lib/graphql/execution/finalize.rb @@ -22,27 +22,6 @@ def initialize(query, data, runner) end end - if !(error_results = runner.error_results[query]).empty? - error_results.each do |result, error_h| - if (prev_f = @finalizers[result]) - error_h.each do |k, v| - if (prev_k_f = prev_f[k]) - if prev_k_f.is_a?(Array) - prev_k_f << v - else - prev_f[k] = [prev_k_f, v] - end - else - prev_f[k] = v - end - end - else - @finalizers[result] = error_h - end - @finalizers_count += error_h.size - end - end - query.context.errors.each do |err| err_path = err.path - @current_exec_path key = err_path.pop diff --git a/lib/graphql/execution/runner.rb b/lib/graphql/execution/runner.rb index dfef856b3e..1a40003c7a 100644 --- a/lib/graphql/execution/runner.rb +++ b/lib/graphql/execution/runner.rb @@ -35,10 +35,6 @@ def initialize(multiplex) @authorizes_cache = Hash.new do |h, query_context| h[query_context] = {}.compare_by_identity end.compare_by_identity - - @error_results = Hash.new do |h, query| - h[query] = {}.compare_by_identity - end.compare_by_identity end attr_reader :runtime_directives, :uses_runtime_directives, :finalizer_keys @@ -57,7 +53,7 @@ def add_step(step) @dataloader.append_job(step) end - attr_reader :steps_queue, :schema, :variables, :dataloader, :resolves_lazies, :authorizes, :static_type_at, :runtime_type_at, :finalizers, :input_values, :error_results + attr_reader :steps_queue, :schema, :variables, :dataloader, :resolves_lazies, :authorizes, :static_type_at, :runtime_type_at, :finalizers, :input_values # @return [void] def add_finalizer(query, result_value, key, finalizer) @@ -132,7 +128,7 @@ def execute queries.each_with_index.map do |query, idx| result = results[idx] - fin_result = if (!@finalizers&.key?(query) && query.context.errors.empty? && @error_results[query].empty?) || !query.valid? + fin_result = if (!@finalizers&.key?(query) && query.context.errors.empty?) || !query.valid? result else if result From 6481932c5560ad04ad3050de0da2d40013896b6f Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Tue, 23 Jun 2026 06:20:21 -0400 Subject: [PATCH 8/8] Remove top-level abort condition --- lib/graphql/execution/field_resolve_step.rb | 5 +---- spec/graphql/execution/interpreter_spec.rb | 2 ++ 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/graphql/execution/field_resolve_step.rb b/lib/graphql/execution/field_resolve_step.rb index 8b17e688ab..06284bffcd 100644 --- a/lib/graphql/execution/field_resolve_step.rb +++ b/lib/graphql/execution/field_resolve_step.rb @@ -145,10 +145,7 @@ def propagate_nulls current_field_step = current_field_step.selections_step.field_resolve_step end - if highest_nulled_depth == 0 - # Actually everything should be killed here - raise "TODO depth of zero" - elsif highest_list_depth.nil? || highest_nulled_depth <= highest_list_depth + 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 diff --git a/spec/graphql/execution/interpreter_spec.rb b/spec/graphql/execution/interpreter_spec.rb index 2cb13b359e..686f090507 100644 --- a/spec/graphql/execution/interpreter_spec.rb +++ b/spec/graphql/execution/interpreter_spec.rb @@ -792,12 +792,14 @@ class Schema < GraphQL::Schema it "works on different branches" do res = ConnectionErrorTest::Schema.execute("{ things { nodes { title } } otherThings { nodes { title } } }") + assert_equal({ "things" => nil, "otherThings" => nil }, res["data"]) assert_equal [["things", "nodes", 0, "title"], ["otherThings", "nodes", 0, "title"]], res["errors"].map { |e| e["path"] } assert_equal 2, res.context[:authorized_calls] end it "Does non-null propagation across branches" do res = ConnectionErrorTest::Schema.execute("{ nonNullThings { nodes { title } } nonNullOtherThings { nodes { title } } }") + assert_nil res.fetch("data") expected_error_paths = if_exec_next([ ["nonNullThings", "nodes", 0, "title"] ], [