Merge pull request #3471 from rmosolgo/fix-rescue-from-on-loading-arguments

Fix rescue from on loading arguments

Author: Robert Mosolgo

lib/graphql/query/null_context.rb

@@ -9,10 +9,16 @@ def visible_field?(t); true; end
         def visible_type?(t); true; end
       end
 
+      class NullQuery
+        def with_error_handling
+          yield
+        end
+      end
+
       attr_reader :schema, :query, :warden, :dataloader
 
       def initialize
-        @query = nil
+        @query = NullQuery.new
         @dataloader = GraphQL::Dataloader::NullDataloader.new
         @schema = GraphQL::Schema.new
         @warden = NullWarden.new(

lib/graphql/schema/argument.rb

@@ -266,7 +266,9 @@ def coerce_into_values(parent_object, values, context, argument_values)
               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)
+              context.query.with_error_handling do
+                owner.load_application_object(self, loads, coerced_value, context)
+              end
             end
           end
 

lib/graphql/schema/scalar.rb

@@ -44,7 +44,9 @@ def default_scalar?
         def validate_non_null_input(value, ctx)
           result = Query::InputValidationResult.new
           coerced_result = begin
-            coerce_input(value, ctx)
+            ctx.query.with_error_handling do
+              coerce_input(value, ctx)
+            end
           rescue GraphQL::CoercionError => err
             err
           end

lib/graphql/static_validation/validator.rb

@@ -73,6 +73,11 @@ def validate(query, validate: true, timeout: nil)
             irep: irep,
           }
         end
+      rescue GraphQL::ExecutionError => e
+        {
+          errors: [e],
+          irep: nil,
+        }
       end
 
       # Invoked when static validation times out.

spec/graphql/execution/errors_spec.rb

@@ -56,6 +56,7 @@ def self.authorized?(obj, ctx)
         if ctx[:authorized] == false
           raise ErrorD
         end
+        true
       end
 
       field :string, String, null: false
@@ -76,6 +77,16 @@ def self.object_from_id(type, value, ctx)
       end
     end
 
+    class PickyString < GraphQL::Schema::Scalar
+      def self.coerce_input(value, ctx)
+        if value == "picky"
+          value
+        else
+          raise ErrorB, "The string wasn't \"picky\""
+        end
+      end
+    end
+
     class Query < GraphQL::Schema::Object
       field :f1, Int, null: true do
         argument :a1, Int, required: false
@@ -116,6 +127,22 @@ def f7
         raise ErrorBGrandchildClass
       end
 
+      field :f8, String, null: true do
+        argument :input, PickyString, required: true
+      end
+
+      def f8(input:)
+        input
+      end
+
+      field :f9, String, null: true do
+        argument :thing_id, ID, required: true, loads: Thing
+      end
+
+      def f9(thing:)
+        thing[:id]
+      end
+
       field :thing, Thing, null: true
       def thing
         :thing
@@ -133,6 +160,18 @@ def non_nullable_array
 
     query(Query)
     lazy_resolve(Proc, :call)
+
+    def self.object_from_id(id, ctx)
+      if id == "boom"
+        raise ErrorB
+      end
+
+      { thing: true, id: id }
+    end
+
+    def self.resolve_type(type, obj, ctx)
+      Thing
+    end
   end
 
   class ErrorsTestSchemaWithoutInterpreter < GraphQL::Schema
@@ -198,6 +237,32 @@ def non_nullable_array
       assert_equal ["raised subclass (ErrorsTestSchema::Query.f5, nil, {})"], context[:errors]
     end
 
+    describe "errors raised when coercing inputs" do
+      it "rescues them" do
+        res1 = ErrorsTestSchema.execute("{ f8(input: \"picky\") }")
+        assert_equal "picky", res1["data"]["f8"]
+        res2 = ErrorsTestSchema.execute("{ f8(input: \"blah\") }")
+        assert_equal ["errors"], res2.keys
+        assert_equal ["boom!"], res2["errors"].map { |e| e["message"] }
+
+        res3 = ErrorsTestSchema.execute("query($v: PickyString!) { f8(input: $v) }", variables: { v: "blah" })
+        assert_equal ["errors"], res3.keys
+        assert_equal ["Variable $v of type PickyString! was provided invalid value"], res3["errors"].map { |e| e["message"] }
+        assert_equal [["boom!"]], res3["errors"].map { |e| e["extensions"]["problems"].map { |pr| pr["explanation"] } }
+      end
+    end
+
+    describe "errors raised when loading objects from ID" do
+      it "rescues them" do
+        res1 = ErrorsTestSchema.execute("{ f9(thingId: \"abc\") }")
+        pp res1
+        assert_equal "abc", res1["data"]["f9"]
+
+        res2 = ErrorsTestSchema.execute("{ f9(thingId: \"boom\") }")
+        assert_equal ["boom!"], res2["errors"].map { |e| e["message"] }
+      end
+    end
+
     describe "errors raised in authorized hooks" do
       it "rescues them" do
         context = { authorized: false }