Skip to content

Commit

Permalink
Improve error messages
Browse files Browse the repository at this point in the history
  • Loading branch information
jaynetics committed Nov 16, 2024
1 parent cf7b3d4 commit b8313b0
Show file tree
Hide file tree
Showing 11 changed files with 50 additions and 18 deletions.
2 changes: 1 addition & 1 deletion lib/taro/errors.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
class Taro::Error < StandardError
def message
# clean up newlines introduced when setting the message with a heredoc
super.chomp.tr("\n", ' ')
super.chomp.sub(/\n(?=\S)/, ' ')
end
end

Expand Down
1 change: 1 addition & 0 deletions lib/taro/rails/declaration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ def add_openapi_names(controller_class:, action_name:)
params.openapi_name = "#{base}_Input"
returns.each do |status, return_type|
return_type.openapi_name = "#{base}_#{status}_Response"
return_type.define_singleton_method(:name) { openapi_name }
end
end

Expand Down
2 changes: 1 addition & 1 deletion lib/taro/types/coercion.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ def call(arg)

def validate_hash(arg)
arg.is_a?(Hash) || raise(Taro::ArgumentError, <<~MSG)
Type coercion argument must be a Hash, got: #{arg.inspect} (#{arg.class})
Type coercion argument must be a Hash, got: #{arg.class}
MSG

types = arg.slice(*KEYS)
Expand Down
12 changes: 2 additions & 10 deletions lib/taro/types/field.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ def retrieve_response_value(object, context, object_is_hash)
elsif object.respond_to?(method, true)
object.public_send(method)
else
raise_response_coercion_error(object)
# Note that the ObjectCoercion module rescues this and adds context.
raise Taro::ResponseError, "No such method or resolver `:#{method}`."
end
end

Expand All @@ -65,14 +66,5 @@ def coerce_value(value, from_input)

type_obj = type.new(value)
from_input ? type_obj.coerce_input : type_obj.coerce_response
rescue Taro::Error => e
raise e.class, "#{e.message}, after using method/key `:#{method}` to resolve field `#{name}`"
end

def raise_response_coercion_error(object)
raise Taro::ResponseError, <<~MSG
Failed to coerce value #{object.inspect} for field `#{name}` using method/key `:#{method}`.
It is not a valid #{type} value.
MSG
end
end
4 changes: 3 additions & 1 deletion lib/taro/types/shared/errors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ def response_error(msg)
end

def coerce_error_message(msg)
"#{object.inspect} (#{object.class}) is not valid as #{self.class}: #{msg}"
type_desc = (self.class.name || self.class.superclass.name)
.sub(/^Taro::Types::.*?([^:]+)Type$/, '\1')
"#{object.class} is not valid as #{type_desc}: #{msg}"
end
end
13 changes: 13 additions & 0 deletions lib/taro/types/shared/object_coercion.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ module Taro::Types::Shared::ObjectCoercion
def coerce_input
self.class.fields.transform_values do |field|
field.value_for_input(object)
rescue Taro::Error => e
raise_enriched_coercion_error(e, field)
end
end

Expand All @@ -11,6 +13,17 @@ def coerce_response
object_is_hash = object.is_a?(Hash)
self.class.fields.transform_values do |field|
field.value_for_response(object, context: self, object_is_hash:)
rescue Taro::Error => e
raise_enriched_coercion_error(e, field)
end
end

def raise_enriched_coercion_error(error, field)
# The indentation is on purpose. These errors can be recursively rescued
# and re-raised by a tree of object types, which should be made apparent.
raise error.class, <<~MSG
Failed to read #{self.class.name} field `#{field.name}` from #{object.class}:
#{error.message.lines.map { |line| " #{line}" }.join}
MSG
end
end
2 changes: 1 addition & 1 deletion spec/taro/rails/declaration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@
params = ActionController::Parameters.new(user: { name: nil })
expect do
subject.parse_params(params)
end.to raise_error(Taro::InputError, /nil.*must be a String/)
end.to raise_error(Taro::InputError, /NilClass is not valid as String/)
end
end
end
4 changes: 2 additions & 2 deletions spec/taro/types/enum_type_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@
it 'coerces input' do
expect(example.new('foo').coerce_input).to eq 'foo'
expect { example.new('qux').coerce_input }
.to raise_error(Taro::InputError, '"qux" (String) is not valid as MyEnum: must be "foo" or "bar"')
.to raise_error(Taro::InputError, 'String is not valid as MyEnum: must be "foo" or "bar"')
end

it 'coerces response data' do
expect(example.new('foo').coerce_response).to eq 'foo'
expect { example.new('qux').coerce_response }
.to raise_error(Taro::ResponseError, '"qux" (String) is not valid as MyEnum: must be "foo" or "bar"')
.to raise_error(Taro::ResponseError, 'String is not valid as MyEnum: must be "foo" or "bar"')
end

it 'raises for empty enums' do
Expand Down
2 changes: 1 addition & 1 deletion spec/taro/types/field_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
field = described_class.new(name: :foo, type: S::StringType, null: false)
expect do
field.value_for_response(42, object_is_hash: false)
end.to raise_error(Taro::RuntimeError, /Failed to coerce/)
end.to raise_error(Taro::ResponseError, /No such method or resolver `:foo`/)
end
end
end
2 changes: 1 addition & 1 deletion spec/taro/types/object_types/page_type_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
example.new(ActiveRecord::Relation.new).coerce_response(after: 'cursor')
end.to raise_error(
Taro::ResponseError,
'42 (Integer) is not valid as Taro::Types::Scalar::StringType: must be a String or Symbol'
'Integer is not valid as String: must be a String or Symbol'
)
end

Expand Down
24 changes: 24 additions & 0 deletions spec/taro/types/shared/object_coercion_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
describe Taro::Types::Shared::ObjectCoercion do
it 'raises instructive errors for nested fields' do
stub_const('InnerType', Class.new(T::ObjectType) do
field(:str_field, type: 'String', null: false)
end)
outer_type = stub_const('OuterType', Class.new(T::ObjectType) do
field(:inner_field, type: 'InnerType', null: false)
end)

object = double(
class: 'OuterRecord', inner_field: double(
class: 'InnerRecord', str_field: 123
)
)

expect do
outer_type.new(object).coerce_response
end.to raise_error Taro::ResponseError, <<~MSG.chomp
Failed to read OuterType field `inner_field` from OuterRecord:
Failed to read InnerType field `str_field` from InnerRecord:
Integer is not valid as String: must be a String or Symbol
MSG
end
end

0 comments on commit b8313b0

Please sign in to comment.