Improve Scalar coercion spec compliance

Previously many result coercion failures for built-in scalar types
raised Ruby exceptions instead of GraphQL execution errors.

The GraphQL spec says that any time a scalar value cannot be coerced to
a spec complaint result, an execution error should be raised. Example: https://spec.graphql.org/draft/#sel-GAHXRFHCAACGS6rU

This introduces a backwards compatible way to opt into this new spec
compliant behaviour. `Schema.spec_compliant_scalar_coercion_errors`
controls this behaviour and setting it to `true` will return execution
errors instead of raising Ruby exceptions.

`Schema.spec_compliant_scalar_coercion_errors` defaults to being `nil`
(disabled) and will produce a warning about the intention to change this
default in a future version. To preserve this legacy (and spec
non-compliant) behaviour, you can customize the error handling logic in
`Schema.type_error`.

Note: while this change is non-breaking, the runtime exceptions raised
have changed. Now any coercion failure will raise
`LegacyScalarCoercionError` instead of scalar specific ones (eg:
`IntegerEncodingError`).

This also improves scalar result coercion errors and makes them more
spec compliant.

Before:
```ruby
{
  "errors" => [
    {
      "message" =>  "Int cannot represent non 32-bit signed integer value: 2147483648",
    },
  ],
},
```

```ruby
{
  "errors" => [
    {
      "message" =>  "Int cannot represent non 32-bit signed integer value: 2147483648",
      "locations" => [{ "line" => 1, "column" => 3 }],
      "path" => ["someField"],
    },
  ],
  "data" => {
    "someField" => nil,
  }
},
```

Author: swalkinshaw

lib/graphql.rb

@@ -94,8 +94,7 @@ class << self
   autoload :CoercionError, "graphql/coercion_error"
   autoload :InvalidNameError, "graphql/invalid_name_error"
   autoload :IntegerDecodingError, "graphql/integer_decoding_error"
-  autoload :IntegerEncodingError, "graphql/integer_encoding_error"
-  autoload :StringEncodingError, "graphql/string_encoding_error"
+  autoload :LegacyScalarCoercionError, "graphql/legacy_scalar_coercion_error"
   autoload :DateEncodingError, "graphql/date_encoding_error"
   autoload :DurationEncodingError, "graphql/duration_encoding_error"
   autoload :TypeKinds, "graphql/type_kinds"

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

@@ -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 LegacyScalarCoercionError < 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/legacy_scalar_coercion_error.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)
@@ -1313,11 +1325,24 @@ def type_error(type_error, context)
           execution_error = GraphQL::ExecutionError.new(type_error.message, ast_node: type_error.ast_node)
           execution_error.path = context[:current_path]
 
-          context.errors << execution_error
-        when GraphQL::UnresolvedTypeError, GraphQL::StringEncodingError, GraphQL::IntegerEncodingError
-          raise type_error
+          ctx.errors << execution_error
+        when GraphQL::LegacyScalarCoercionError
+          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/schema.rb

@@ -5,12 +5,23 @@ module Types
     class Float < GraphQL::Schema::Scalar
       description "Represents signed double-precision fractional values as specified by [IEEE 754](https://en.wikipedia.org/wiki/IEEE_floating_point)."
 
-      def self.coerce_input(value, _ctx)
+      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::LegacyScalarCoercionError.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/string_encoding_error.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::LegacyScalarCoercionError.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/float.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::LegacyScalarCoercionError.new(
+          "String cannot represent value: #{value.inspect}",
+          value: value,
+          context: ctx
+        )
+        ctx.schema.type_error(error, ctx)
       end
 
       def self.coerce_input(value, _ctx)

lib/graphql/types/int.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::LegacyScalarCoercionError) do
+        GraphQL::Types::Float.coerce_result("foo", err_ctx)
+      end
+
+      assert_raises(GraphQL::LegacyScalarCoercionError) 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, null: true do
+            argument :value, GraphQL::Types::Float, required: true
+          end
+
+          field :bad_float, GraphQL::Types::Float, null: true
+
+          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::LegacyScalarCoercionError) do
+          FloatNonSpecComplaintErrors.execute(query)
+        end
+
+        assert_equal("Float cannot represent non numeric value: Infinity", error.message)
+      end
+    end
+  end
 end

lib/graphql/types/string.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 require "spec_helper"
 
+
 describe GraphQL::Types::Int do
   describe "coerce_input" do
     it "accepts ints within the bounds" do
@@ -26,21 +27,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_raises(GraphQL::LegacyScalarCoercionError) { GraphQL::Types::Int.coerce_result(2**31, err_ctx) }
+        err = assert_raises(GraphQL::LegacyScalarCoercionError) { GraphQL::Types::Int.coerce_result(-(2**31 + 1), err_ctx) }
+        assert_equal "Int cannot represent non 32-bit signed integer value: -2147483649", err.message
 
-        err = assert_raises GraphQL::IntegerEncodingError do
+        err = assert_raises GraphQL::LegacyScalarCoercionError 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, null: true do
+              argument :value, GraphQL::Types::Int, required: true
+            end
+
+            field :bad_int, GraphQL::Types::Int, null: true
+
+            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::LegacyScalarCoercionError) do
+            IntNonSpecComplaintErrors.execute(query)
+          end
+
+          assert_equal("Int cannot represent non 32-bit signed integer value: 2147483648", error.message)
+        end
       end
     end
   end