diff --git a/lib/graphql.rb b/lib/graphql.rb index 8889c2c032..25dda81b23 100644 --- a/lib/graphql.rb +++ b/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" diff --git a/lib/graphql/execution/interpreter/runtime.rb b/lib/graphql/execution/interpreter/runtime.rb index 75429d5dc9..ecf3a27594 100644 --- a/lib/graphql/execution/interpreter/runtime.rb +++ b/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 diff --git a/lib/graphql/integer_encoding_error.rb b/lib/graphql/integer_encoding_error.rb index 5de3b52b34..aab8c34296 100644 --- a/lib/graphql/integer_encoding_error.rb +++ b/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] 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 diff --git a/lib/graphql/scalar_coercion_error.rb b/lib/graphql/scalar_coercion_error.rb new file mode 100644 index 0000000000..b93762a515 --- /dev/null +++ b/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] 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 diff --git a/lib/graphql/schema.rb b/lib/graphql/schema.rb index b72b24b5af..8cd100168b 100644 --- a/lib/graphql/schema.rb +++ b/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 diff --git a/lib/graphql/string_encoding_error.rb b/lib/graphql/string_encoding_error.rb index d177e45bbc..117d71fe4d 100644 --- a/lib/graphql/string_encoding_error.rb +++ b/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 diff --git a/lib/graphql/types/float.rb b/lib/graphql/types/float.rb index 7551899ed5..9d5955d58d 100644 --- a/lib/graphql/types/float.rb +++ b/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 diff --git a/lib/graphql/types/int.rb b/lib/graphql/types/int.rb index e9ec3d5531..f1ab5b9379 100644 --- a/lib/graphql/types/int.rb +++ b/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 diff --git a/lib/graphql/types/string.rb b/lib/graphql/types/string.rb index 3158b971d4..e0a87d87c8 100644 --- a/lib/graphql/types/string.rb +++ b/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) diff --git a/spec/graphql/types/float_spec.rb b/spec/graphql/types/float_spec.rb index ba2b802880..56f4697b45 100644 --- a/spec/graphql/types/float_spec.rb +++ b/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 diff --git a/spec/graphql/types/int_spec.rb b/spec/graphql/types/int_spec.rb index 1145ece0de..c5a2e6232a 100644 --- a/spec/graphql/types/int_spec.rb +++ b/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 diff --git a/spec/graphql/types/string_spec.rb b/spec/graphql/types/string_spec.rb index 008cdfb05a..e1e122021f 100644 --- a/spec/graphql/types/string_spec.rb +++ b/spec/graphql/types/string_spec.rb @@ -37,8 +37,7 @@ it "raises GraphQL::StringEncodingError" do err = assert_raises(GraphQL::StringEncodingError) { subject } - assert_equal "String \"\\x00\\x00\\x00foo\\xAD\\xAD\\xAD\" was encoded as ASCII-8BIT. GraphQL requires an encoding compatible with UTF-8.", err.message - assert_equal string, err.string + assert_equal "String cannot represent value: \"\\x00\\x00\\x00foo\\xAD\\xAD\\xAD\"", err.message end end @@ -57,14 +56,6 @@ def bad_string query(query_type) end } - - it "includes location in the error message" do - err = assert_raises GraphQL::StringEncodingError do - schema.execute("{ badString }") - end - expected_err = "String \"\\x00\\x00\\x00foo\\xAD\\xAD\\xAD\" was encoded as ASCII-8BIT @ badString (Query.badString). GraphQL requires an encoding compatible with UTF-8." - assert_equal expected_err, err.message - end end end @@ -145,4 +136,65 @@ def get_string(string:) assert_equal "\\u0064", res["data"]["example4"] end end + + describe "with Schema.spec_compliant_scalar_coercion_errors" do + class StringErrorsSchema < GraphQL::Schema + class Query < GraphQL::Schema::Object + field :string, GraphQL::Types::String do + argument :value, GraphQL::Types::String + end + + field :bad_string, GraphQL::Types::String + + def string(value:) + value + end + + def bad_string + "\0\0\0foo\255\255\255".dup.force_encoding("BINARY") + end + end + + query(Query) + end + + class StringSpecCompliantErrors < StringErrorsSchema + spec_compliant_scalar_coercion_errors true + end + + class StringNonSpecComplaintErrors < StringErrorsSchema + spec_compliant_scalar_coercion_errors false + end + + it "returns GraphQL execution errors with spec_compliant_scalar_coercion_errors enabled" do + query = "{ badString }" + result = StringSpecCompliantErrors.execute(query) + + assert_equal( + { + "errors" => [ + { + "message" => "String cannot represent value: \"\\x00\\x00\\x00foo\\xAD\\xAD\\xAD\"", + "locations" => [{ "line" => 1, "column" => 3 }], + "path" => ["badString"], + }, + ], + "data" => { + "badString" => nil, + } + }, + result.to_h + ) + end + + it "raises Ruby exceptions with spec_compliant_scalar_coercion_errors disabled" do + query = "{ badString }" + + error = assert_raises(GraphQL::StringEncodingError) do + StringNonSpecComplaintErrors.execute(query) + end + + assert_equal("String cannot represent value: \"\\x00\\x00\\x00foo\\xAD\\xAD\\xAD\"", error.message) + end + end end diff --git a/spec/support/dummy/schema.rb b/spec/support/dummy/schema.rb index 0fbcb6c7f1..441cf2c600 100644 --- a/spec/support/dummy/schema.rb +++ b/spec/support/dummy/schema.rb @@ -557,17 +557,6 @@ def self.resolve_type(type, obj, ctx) -> { Schema.types[obj.class.name.split("::").last] } end - # This is used to confirm that the hook is called: - MAGIC_INT_COERCE_VALUE = -1 - - def self.type_error(err, ctx) - if err.is_a?(GraphQL::IntegerEncodingError) && err.integer_value == 99**99 - MAGIC_INT_COERCE_VALUE - else - super - end - end - use GraphQL::Dataloader lazy_resolve(Proc, :call)