Implement ability to skip over list items if their field resolution raises an error

lib/graphql/execution.rb

@@ -14,5 +14,12 @@ class Skip < GraphQL::Error; end
     # Just a singleton for implementing {Query::Context#skip}
     # @api private
     SKIP = Skip.new
+
+    # @api private
+    class SkipFromParentList < GraphQL::Error; end
+
+    # Just a singleton for implementing {Query::Context#skip}
+    # @api private
+    SKIP_FROM_PARENT_LIST = SkipFromParentList.new
   end
 end

lib/graphql/execution/interpreter/runtime.rb

@@ -22,7 +22,12 @@ def initialize
         end
 
         module GraphQLResult
-          def initialize(result_name, parent_result, is_non_null_in_parent)
+          # FIXME: storing `represented_value` here holds onto a lot of objects, keeping them from GCing and increasing peak memory.
+          #   ... but the value is only needed for lists with skippable items.
+          #   Maybe we can set to `nil` unless we know this object is a skippable list item
+          #   (i.e. a direct child of a list with `skip_items_on_raise: true`)
+          # FIXME: likewise for `field`.
+          def initialize(result_name, parent_result, is_non_null_in_parent, represented_value, field)
             @graphql_parent = parent_result
             if parent_result && parent_result.graphql_dead
               @graphql_dead = true
@@ -31,6 +36,8 @@ def initialize(result_name, parent_result, is_non_null_in_parent)
             @graphql_is_non_null_in_parent = is_non_null_in_parent
             # Jump through some hoops to avoid creating this duplicate storage if at all possible.
             @graphql_metadata = nil
+            @represented_value = represented_value
+            @field = field
           end
 
           def path
@@ -43,14 +50,38 @@ def build_path(path_array)
           end
 
           attr_accessor :graphql_dead
-          attr_reader :graphql_parent, :graphql_result_name, :graphql_is_non_null_in_parent
+          attr_reader :graphql_parent, :graphql_result_name, :graphql_is_non_null_in_parent, :represented_value, :field
+
+          # True when this result is a List that was marked `skip_items_on_raise: true`.
+          # Descendants of this result will have `can_be_skipped?` be true.
+          # @return [nil, true]
+          attr_accessor :graphql_skip_list_items_that_raise
+
+          # A result can be skipped when it's an ancestor of an item in a List marked `skip_items_on_raise: true`.
+          # @return [Boolean]
+          def can_be_skipped?
+            !nearest_skippable_parent_result.nil?
+          end
+
+          # Crawls up the tree to find the first ancestor that can be skipped, that is, the first item that's a child
+          # of a List marked `skip_items_on_raise: true`.
+          # @return [GraphQLResult]
+          def nearest_skippable_parent_result
+            return @nearest_skippable_parent_result if defined?(@nearest_skippable_parent_result)
+
+            @nearest_skippable_parent_result = if graphql_parent && graphql_parent.graphql_skip_list_items_that_raise
+              self
+            else
+              graphql_parent && graphql_parent.nearest_skippable_parent_result
+            end
+          end
 
           # @return [Hash] Plain-Ruby result data (`@graphql_metadata` contains Result wrapper objects)
           attr_accessor :graphql_result_data
         end
 
         class GraphQLResultHash
-          def initialize(_result_name, _parent_result, _is_non_null_in_parent)
+          def initialize(_result_name, _parent_result, _is_non_null_in_parent, _represented_value, _field)
             super
             @graphql_result_data = {}
           end
@@ -138,7 +169,7 @@ def merge_into(into_result)
         class GraphQLResultArray
           include GraphQLResult
 
-          def initialize(_result_name, _parent_result, _is_non_null_in_parent)
+          def initialize(_result_name, _parent_result, _is_non_null_in_parent, _represented_value, _field)
             super
             @graphql_result_data = []
           end
@@ -198,7 +229,7 @@ def initialize(query:, lazies_at_depth:)
           @lazies_at_depth = lazies_at_depth
           @schema = query.schema
           @context = query.context
-          @response = GraphQLResultHash.new(nil, nil, false)
+          @response = GraphQLResultHash.new(nil, nil, false, query.root_value, nil)
           # Identify runtime directives by checking which of this schema's directives have overridden `def self.resolve`
           @runtime_directive_names = []
           noop_resolve_owner = GraphQL::Schema::Directive.singleton_class
@@ -265,7 +296,7 @@ def run_eager
               # directly evaluated and the results can be written right into the main response hash.
               tap_or_each(gathered_selections) do |selections, is_selection_array|
                 if is_selection_array
-                  selection_response = GraphQLResultHash.new(nil, nil, false)
+                  selection_response = GraphQLResultHash.new(nil, nil, false, query.root_value, nil)
                   final_response = @response
                 else
                   selection_response = @response
@@ -547,7 +578,20 @@ def evaluate_selection_with_resolved_keyword_args(kwarg_arguments, resolved_argu
               err
             rescue StandardError => err
               begin
-                query.handle_or_reraise(err)
+                nearest_skippable_parent_result = selection_result.nearest_skippable_parent_result
+
+                if nearest_skippable_parent_result && (on_raise_callback = nearest_skippable_parent_result.field.on_raise_callback)
+                  begin
+                    nearest_skippable_list_item = nearest_skippable_parent_result.represented_value.object
+                    on_raise_callback.call(err, object, kwarg_arguments, context, nearest_skippable_parent_result.field, nearest_skippable_list_item)
+                  rescue GraphQL::ExecutionError => err  # TODO: DRY me
+                    err
+                  rescue StandardError => err
+                    query.handle_or_reraise(err)
+                  end
+                else
+                  query.handle_or_reraise(err)
+                end
               rescue GraphQL::ExecutionError => ex_err
                 ex_err
               end
@@ -598,6 +642,33 @@ def set_result(selection_result, result_name, value, is_child_result, is_non_nul
               end
             elsif is_child_result
               selection_result.set_child_result(result_name, value)
+            elsif value == GraphQL::Execution::SKIP_FROM_PARENT_LIST
+              if selection_result.graphql_skip_list_items_that_raise # This is the first list that this item can be skipped from.
+                unless selection_result.is_a?(GraphQLResultArray)
+                  raise "Invariant: unexpected result class #{selection_result.class} (#{selection_result.inspect})"
+                end
+
+                selection_result.graphql_skip_at(result_name)
+              else # Propograte up to find the first list this item can be skipped from.
+                parent = selection_result.graphql_parent
+                name_in_parent = selection_result.graphql_result_name
+
+                set_result(parent, name_in_parent, GraphQL::Execution::SKIP_FROM_PARENT_LIST, false, is_non_null_in_parent)
+                set_graphql_dead(selection_result)
+              end
+            elsif value == GraphQL::Execution::SKIP_FROM_PARENT_LIST
+              if selection_result.graphql_skip_list_items_that_raise # This is the first list that this item can be skipped from.
+                unless selection_result.is_a?(GraphQLResultArray)
+                  raise "Invariant: unexpected result class #{selection_result.class} (#{selection_result.inspect})"
+                end
+
+                selection_result.graphql_skip_at(result_name)
+              else # Propograte up to find the first list this item can be skipped from.
+                parent = selection_result.graphql_parent
+                name_in_parent = selection_result.graphql_result_name
+                set_result(parent, name_in_parent, GraphQL::Execution::SKIP_FROM_PARENT_LIST)
+                set_graphql_dead(selection_result)
+              end
             else
               selection_result.set_leaf(result_name, value)
             end
@@ -690,6 +761,12 @@ def continue_value(value, parent_type, field, is_non_null, ast_node, result_name
                 raise "Invariant: unexpected result class #{selection_result.class} (#{selection_result.inspect})"
               end
               HALT
+            elsif GraphQL::Execution::SKIP_FROM_PARENT_LIST == value
+              unless selection_result.can_be_skipped?
+                raise "Cannot skip list items from lists not marked `skip_items_on_raise: true`"
+              end
+
+              set_result(selection_result, result_name, GraphQL::Execution::SKIP_FROM_PARENT_LIST, false, is_non_null)
             else
               # What could this actually _be_? Anyhow,
               # preserve the default behavior of doing nothing with it.
@@ -781,7 +858,7 @@ def continue_field(value, owner_type, field, current_type, ast_node, next_select
             after_lazy(object_proxy, ast_node: ast_node, field: field, owner_object: owner_object, arguments: arguments, trace: false, result_name: result_name, result: selection_result) do |inner_object|
               continue_value = continue_value(inner_object, owner_type, field, is_non_null, ast_node, result_name, selection_result)
               if HALT != continue_value
-                response_hash = GraphQLResultHash.new(result_name, selection_result, is_non_null)
+                response_hash = GraphQLResultHash.new(result_name, selection_result, is_non_null, continue_value, field)
                 set_result(selection_result, result_name, response_hash, true, is_non_null)
                 gathered_selections = gather_selections(continue_value, current_type, next_selections)
                 # There are two possibilities for `gathered_selections`:
@@ -794,7 +871,7 @@ def continue_field(value, owner_type, field, current_type, ast_node, next_select
                 #    (Technically, it's possible that one of those entries _doesn't_ require isolation.)
                 tap_or_each(gathered_selections) do |selections, is_selection_array|
                   if is_selection_array
-                    this_result = GraphQLResultHash.new(result_name, selection_result, is_non_null)
+                    this_result = GraphQLResultHash.new(result_name, selection_result, is_non_null, continue_value, nil)
                     final_result = response_hash
                   else
                     this_result = response_hash
@@ -831,7 +908,8 @@ def continue_field(value, owner_type, field, current_type, ast_node, next_select
             # This is true for objects, unions, and interfaces
             use_dataloader_job = !inner_type.unwrap.kind.input?
             inner_type_non_null = inner_type.non_null?
-            response_list = GraphQLResultArray.new(result_name, selection_result, is_non_null)
+            response_list = GraphQLResultArray.new(result_name, selection_result, is_non_null, value, field)
+            response_list.graphql_skip_list_items_that_raise = current_type.skip_nodes_on_raise?
             set_result(selection_result, result_name, response_list, true, is_non_null)
             idx = nil
             list_value = begin

lib/graphql/query/context.rb

@@ -11,6 +11,15 @@ def skip
           GraphQL::Execution::SKIP
         end
 
+        # Return this value to tell the runtime
+        # to exclude this whole object from parent list.
+        #
+        # The runtime will find the find the nearest parent list marked `skip_items_on_raise: true`,
+        # and exclude the entire list item (including this object).
+        def skip_from_parent_list
+          GraphQL::Execution::SKIP_FROM_PARENT_LIST
+        end
+
         # Add error at query-level.
         # @param error [GraphQL::ExecutionError] an execution error
         # @return [void]

lib/graphql/schema/build_from_definition.rb

@@ -478,7 +478,7 @@ def build_resolve_type(lookup_hash, directives, missing_type_handler)
             when GraphQL::Language::Nodes::NonNullType
               resolve_type_proc.call(ast_node.of_type).to_non_null_type
             when GraphQL::Language::Nodes::ListType
-              resolve_type_proc.call(ast_node.of_type).to_list_type
+              resolve_type_proc.call(ast_node.of_type).to_list_type(skip_nodes_on_raise: false)
             when String
               directives[ast_node]
             else

lib/graphql/schema/field.rb

@@ -83,6 +83,8 @@ def subscription_scope
       end
       attr_writer :subscription_scope
 
+      attr_reader :on_raise_callback
+
       # Create a field instance from a list of arguments, keyword arguments, and a block.
       #
       # This method implements prioritization between the `resolver` or `mutation` defaults
@@ -218,8 +220,10 @@ def method_conflict_warning?
       # @param ast_node [Language::Nodes::FieldDefinition, nil] If this schema was parsed from definition, this AST node defined the field
       # @param method_conflict_warning [Boolean] If false, skip the warning if this field's method conflicts with a built-in method
       # @param validates [Array<Hash>] Configurations for validating this field
-      # @fallback_value [Object] A fallback value if the method is not defined
-      def initialize(type: nil, name: nil, owner: nil, null: nil, description: NOT_CONFIGURED, deprecation_reason: nil, method: nil, hash_key: nil, dig: nil, resolver_method: nil, connection: nil, max_page_size: NOT_CONFIGURED, default_page_size: NOT_CONFIGURED, scope: nil, introspection: false, camelize: true, trace: nil, complexity: nil, ast_node: nil, extras: EMPTY_ARRAY, extensions: EMPTY_ARRAY, connection_extension: self.class.connection_extension, resolver_class: nil, subscription_scope: nil, relay_node_field: false, relay_nodes_field: false, method_conflict_warning: true, broadcastable: NOT_CONFIGURED, arguments: EMPTY_HASH, directives: EMPTY_HASH, validates: EMPTY_ARRAY, fallback_value: NOT_CONFIGURED, &definition_block)
+      # @param fallback_value [Object] A fallback value if the method is not defined
+      # @param skip_nodes_on_raise [Boolean] true if this field's list items should be skipped, if resolving them led to an error being raised. Only applicable to List types.
+      # @param on_raise [Proc] A callback that's invoked when an error is raised while resolving this field's list items. Only applicable to List types.
+      def initialize(type: nil, name: nil, owner: nil, null: nil, description: NOT_CONFIGURED, deprecation_reason: nil, method: nil, hash_key: nil, dig: nil, resolver_method: nil, connection: nil, max_page_size: NOT_CONFIGURED, default_page_size: NOT_CONFIGURED, scope: nil, introspection: false, camelize: true, trace: nil, complexity: nil, ast_node: nil, extras: EMPTY_ARRAY, extensions: EMPTY_ARRAY, connection_extension: self.class.connection_extension, resolver_class: nil, subscription_scope: nil, relay_node_field: false, relay_nodes_field: false, method_conflict_warning: true, broadcastable: NOT_CONFIGURED, arguments: EMPTY_HASH, directives: EMPTY_HASH, validates: EMPTY_ARRAY, fallback_value: NOT_CONFIGURED, skip_nodes_on_raise: false, on_raise: nil, &definition_block)
         if name.nil?
           raise ArgumentError, "missing first `name` argument or keyword `name:`"
         end
@@ -275,6 +279,8 @@ def initialize(type: nil, name: nil, owner: nil, null: nil, description: NOT_CON
         else
           true
         end
+        @skip_nodes_on_raise = skip_nodes_on_raise
+        @on_raise_callback = on_raise
         @connection = connection
         @max_page_size = max_page_size
         @default_page_size = default_page_size
@@ -349,6 +355,17 @@ def initialize(type: nil, name: nil, owner: nil, null: nil, description: NOT_CON
 
         self.extensions.each(&:after_define_apply)
         @call_after_define = true
+
+        # TODO: should this check be centralized in `GraphQL::Schema::Member::BuildType#parse_type` instead?
+        if skip_nodes_on_raise && !self.type.list?
+          raise ArgumentError, "The `skip_nodes_on_raise` option is only applicable to lists."
+        end
+
+        # TODO: Rename "on_raise".
+        # This callback is specific to list _items_, but the "on_raise" naming doesn't make that clear.
+        if on_raise && !self.type.list?
+          raise ArgumentError, "The `on_raise` option is only applicable to lists."
+        end
       end
 
       # If true, subscription updates with this field can be shared between viewers
@@ -584,9 +601,10 @@ def type
             raise MissingReturnTypeError, "Can't determine the return type for #{self.path} (it has `resolver: #{@resolver_class}`, perhaps that class is missing a `type ...` declaration, or perhaps its type causes a cyclical loading issue)"
           end
           nullable = @return_type_null.nil? ? @resolver_class.null : @return_type_null
-          Member::BuildType.parse_type(return_type, null: nullable)
+          skip_nodes_on_raise = @skip_nodes_on_raise.nil? ? @resolver_class.skip_nodes_on_raise : @skip_nodes_on_raise
+          Member::BuildType.parse_type(return_type, null: nullable, skip_nodes_on_raise: skip_nodes_on_raise)
         else
-          @type ||= Member::BuildType.parse_type(@return_type_expr, null: @return_type_null)
+          @type ||= Member::BuildType.parse_type(@return_type_expr, null: @return_type_null, skip_nodes_on_raise: @skip_nodes_on_raise)
         end
       rescue GraphQL::Schema::InvalidDocumentError, MissingReturnTypeError => err
         # Let this propagate up

lib/graphql/schema/late_bound_type.rb

@@ -21,8 +21,13 @@ def to_non_null_type
         @to_non_null_type ||= GraphQL::Schema::NonNull.new(self)
       end
 
-      def to_list_type
-        @to_list_type ||= GraphQL::Schema::List.new(self)
+      # can we just inherit this from graphql/schema/member/type_system_helpers.rb?
+      def to_list_type(skip_nodes_on_raise: false)
+        if skip_nodes_on_raise
+          @to_skipping_list_type ||= GraphQL::Schema::List.new(self, skip_nodes_on_raise: true)
+        else
+          @to_list_type ||= GraphQL::Schema::List.new(self)
+        end
       end
 
       def inspect

lib/graphql/schema/list.rb

@@ -8,6 +8,11 @@ class Schema
     class List < GraphQL::Schema::Wrapper
       include Schema::Member::ValidatesInput
 
+      def initialize(of_type, skip_nodes_on_raise: false)
+        super(of_type)
+        @skip_nodes_on_raise = skip_nodes_on_raise
+      end
+
       # @return [GraphQL::TypeKinds::LIST]
       def kind
         GraphQL::TypeKinds::LIST
@@ -18,6 +23,10 @@ def list?
         true
       end
 
+      def skip_nodes_on_raise?
+        @skip_nodes_on_raise
+      end
+
       def to_type_signature
         "[#{@of_type.to_type_signature}]"
       end

lib/graphql/schema/member/build_type.rb

@@ -9,7 +9,7 @@ module BuildType
         module_function
         # @param type_expr [String, Class, GraphQL::BaseType]
         # @return [GraphQL::BaseType]
-        def parse_type(type_expr, null:)
+        def parse_type(type_expr, null:, skip_nodes_on_raise: false)
           list_type = false
 
           return_type = case type_expr
@@ -85,7 +85,7 @@ def parse_type(type_expr, null:)
           # Apply list_type first, that way the
           # .to_non_null_type applies to the list type, not the inner type
           if list_type
-            return_type = return_type.to_list_type
+            return_type = return_type.to_list_type(skip_nodes_on_raise: skip_nodes_on_raise)
           end
 
           if !null