Include ValueError exception message in the result error

[georgysavva]

Hi. I really like how simple it is to use a custom validation function. You just pass a callable that checks and optionally converts the value, in cause of an error function can raise custom ValueInvalid error or just standard ValueError exception and it's very convenient since it allows you to use function that doesn't know anything about voluptuous and just raises ValueError exception.
But here is a problem. In case of ValueError exception the original message won't be include in the result ValueInvalid error so we lose some information about that exactly was invalid with this value. If validation function raises ValueInvalid exception everything is fine and the message is included.

Imagine the following example, I have a function that parses datetime and ensures that it contains timezone info, i.e. it's aware.:

import datetime
from voluptuous import Schema

def parse_aware_dt(value):
    # if value has an invalid format it will raise ValueError with the details.
    dt = datetime.datetime.fromisoformat(value)
    if dt.tzinfo is None:
        raise ValueError("datetime doesn't contain timezone info")
    return dt

schema = Schema({'started_at': parse_aware_dt})

# Value can't be parsed as datetime.
result = schema({'started_at': 'not a datetime at all'})

# Value is a datetime, but it doesn't contain timezone.
result = schema({'started_at': '2019-12-21T12:00:13.075029'})

In both cases we will get the same error:
not a valid value for dictionary value @ data['started_at']
Without explanation that exactly wrong with the value, as it would be if the function were raising custom ValueInvalid instead.

Here is the part of the code where it happens:

voluptuous/voluptuous/schema_builder.py

Lines 815 to 822 in 45ba818

	def validate_callable(path, data):
	try:
	return schema(data)
	except ValueError as e:
	raise er.ValueInvalid('not a valid value', path)
	except er.Invalid as e:
	e.prepend(path)
	raise

My suggestion is to do it like that instead:

     except ValueError as e: 
         raise er.ValueInvalid('not a valid value: %s' % e, path)

If you like my proposal I can submit a pull request. Thanks!

[alecthomas]

As you can imagine, I did try that early on, but the error messages from ValueError are often terrible. The solution in Voluptuous is to pass a msg argument.

[georgysavva]

Ah ok. Make sense. If you tried this, ,maybe you could share a case when the error message from ValueError is terrible. Just curious. Thanks!