Improve Scalar result coercion spec compliance

lib/graphql.rb

@@ -93,6 +93,7 @@ class << self
   autoload :AnalysisError, "graphql/analysis_error"
   autoload :CoercionError, "graphql/coercion_error"
   autoload :InvalidNameError, "graphql/invalid_name_error"
+  autoload :ScalarCoercionError, "graphql/scalar_coercion_error"
   autoload :IntegerDecodingError, "graphql/integer_decoding_error"
   autoload :IntegerEncodingError, "graphql/integer_encoding_error"
   autoload :StringEncodingError, "graphql/string_encoding_error"

lib/graphql/execution/interpreter/runtime.rb

@@ -675,6 +675,8 @@ def continue_field(value, owner_type, field, current_type, ast_node, next_select
           when "SCALAR", "ENUM"
             r = begin
               current_type.coerce_result(value, context)
+            rescue GraphQL::ExecutionError => ex_err
+              return continue_value(ex_err, field, is_non_null, ast_node, result_name, selection_result)
             rescue StandardError => err
               query.handle_or_reraise(err)
             end

lib/graphql/integer_encoding_error.rb

@@ -8,29 +8,6 @@ module GraphQL
   # - `GraphQL::Types::BigInt` for really big integer values
   #
   # @see GraphQL::Types::Int which raises this error
-  class IntegerEncodingError < GraphQL::RuntimeTypeError
-    # The value which couldn't be encoded
-    attr_reader :integer_value
-
-    # @return [GraphQL::Schema::Field] The field that returned a too-big integer
-    attr_reader :field
-
-    # @return [Array<String, Integer>] Where the field appeared in the GraphQL response
-    attr_reader :path
-
-    def initialize(value, context:)
-      @integer_value = value
-      @field = context[:current_field]
-      @path = context[:current_path]
-      message = "Integer out of bounds: #{value}".dup
-      if @path
-        message << " @ #{@path.join(".")}"
-      end
-      if @field
-        message << " (#{@field.path})"
-      end
-      message << ". Consider using ID or GraphQL::Types::BigInt instead."
-      super(message)
-    end
+  class IntegerEncodingError < GraphQL::ScalarCoercionError
   end
 end

lib/graphql/scalar_coercion_error.rb

@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+module GraphQL
+  # This error is raised when a scalar type cannot coerce a value to its expected type. It is considered legacy because it's raised as a RuntimeTypeError from `Schema.type_error` when `Schema.spec_compliant_scalar_coercion_errors` is not enabled.
+  class ScalarCoercionError < GraphQL::RuntimeTypeError
+    # The value which couldn't be coerced
+    attr_reader :value
+
+    # @return [GraphQL::Schema::Field] The field that returned a type error
+    attr_reader :field
+
+    # @return [Array<String, Integer>] Where the field appeared in the GraphQL response
+    attr_reader :path
+
+    def initialize(message, value:, context:)
+      @value = value
+      @field = context[:current_field]
+      @path = context[:current_path]
+
+      super(message)
+    end
+  end
+end

lib/graphql/schema.rb

@@ -905,6 +905,18 @@ def error_bubbling(new_error_bubbling = nil)
 
       attr_writer :error_bubbling
 
+      def spec_compliant_scalar_coercion_errors(new_spec_compliant_scalar_coercion_errors = NOT_CONFIGURED)
+        if NOT_CONFIGURED.equal?(new_spec_compliant_scalar_coercion_errors)
+          if defined?(@spec_compliant_scalar_coercion_errors)
+            @spec_compliant_scalar_coercion_errors
+          else
+            find_inherited_value(:spec_compliant_scalar_coercion_errors, false)
+          end
+        else
+          @spec_compliant_scalar_coercion_errors = new_spec_compliant_scalar_coercion_errors
+        end
+      end
+
       attr_writer :max_depth
 
       def max_depth(new_max_depth = nil, count_introspection_fields: true)
@@ -1314,10 +1326,23 @@ def type_error(type_error, context)
           execution_error.path = context[:current_path]
 
           context.errors << execution_error
-        when GraphQL::UnresolvedTypeError, GraphQL::StringEncodingError, GraphQL::IntegerEncodingError
-          raise type_error
+        when GraphQL::ScalarCoercionError
+          if spec_compliant_scalar_coercion_errors == true
+            execution_error = GraphQL::CoercionError.new(type_error.message)
+            raise execution_error
+          else
+            warn <<~MSG
+            Scalar coercion errors will return GraphQL execution errors instead of raising Ruby exceptions in a future version.
+            To opt into this new behavior, set `Schema.spec_compliant_scalar_coercion_errors = true`.
+            To keep or customize the current behavior, add custom error handling in `Schema.type_error`.
+            MSG
+
+            raise type_error
+          end
         when GraphQL::IntegerDecodingError
           nil
+        when GraphQL::UnresolvedTypeError
+          raise type_error
         end
       end
 

lib/graphql/string_encoding_error.rb

@@ -1,20 +1,5 @@
 # frozen_string_literal: true
 module GraphQL
-  class StringEncodingError < GraphQL::RuntimeTypeError
-    attr_reader :string, :field, :path
-    def initialize(str, context:)
-      @string = str
-      @field = context[:current_field]
-      @path = context[:current_path]
-      message = "String #{str.inspect} was encoded as #{str.encoding}".dup
-      if @path
-        message << " @ #{@path.join(".")}"
-      end
-      if @field
-        message << " (#{@field.path})"
-      end
-      message << ". GraphQL requires an encoding compatible with UTF-8."
-      super(message)
-    end
+  class StringEncodingError < GraphQL::ScalarCoercionError
   end
 end

lib/graphql/types/float.rb

@@ -9,8 +9,19 @@ def self.coerce_input(value, _ctx)
         value.is_a?(Numeric) ? value.to_f : nil
       end
 
-      def self.coerce_result(value, _ctx)
-        value.to_f
+      def self.coerce_result(value, ctx)
+        coerced_value = Float(value, exception: false)
+
+        if coerced_value.nil? || !coerced_value.finite?
+          error = GraphQL::ScalarCoercionError.new(
+            "Float cannot represent non numeric value: #{value.inspect}",
+            value: value,
+            context: ctx
+          )
+          ctx.schema.type_error(error, ctx)
+        else
+          coerced_value
+        end
       end
 
       default_scalar true

lib/graphql/types/int.rb

@@ -21,11 +21,16 @@ def self.coerce_input(value, ctx)
       end
 
       def self.coerce_result(value, ctx)
-        value = value.to_i
-        if value >= MIN && value <= MAX
+        value = Integer(value, exception: false)
+
+        if value && (value >= MIN && value <= MAX)
           value
         else
-          err = GraphQL::IntegerEncodingError.new(value, context: ctx)
+          err = GraphQL::IntegerEncodingError.new(
+            "Int cannot represent non 32-bit signed integer value: #{value.inspect}",
+            value: value,
+            context: ctx
+          )
           ctx.schema.type_error(err, ctx)
         end
       end

lib/graphql/types/string.rb

@@ -15,8 +15,12 @@ def self.coerce_result(value, ctx)
           str.encode!(Encoding::UTF_8)
         end
       rescue EncodingError
-        err = GraphQL::StringEncodingError.new(str, context: ctx)
-        ctx.schema.type_error(err, ctx)
+        error = GraphQL::StringEncodingError.new(
+          "String cannot represent value: #{value.inspect}",
+          value: value,
+          context: ctx
+        )
+        ctx.schema.type_error(error, ctx)
       end
 
       def self.coerce_input(value, _ctx)

spec/graphql/types/float_spec.rb

@@ -16,4 +16,88 @@
       assert_nil GraphQL::Types::Float.coerce_isolated_input(enum)
     end
   end
+
+  describe "coerce_result" do
+    it "coercess ints and floats" do
+      err_ctx = GraphQL::Query.new(Dummy::Schema, "{ __typename }").context
+
+      assert_equal 1.0, GraphQL::Types::Float.coerce_result(1, err_ctx)
+      assert_equal 1.0, GraphQL::Types::Float.coerce_result("1", err_ctx)
+      assert_equal 1.0, GraphQL::Types::Float.coerce_result("1.0", err_ctx)
+      assert_equal 6.1, GraphQL::Types::Float.coerce_result(6.1, err_ctx)
+    end
+
+    it "rejects other types" do
+      err_ctx = GraphQL::Query.new(Dummy::Schema, "{ __typename }").context
+
+      assert_raises(GraphQL::ScalarCoercionError) do
+        GraphQL::Types::Float.coerce_result("foo", err_ctx)
+      end
+
+      assert_raises(GraphQL::ScalarCoercionError) do
+        GraphQL::Types::Float.coerce_result(1.0 / 0, err_ctx)
+      end
+    end
+
+    describe "with Schema.spec_compliant_scalar_coercion_errors" do
+      class FloatScalarSchema < GraphQL::Schema
+        class Query < GraphQL::Schema::Object
+          field :float, GraphQL::Types::Float do
+            argument :value, GraphQL::Types::Float
+          end
+
+          field :bad_float, GraphQL::Types::Float
+
+          def float(value:)
+            value
+          end
+
+          def bad_float
+            Float::INFINITY
+          end
+        end
+
+        query(Query)
+      end
+
+      class FloatSpecCompliantErrors < FloatScalarSchema
+        spec_compliant_scalar_coercion_errors true
+      end
+
+      class FloatNonSpecComplaintErrors < FloatScalarSchema
+        spec_compliant_scalar_coercion_errors false
+      end
+
+      it "returns GraphQL execution errors with spec_compliant_scalar_coercion_errors enabled" do
+        query = "{ badFloat }"
+        result = FloatSpecCompliantErrors.execute(query)
+
+        assert_equal(
+          {
+            "errors" => [
+              {
+                "message" =>  "Float cannot represent non numeric value: Infinity",
+                "locations" => [{ "line" => 1, "column" => 3 }],
+                "path" => ["badFloat"],
+              },
+            ],
+            "data" => {
+              "badFloat" => nil,
+            }
+          },
+          result.to_h
+        )
+      end
+
+      it "raises Ruby exceptions with spec_compliant_scalar_coercion_errors disabled" do
+        query = "{ badFloat }"
+
+        error = assert_raises(GraphQL::ScalarCoercionError) do
+          FloatNonSpecComplaintErrors.execute(query)
+        end
+
+        assert_equal("Float cannot represent non numeric value: Infinity", error.message)
+      end
+    end
+  end
 end

spec/graphql/types/int_spec.rb

@@ -26,21 +26,77 @@
         assert_equal(-(2**31), GraphQL::Types::Int.coerce_result(-(2**31), context))
       end
 
-      it "replaces values, if configured to do so" do
-        assert_equal Dummy::Schema::MAGIC_INT_COERCE_VALUE, GraphQL::Types::Int.coerce_result(99**99, context)
-      end
-
       it "raises on values out of bounds" do
         err_ctx = GraphQL::Query.new(Dummy::Schema, "{ __typename }").context
         assert_raises(GraphQL::IntegerEncodingError) { GraphQL::Types::Int.coerce_result(2**31, err_ctx) }
         err = assert_raises(GraphQL::IntegerEncodingError) { GraphQL::Types::Int.coerce_result(-(2**31 + 1), err_ctx) }
-        assert_equal "Integer out of bounds: -2147483649. Consider using ID or GraphQL::Types::BigInt instead.", err.message
+        assert_equal "Int cannot represent non 32-bit signed integer value: -2147483649", err.message
 
         err = assert_raises GraphQL::IntegerEncodingError do
           Dummy::Schema.execute("{ hugeInteger }")
         end
-        expected_err = "Integer out of bounds: 2147483648 @ hugeInteger (Query.hugeInteger). Consider using ID or GraphQL::Types::BigInt instead."
-        assert_equal expected_err, err.message
+        assert_equal "Int cannot represent non 32-bit signed integer value: 2147483648", err.message
+      end
+
+      describe "with Schema.spec_compliant_scalar_coercion_errors" do
+        class IntScalarSchema < GraphQL::Schema
+          class Query < GraphQL::Schema::Object
+            field :int, GraphQL::Types::Int do
+              argument :value, GraphQL::Types::Int
+            end
+
+            field :bad_int, GraphQL::Types::Int
+
+            def int(value:)
+              value
+            end
+
+            def bad_int
+              2**31 # Out of range
+            end
+          end
+
+          query(Query)
+        end
+
+        class IntSpecCompliantErrors < IntScalarSchema
+          spec_compliant_scalar_coercion_errors true
+        end
+
+        class IntNonSpecComplaintErrors < IntScalarSchema
+          spec_compliant_scalar_coercion_errors false
+        end
+
+        it "returns GraphQL execution errors with spec_compliant_scalar_coercion_errors enabled" do
+          query = "{ badInt }"
+          result = IntSpecCompliantErrors.execute(query)
+
+          assert_equal(
+            {
+              "errors" => [
+                {
+                  "message" =>  "Int cannot represent non 32-bit signed integer value: 2147483648",
+                  "locations" => [{ "line" => 1, "column" => 3 }],
+                  "path" => ["badInt"],
+                },
+              ],
+              "data" => {
+                "badInt" => nil,
+              }
+            },
+            result.to_h
+          )
+        end
+
+        it "raises Ruby exceptions with spec_compliant_scalar_coercion_errors disabled" do
+          query = "{ badInt }"
+
+          error = assert_raises(GraphQL::IntegerEncodingError) do
+            IntNonSpecComplaintErrors.execute(query)
+          end
+
+          assert_equal("Int cannot represent non 32-bit signed integer value: 2147483648", error.message)
+        end
       end
     end
   end