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)
@@ -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::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

@@ -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::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 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::LegacyScalarCoercionError) do
+          FloatNonSpecComplaintErrors.execute(query)
+        end
+
+        assert_equal("Float cannot represent non numeric value: Infinity", error.message)
+      end
+    end
+  end
 end