Skip to content
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

Error message for constrained, using format, string in struct is misleading #120

Closed
d-Pixie opened this issue Aug 10, 2016 · 3 comments
Closed
Labels

Comments

@d-Pixie
Copy link
Contributor

d-Pixie commented Aug 10, 2016

Using this setup:

require 'dry-types'

module Types
  include Dry::Types.module

  CountryCode = Types::Strict::String.constrained(format: /\A[A-Z]{2}\z/)
end

class Model < Dry::Types::Struct
  attribute :country_code, Types::CountryCode
end

When you use the type on its own you get an appropriate error message in the exception:

p Types::CountryCode['d2sg']
# => Exception: Dry::Types::ConstraintError: "d2sg" violates constraints (format?(/\A[A-Z]{2}\z/) failed)

But when used as an attribute in a struct it gives a misleading message about the values type instead of its format:

model = Model.new( country_code: 'd2sg' )
# => Exception: Dry::Types::StructError: [Model.new] "d2sg" (String) has invalid type for :country_code

I would hope that the context you use a type in should not change its error message, at least not to one that is wrong :)

@d-Pixie
Copy link
Contributor Author

d-Pixie commented Aug 10, 2016

As an additional note, that might be related, with my full type declaration:

  CountryCode = Types::Strict::String.constrained(format: /\A[A-Z]{2}\z/).constructor{|v| v.to_s.upcase[0...2]}

if I try to instantiate a model using a bogus value, 'd2ss', that should become 'D2' thanks to the constructor and then be checked against the constraint? I think the right value is checked against the constraint because if I use 'dss', which after the constructor is valid, but not before, it works as expected. But in the case where the value after the constructor is still not valid, as with 'D2', the error message contains the full original value 'd2ss', and not the post constructor conversion:

Exception: Dry::Types::StructError: [User.new] "d2ss" (String) has invalid type for :country_code

I don't know if this is done on purpose, to align with what the user actually sent in, or if it's a bug. Not even sure which I prefer :) Worth a quick discussion though.

@solnic
Copy link
Member

solnic commented Sep 10, 2016

We'll be improving error message handling for structs in dry-struct. There were lots of refactorings and clean ups in the way hash schemas work in dry-types and it should allow us to make error messages better for structs (which use hash schemas under the hood). This is a big improvement that will be definitely done before these gems hit 1.0.0.

@solnic
Copy link
Member

solnic commented Sep 22, 2016

This issue was moved to dry-rb/dry-struct#16

@solnic solnic closed this as completed Sep 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants