-
Notifications
You must be signed in to change notification settings - Fork 63
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Difficulty adding error codes to SerializableActiveModelError #86
Comments
Hi Adam, thanks for the in depth write up.
Errors support is indeed largely perfectible. The main issue is that Rails’
ActiveModel doesn’t report errors individually, which breaks the “one
object – one serializer” paradigm followed for normal rendering in
jsonapi-rb.
I am currently fairly busy so I won’t be able to come up with an elegant
solution in the coming weeks, but I’d review a PR in case you’d like to
take a stab.
On Thu 26 Apr 2018 at 22:10, Adam Prescott ***@***.***> wrote:
I work on an app where one of our API clients would like to customize an
error message shown to the user. The server is using jsonapi-rails (version
0.3.1), with the default error serializer, SerializableActiveModelErrors.
The Rails app in this case runs errors.add(:base, ...) and the error
value could be one of many types.
There are title, detail, and source attributes on errors
<https://github.com/jsonapi-rb/jsonapi-rails/blob/v0.3.1/lib/jsonapi/rails/serializable_active_model_errors.rb#L5-L15>,
but there's no natural way for the client to override a specific error
message. The client could check title === "An error title value" and
customize that, but the server may change title over time, since it's
defined by the JSON API spec to be a human-readable summary.
What we'd like is to use code, which JSON API supports
<http://jsonapi.org/format/#error-objects>:
An error object MAY have the following members:
[...]
- code: an application-specific error code, expressed as a string
value.
This would let us detect, say, code === "foo", and even if title changes
the client would continue with its custom error message.
In our case, we plan on detecting errors.add(:base, :foo), seeing that
:foo is a symbol, and treating that as an error code. (If the Rails app
ever used a string, as with errors.add(:base, "There was an error"), that
would not result in a code.)
This seems to be fairly tricky to accomplish.
Right now, the default error class of SerializableActiveModelErrors is
configurable, but SerializableActiveModelErrors is private. If we were to
ignore that it's private and, say, subclass SerializableActiveModelErrors,
it looks like we'd need to reimplement code related to the exposures
object
<https://github.com/jsonapi-rb/jsonapi-rails/blob/v0.3.1/lib/jsonapi/rails/serializable_active_model_errors.rb#L27-L34>
:
def initialize(exposures)
@errors = exposures[:object]
@reverse_mapping = exposures[:_jsonapi_pointers] || {}
freezeend
Another issue is that SerializableActiveModelError (singular), which is
also private, doesn't have access to the original errors object (or one
of the single errors), and so it isn't possible to retrieve the :foo
corresponding to errors.add(:base, :foo).
As far as I can tell, we'd have to more or less implement our own error
serialization to pull code values out of ActiveModel errors.
Here's a very monkey-patch-y implementation of a serializer in the way I'm
describing.
class CodedSerializableActiveModelErrors < ::JSONAPI::Rails::SerializableActiveModelErrors
def as_jsonapi
# Example: { email: [{ error: :blank }] }
@errors.details.flat_map do |key, attribute_errors|
attribute_errors.map do |error:|
# full_message(:email, generate_message(:email, :blank))
full_message = @errors.full_message(key, @errors.generate_message(key, error))
error_attributes = ::JSONAPI::Rails::SerializableActiveModelError.new(field: key, message: full_message, pointer: @reverse_mapping[key]).as_jsonapi
if error_attributes.key?(:code)
raise "Code already provided by the library. Refusing to override."
end
# Override the error_attributes this way so that we inherit all SerializableActiveModelError
# attributes, relationships, etc.
if error
error_attributes[:code] = error.to_s
end
error_attributes
end
end
endend
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#86>, or mute the
thread
<https://github.com/notifications/unsubscribe-auth/ACHPYgYL8cku2cnfuqYrURmdLJCSxcFEks5tsimdgaJpZM4TnajR>
.
--
Lucas Hosseini
[email protected]
|
Hi @beauby! Thanks for the quick reply. There may be some design decisions to make around how to allow defining codes. For our application, we can enforce It was definitely not easy to work out how to unwrap multiple errors per attribute. |
A small edge case, in case anyone tries this: note that using obj.errors.details
# => {:code=>[{:error=>:taken, :value=>"anything"}, {:error=>:taken}]}
obj.errors.messages
# => {:code=>["has already been taken"]}
obj.errors.keys.map { |k| obj.errors.full_messages_for(k) }
# => [["Code has already been taken"]] Just a thing to watch out for, in complex cases where there are many paths to an attribute error and they all run. |
EDIT: I wrote this out but now I actually think this first method is bad i.e. This would probably be super flexible but also maybe overkill BETTER? # config/jsonapi_error_mappings.yml
taken:
code: taken,
status: unprocessable_entity,
about_link_path: email_taken_help_path
email:
taken:
code: email_taken |
This could handle everything aside from |
I've done this... I don't particularly like polluting the I18n locales but it's working for us atm # app/serializable/serializable_active_model_errors.rb
class SerializableActiveModelError < JSONAPI::Serializable::Error
title do
"Invalid #{@field}" unless @field.nil?
end
detail do
@message
end
source do
pointer @pointer unless @pointer.nil?
end
code do
if I18n.exists?(key = "#{@field}.#{@error_key}.code", :jsonapi)
I18n.t(key, locale: :jsonapi)
elsif I18n.exists?(key = "#{@error_key}.code", :jsonapi)
I18n.t(key, locale: :jsonapi)
end
end
status do
if I18n.exists?(key = "#{@field}.#{@error_key}.status", :jsonapi)
I18n.t(key, locale: :jsonapi)
elsif I18n.exists?(key = "#{@error_key}.status", :jsonapi)
I18n.t(key, locale: :jsonapi)
end
end
end
class SerializableActiveModelErrors
def initialize(exposures)
@errors = exposures[:object]
@reverse_mapping = exposures[:_jsonapi_pointers] || {}
freeze
end
def as_jsonapi
@errors.keys.flat_map do |key|
@errors.full_messages_for(key).map.with_index do |message, index|
SerializableActiveModelError.new(
field: key,
message: message,
pointer: @reverse_mapping[key],
error_key: @errors.details[key][index][:error]
).as_jsonapi
end
end
end
end # config/locales/jsonapi.yml
jsonapi:
email_not_verified:
code: email_not_verified
status: unprocessable_entity You'll need to set the error serializer class correctly. I modified the configuration of jsonapi-rails here #92 which makes it a lot easier to customize the default classes # frozen_string_literal: true
require_relative '../../app/serializable/serializable_active_model_errors'
JSONAPI::Rails.configure do |config|
# # Set a default serializable class mapper
# # Can be a Proc or any object that responds to #call(class_name)
# # e.g. MyCustomMapper.call('User::Article') => User::SerializableArticle
# config.jsonapi_class_mapper = -> (class_name) do
# names = class_name.to_s.split('::')
# klass = names.pop
# [*names, "Serializable#{klass}"].join('::').safe_constantize
# end
#
# # Set any default serializable class mappings
# config.jsonapi_class_mappings = {
# :Car => SerializableVehicle,
# :Boat => SerializableVehicle
# }
#
# # Set a default serializable class mapper for errors.
# # Can be a Proc or any object that responds to #call(class_name)
# # e.g. MyCustomMapper.call('PORO::Error') => PORO::SerializableError
# # If no jsonapi_errors_class_mapper is configured jsonapi_class_mapper will
# # be used
# config.jsonapi_errors_class_mapper = config.jsonapi_class_mapper.dup
#
# # Set any default serializable class errors mappings
config.jsonapi_errors_class_mappings = {
:'ActiveModel::Errors' => SerializableActiveModelErrors,
:Hash => JSONAPI::Rails::SerializableErrorHash
} |
I work on an app where one of our API clients would like to customize an error message shown to the user. The server is using jsonapi-rails (version 0.3.1), with the default error serializer,
SerializableActiveModelErrors
.The Rails app in this case runs
errors.add(:base, ...)
and the error value could be one of many types.There are
title
,detail
, andsource
attributes on errors, but there's no natural way for the client to override a specific error message. The client could checktitle === "An error title value"
and customize that, but the server may changetitle
over time, since it's defined by the JSON API spec to be a human-readable summary.What we'd like is to use
code
, which JSON API supports:This would let us detect, say,
code === "foo"
, and even iftitle
changes the client would continue with its custom error message.In our case, we plan on detecting
errors.add(:base, :foo)
, seeing that:foo
is a symbol, and treating that as an error code. (If the Rails app ever used a string, as witherrors.add(:base, "There was an error")
, that would not result in a code.)This seems to be fairly tricky to accomplish.
Right now, the default error class of
SerializableActiveModelErrors
is configurable, butSerializableActiveModelErrors
is private. If we were to ignore that it's private and, say, subclassSerializableActiveModelErrors
, it looks like we'd need to reimplement code related to the exposures object:Another issue is that
SerializableActiveModelError
(singular), which is also private, doesn't have access to the originalerrors
object (or one of the single errors), and so it isn't possible to retrieve the:foo
corresponding toerrors.add(:base, :foo)
.As far as I can tell, we'd have to more or less implement our own error serialization to pull
code
values out of ActiveModel errors.Here's a very monkey-patch-y implementation of a serializer in the way I'm describing.
The text was updated successfully, but these errors were encountered: