Merge pull request #3957 from rmosolgo/inline-errors

Try reducing stack trace size during queries

Author: rmosolgo

lib/graphql/execution/errors.rb

@@ -2,79 +2,42 @@
 
 module GraphQL
   module Execution
-    # A plugin that wraps query execution with error handling. Installed by default.
-    #
-    # @example Handling ActiveRecord::NotFound
-    #
-    #   class MySchema < GraphQL::Schema
-    #     rescue_from(ActiveRecord::NotFound) do |err, obj, args, ctx, field|
-    #       ErrorTracker.log("Not Found: #{err.message}")
-    #       nil
-    #     end
-    #   end
-    #
     class Errors
-      NEW_HANDLER_HASH = ->(h, k) {
-        h[k] = {
-          class: k,
-          handler: nil,
-          subclass_handlers: Hash.new(&NEW_HANDLER_HASH),
-         }
-      }
-
-      def initialize(schema)
-        @schema = schema
-        @handlers = {
-          class: nil,
-          handler: nil,
-          subclass_handlers: Hash.new(&NEW_HANDLER_HASH),
-        }
-      end
-
-      # @api private
-      def each_rescue
-        handlers = @handlers.values
-        while (handler = handlers.shift) do
-          yield(handler[:class], handler[:handler])
-          handlers.concat(handler[:subclass_handlers].values)
-        end
-      end
-
       # Register this handler, updating the
       # internal handler index to maintain least-to-most specific.
       #
       # @param error_class [Class<Exception>]
+      # @param error_handlers [Hash]
       # @param error_handler [Proc]
       # @return [void]
-      def rescue_from(error_class, error_handler)
+      def self.register_rescue_from(error_class, error_handlers, error_handler)
         subclasses_handlers = {}
         this_level_subclasses = []
         # During this traversal, do two things:
         # - Identify any already-registered subclasses of this error class
         #   and gather them up to be inserted _under_ this class
         # - Find the point in the index where this handler should be inserted
         #   (That is, _under_ any superclasses, or at top-level, if there are no superclasses registered)
-        handlers = @handlers[:subclass_handlers]
-        while (handlers) do
+        while (error_handlers) do
           this_level_subclasses.clear
           # First, identify already-loaded handlers that belong
           # _under_ this one. (That is, they're handlers
           # for subclasses of `error_class`.)
-          handlers.each do |err_class, handler|
+          error_handlers.each do |err_class, handler|
             if err_class < error_class
               subclasses_handlers[err_class] = handler
               this_level_subclasses << err_class
             end
           end
           # Any handlers that we'll be moving, delete them from this point in the index
           this_level_subclasses.each do |err_class|
-            handlers.delete(err_class)
+            error_handlers.delete(err_class)
           end
 
           # See if any keys in this hash are superclasses of this new class:
-          next_index_point = handlers.find { |err_class, handler| error_class < err_class }
+          next_index_point = error_handlers.find { |err_class, handler| error_class < err_class }
           if next_index_point
-            handlers = next_index_point[1][:subclass_handlers]
+            error_handlers = next_index_point[1][:subclass_handlers]
           else
             # this new handler doesn't belong to any sub-handlers,
             # so insert it in the current set of `handlers`
@@ -83,40 +46,15 @@ def rescue_from(error_class, error_handler)
         end
         # Having found the point at which to insert this handler,
         # register it and merge any subclass handlers back in at this point.
-        this_class_handlers = handlers[error_class]
+        this_class_handlers = error_handlers[error_class]
         this_class_handlers[:handler] = error_handler
         this_class_handlers[:subclass_handlers].merge!(subclasses_handlers)
         nil
       end
 
-      # Call the given block with the schema's configured error handlers.
-      #
-      # If the block returns a lazy value, it's not wrapped with error handling. That area will have to be wrapped itself.
-      #
-      # @param ctx [GraphQL::Query::Context]
-      # @return [Object] Either the result of the given block, or some object to replace the result, in case of error handling.
-      def with_error_handling(ctx)
-        yield
-      rescue StandardError => err
-        handler = find_handler_for(err.class)
-        if handler
-          runtime_info = ctx.namespace(:interpreter) || {}
-          obj = runtime_info[:current_object]
-          args = runtime_info[:current_arguments]
-          args = args && args.keyword_arguments
-          field = runtime_info[:current_field]
-          if obj.is_a?(GraphQL::Schema::Object)
-            obj = obj.object
-          end
-          handler[:handler].call(err, obj, args, ctx, field)
-        else
-          raise err
-        end
-      end
-
       # @return [Proc, nil] The handler for `error_class`, if one was registered on this schema or inherited
-      def find_handler_for(error_class)
-        handlers = @handlers[:subclass_handlers]
+      def self.find_handler_for(schema, error_class)
+        handlers = schema.error_handlers[:subclass_handlers]
         handler = nil
         while (handlers) do
           _err_class, next_handler = handlers.find { |err_class, handler| error_class <= err_class }
@@ -131,8 +69,8 @@ def find_handler_for(error_class)
         end
 
         # check for a handler from a parent class:
-        if @schema.superclass.respond_to?(:error_handler) && (parent_errors = @schema.superclass.error_handler)
-          parent_handler = parent_errors.find_handler_for(error_class)
+        if schema.superclass.respond_to?(:error_handlers)
+          parent_handler = find_handler_for(schema.superclass, error_class)
         end
 
         # If the inherited handler is more specific than the one defined here,

lib/graphql/execution/interpreter/runtime.rb

@@ -498,13 +498,17 @@ def evaluate_selection_with_args(arguments, field_defn, next_path, ast_node, fie
             field_result = call_method_on_directives(:resolve, object, directives) do
               # Actually call the field resolver and capture the result
               app_result = begin
-                query.with_error_handling do
-                  query.trace("execute_field", {owner: owner_type, field: field_defn, path: next_path, ast_node: ast_node, query: query, object: object, arguments: kwarg_arguments}) do
-                    field_defn.resolve(object, kwarg_arguments, context)
-                  end
+                query.trace("execute_field", {owner: owner_type, field: field_defn, path: next_path, ast_node: ast_node, query: query, object: object, arguments: kwarg_arguments}) do
+                  field_defn.resolve(object, kwarg_arguments, context)
                 end
               rescue GraphQL::ExecutionError => err
                 err
+              rescue StandardError => err
+                begin
+                  query.handle_or_reraise(err)
+                rescue GraphQL::ExecutionError => ex_err
+                  ex_err
+                end
               end
               after_lazy(app_result, owner: owner_type, field: field_defn, path: next_path, ast_node: ast_node, owner_object: object, arguments: resolved_arguments, result_name: result_name, result: selection_result) do |inner_result|
                 continue_value = continue_value(next_path, inner_result, owner_type, field_defn, return_type.non_null?, ast_node, result_name, selection_result)
@@ -871,21 +875,21 @@ def after_lazy(lazy_obj, owner:, field:, path:, owner_object:, arguments:, ast_n
               # Wrap the execution of _this_ method with tracing,
               # but don't wrap the continuation below
               inner_obj = begin
-                query.with_error_handling do
-                  begin
-                    if trace
-                      query.trace("execute_field_lazy", {owner: owner, field: field, path: path, query: query, object: owner_object, arguments: arguments, ast_node: ast_node}) do
-                        schema.sync_lazy(lazy_obj)
-                      end
-                    else
-                      schema.sync_lazy(lazy_obj)
-                    end
-                  rescue GraphQL::ExecutionError, GraphQL::UnauthorizedError => err
-                    err
+                if trace
+                  query.trace("execute_field_lazy", {owner: owner, field: field, path: path, query: query, object: owner_object, arguments: arguments, ast_node: ast_node}) do
+                    schema.sync_lazy(lazy_obj)
                   end
+                else
+                  schema.sync_lazy(lazy_obj)
                 end
-              rescue GraphQL::ExecutionError => ex_err
+              rescue GraphQL::ExecutionError, GraphQL::UnauthorizedError => ex_err
                 ex_err
+              rescue StandardError => err
+                begin
+                  query.handle_or_reraise(err)
+                rescue GraphQL::ExecutionError => ex_err
+                  ex_err
+                end
               end
               yield(inner_obj)
             end

lib/graphql/query.rb

@@ -331,10 +331,8 @@ def subscription?
     end
 
     # @api private
-    def with_error_handling
-      schema.error_handler.with_error_handling(context) do
-        yield
-      end
+    def handle_or_reraise(err)
+      schema.handle_or_reraise(context, err)
     end
 
     private

lib/graphql/query/null_context.rb

@@ -12,9 +12,6 @@ def visible_type_membership?(tm, ctx); true; end
       end
 
       class NullQuery
-        def with_error_handling
-          yield
-        end
       end
 
       class NullSchema < GraphQL::Schema

lib/graphql/schema.rb

@@ -693,7 +693,41 @@ def context_class(new_context_class = nil)
 
       def rescue_from(*err_classes, &handler_block)
         err_classes.each do |err_class|
-          error_handler.rescue_from(err_class, handler_block)
+          Execution::Errors.register_rescue_from(err_class, error_handlers[:subclass_handlers], handler_block)
+        end
+      end
+
+      NEW_HANDLER_HASH = ->(h, k) {
+        h[k] = {
+          class: k,
+          handler: nil,
+          subclass_handlers: Hash.new(&NEW_HANDLER_HASH),
+         }
+      }
+
+      def error_handlers
+        @error_handlers ||= {
+          class: nil,
+          handler: nil,
+          subclass_handlers: Hash.new(&NEW_HANDLER_HASH),
+        }
+      end
+
+      # @api private
+      def handle_or_reraise(context, err)
+        handler = Execution::Errors.find_handler_for(self, err.class)
+        if handler
+          runtime_info = context.namespace(:interpreter) || {}
+          obj = runtime_info[:current_object]
+          args = runtime_info[:current_arguments]
+          args = args && args.keyword_arguments
+          field = runtime_info[:current_field]
+          if obj.is_a?(GraphQL::Schema::Object)
+            obj = obj.object
+          end
+          handler[:handler].call(err, obj, args, context, field)
+        else
+          raise err
         end
       end
 
@@ -821,11 +855,6 @@ def parse_error(parse_err, ctx)
         ctx.errors.push(parse_err)
       end
 
-      # @return [GraphQL::Execution::Errors]
-      def error_handler
-        @error_handler ||= GraphQL::Execution::Errors.new(self)
-      end
-
       def lazy_resolve(lazy_class, value_method)
         lazy_methods.set(lazy_class, value_method)
       end

lib/graphql/schema/argument.rb

@@ -250,24 +250,30 @@ def coerce_into_values(parent_object, values, context, argument_values)
         end
 
         loaded_value = nil
-        coerced_value = context.schema.error_handler.with_error_handling(context) do
+        coerced_value = begin
           type.coerce_input(value, context)
+        rescue StandardError => err
+          context.schema.handle_or_reraise(context, err)
         end
 
         # 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 |resolved_coerced_value|
           if loads && !from_resolver?
-            loaded_value = context.query.with_error_handling do
+            loaded_value = begin
               load_and_authorize_value(owner, coerced_value, context)
+            rescue StandardError => err
+              context.schema.handle_or_reraise(context, err)
             end
           end
 
           maybe_loaded_value = loaded_value || resolved_coerced_value
           context.schema.after_lazy(maybe_loaded_value) do |resolved_loaded_value|
             owner.validate_directive_argument(self, resolved_loaded_value)
-            prepared_value = context.schema.error_handler.with_error_handling(context) do
+            prepared_value = begin
               prepare_value(parent_object, resolved_loaded_value, context: context)
+            rescue StandardError => err
+              context.schema.handle_or_reraise(context, err)
             end
 
             # TODO code smell to access such a deeply-nested constant in a distant module