Passing context information in errors

[foca]

Something I've run into often is that validation errors can't be fully compressed into a single code. A common enough case is validating length:

assert_length :username, 3..16

If the user's provided username was out of bounds, we'd get a { foo: [:not_in_range] } error. Now, the error message we want to render will most likely need to include the range in which it is valid. Something like "The username must be between 3 and 16 characters long".

However, to generate this error, we need to know the range of validity in the view, which means duplicating knowledge. This is worsened when you have multiple front-ends, and the API serialises error codes instead of rendered messages due to i18n).

When the form changes, we need to remember to change the error message in the serialiser and/or probably in the web views if the app also has those.

It would be useful, then, to include context information about the failure in the error itself, so that clients that render this error have all the information from the single source of truth regarding validations: the Scrivener object.

The solution I used (which is probably not be the best, but it worked for me) was to introduce a Scrivener::Error object that emulated the error symbol, but added a context attribute. Essentially:

class Error < SimpleDelegator
  attr_reader :context
  def initialize(code, context = nil)
    super(code)
    @context = context
  end
end

And then went and copied scrivener/validations into the project and replaced all the assertions with:

def assert_length(att, range, error = [att, Error.new(:not_in_range, range)])
  …
end

This works, and particularly, kept compatibility with all our existing forms as the error "is still" a Symbol. However, seems a bit heavy handed to wrap an (extra) object around every error code.

While writing this issue, I thought of an alternative, by changing errors into 2-element pairs:

def assert_length(att, range, error = [att, [:not_in_range, range]])
  …
end


def assert_url(att, error = [att, [:not_url]]) # or [:not_url, nil] to be explicit ¯\_(ツ)_/¯
  …
end

This is pretty minimalistic, and a bit more memory efficient than the above solution, but would break everyone's code if implemented into Scrivener 😄

Is this a "problem" only for me?

We introduced this when a requirement changed and we had to wait for a new release of the mobile application to support these contexts in the API before we were able to implement the change in the business rule itself, which was a bit annoying, as mobile apps aren't particularly fast at upgrading 😛

Is this worth changing in Scrivener itself? Or is everyone fine with duplicating error contexts in wherever you generate error messages / serialise responses to your APIs / whatever.

[frodsan]

I had that problem too: https://gist.github.com/frodsan/e3547db964c32d9a210f3d2bd58ef2a7

It would be nice to have context information. I don't know if that's easy to do without breaking code, maybe error can be an object?

[soveran]

I've never run into this, but I think it's mainly because I don't create applications often. I agree with all the points: having the context for some particular assertions looks necessary, but creating an object for each error looks too heavy and changing Scrivener to push an array of [<code>, <context>] would break existing apps. Another solution is to customize the error message when defining the validation, but this also looks a bit cumbersome to work with as it would also require custom handling in the views:

assert_length :username, 3..16, [:username, [:not_in_range, 3..16]]

To avoid the repetition, a custom assertion can be defined:

def assert_in_range(att, range)
  assert_length(att, range, [att, [:not_in_range, range]])
end

Then:

assert_in_range :username, 3..16

We can keep thinking about other ideas.

[soveran]

@foca I'm thinking again about creating an object. It breaks compatibility, but on the other hand it may be a good approach. It would be ideal if we could create the object only if the validation fails (we could, for example, initialize the error as nil and create the Error object only if an error wasn't provided and if the validation failed). The other idea of returning an array is also good. I think we should think first about that option and how we can present the error in the view if what we have is an array and not a symbol. If we can't make that work, we can continue with the Error class.

[foca]

I think we should think first about that option and how we can present the error in the view if what we have is an array and not a symbol.

A naive implementation could be:

messages = form.errors[field].map do |(code, context)|
  ERRORS.fetch(code).call(*context) # note the splat to ignore when `context` is `nil`
end

ERRORS = {
  not_present: -> { "can't be empty".freeze },
  too_short: ->(val) { "must be at least %d characters".freeze % val },
  not_in_range: ->(rng) { "must be between %d and %d".freeze % [rng.first, rng.last] },
  # etc
}

(This is mostly what I'm doing already)

[soveran]

Perfect, it looks good to me :-)

[foca]

Cool, will draft a PR then :D

On Fri, May 20, 2016 at 11:08 PM Michel Martens notifications@github.com
wrote:

Perfect, it looks good to me :-)

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#22 (comment)