-
-
Notifications
You must be signed in to change notification settings - Fork 24
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
Wish there was even more detailed error handling in sanctuary-def #274
Comments
This feature is much more popular than I initial thought. #220 and #219 are also about this. Part I@davidchambers posted a proof-of-concept pure function on gitter that provides good error messages in English. The drawbacks being that it's painful to translate the error messages into user facing messages. As mention in both #219 and #220, there already exist an exposed (though supposedly private) method to validate types on each type. My initial thought was to translate the internal (to sanctuary-def) error message into 3 well-define error messages. As far as I can tell there is only 5 possible outcome of an validation of any type (unless we bake in interconnected rules, which we will not).
Currently we have This comment is going to be long so I'm splitting it into several comments... |
Part IIMy first attempts was to create a pure function that is separate from sanctuary-def, so its immediately available to try out in projects without a new release of sanctuary-def. Having well-defined error messages/objects will ease translation of said errors to public facing error messages. E.g. "Start date is not a valid date"/"Start dato er ikke en godkendt dato" or "Start date is missing"/"Start dato mangler" etc. Example: const BarFoo = $.NamedRecordType
('BarFoo')
('')
([])
({
foo: $.String,
bar: $.NonEmpty (
$.NullaryType
('DateIso')
('')
([$.String])
(x => {
console.debug (`test ran with x = ${x}`);
return /^\d{4}-[0-1]\d-[0-3]\d$/.test (x);
})
),
fez: $.Number,
}); Examining the return types from // 1.1
BarFoo.validate ($.env) (null)
// -> Left ({"propPath": [], "value": null})
// 1.2
BarFoo.validate ($.env) (undefined)
// -> Left ({"propPath": [], "value": undefined})
// 2.1
BarFoo.validate ($.env) ({foo: 'blue'})
// -> Left ({"propPath": [], "value": {"foo": "blue"}})
// 2.2
BarFoo.validate ($.env) ({foo: 'blue', bar: '2020-10-10'})
// -> Left ({"propPath": [], "value": {"bar": "2020-10-10", "foo": "blue"}})
// 3.1
BarFoo.validate ($.env) ({foo: 'blue', bar: '2020-10-10', fez: 2, lorem:"zzz"})
// -> Right ({"bar": "2020-10-10", "fez": 2, "foo": "blue", "lorem": "zzz"})
// test ran with x = 2020-10-10
// 3.2
BarFoo.validate ($.env) ({foo: 'blue', bar: '2020-10-10', fez: 2, lorem:"zzz", ipsum: 123})
// -> Right ({"bar": "2020-10-10", "fez": 2, "foo": "blue", "ipsum": 123, "lorem": "zzz"})
// test ran with x = 2020-10-10
// 4.1
BarFoo.validate ($.env) ({foo: 'blue', bar: '2020-29-10', fez: 2})
// -> Left ({"propPath": ["bar", "$1"], "value": "2020-29-10"})
// test ran with x = 2020-29-10
// 4.2
BarFoo.validate ($.env) ({foo: 'blue', bar: '2020-29-10', fez: 'lol'})
// -> Left ({"propPath": [], "value": {"bar": "2020-29-10", "fez": "lol", "foo": "blue"}})
// 5
BarFoo.validate ($.env) ({foo: 'blue', bar: '2020-10-10', fez: 2})
// -> Right ({"bar": "2020-10-10", "fez": 2, "foo": "blue"})
// test ran with x = 2020-10-10 Initially it looks like converting the internal error objects to well-define error objects is straight forward. I think we can ignore extraneous properties in 3 and 5 is obviously correct. However there is ambiguity between multiple properties being wrong (4.2) and 2, one or more properties are missing. Also in both 2 and 4.2, it is not enough to examine the return value of In that case it would probably be better to leverage the For our Type Type ~> Array Type ~> (Value -> Boolean) Which brings me to part III. |
Part IIIThe original proof-of-concept function from @davidchambers: // validate :: Type -> a -> Either (Array String) a
const validate = t => x => {
if (x == null) {
return Left ([show (x) + ' is not a member of ‘' + show (t) + '’']);
}
const missingFields = t.keys.filter (
k => !(Object.prototype.propertyIsEnumerable.call (x, k))
);
if (missingFields.length > 0) {
return Left (missingFields.map (k => show (k) + ' field is missing'));
}
for (const fieldName of t.keys) {
const fieldType = t.types[fieldName];
const fieldValue = x[fieldName];
if (!(fieldType._test ([]) (fieldValue))) {
return Left ([
'Value of ' + show (fieldName) + ' field, ' + show (fieldValue) +
', is not a member of ‘' + show (fieldType) + '’',
]);
}
}
return Right (x);
}; This works perfectly for none-nested types. // FooBar :: Type
const FooBar = $.NamedRecordType
('FooBar')
('')
([])
({foo: $.String,
bar: $.Number});
eq (validate (FooBar) (null))
(Left (['null is not a member of ‘FooBar’']));
eq (validate (FooBar) (undefined))
(Left (['undefined is not a member of ‘FooBar’']));
eq (validate (FooBar) ({}))
(Left (['"bar" field is missing', '"foo" field is missing']));
eq (validate (FooBar) ({foo: null}))
(Left (['"bar" field is missing']));
eq (validate (FooBar) ({foo: null, bar: null}))
(Left (['Value of "bar" field, null, is not a member of ‘Number’']));
eq (validate (FooBar) ({foo: null, bar: 42}))
(Left (['Value of "foo" field, null, is not a member of ‘String’']));
eq (validate (FooBar) ({foo: 'blue', bar: 42}))
(Right ({foo: 'blue', bar: 42})); But this solution won't work for validate (BarFoo) ({foo: 'blue', bar: '2020-10-10', fez: 2})
// -> Right ({"bar": "2020-10-10", "fez": 2, "foo": "blue"})
validate (BarFoo) ({foo: 'blue', bar: '2020-29-10', fez: 2})
// -> Right ({"bar": "2020-29-10", "fez": 2, "foo": "blue"}) A wrong date for our very simple test is accepted. I could add test for super types to // validate :: Type -> a -> Either (Array String) a
const validate = t => x => {
if (x == null) {
return S.Left ([S.show (x) + ' is not a member of ‘' + S.show (t) + '’']);
}
const missingFields = t.keys.filter (
k => !(Object.prototype.propertyIsEnumerable.call (x, k))
);
if (missingFields.length > 0) {
return S.Left (missingFields.map (k => S.show (k) + ' field is missing'));
}
for (const fieldName of t.keys) {
const fieldType = t.types[fieldName];
const fieldValue = x[fieldName];
if (!(fieldType._test (fieldType.supertypes) (fieldValue))) {
return S.Left ([
'Value of ' + S.show (fieldName) + ' field, ' + S.show (fieldValue) +
', is not a member of ‘' + S.show (fieldType) + '’',
]);
}
}
return S.Right (x);
}; But the issue is not that ERRATA: It turns out that |
Part IVI believe the following change to make // validate :: Type -> a -> Either (Array String) a
const validate = t => x => {
if (x == null) {
return S.Left ([S.show (x) + ' is not a member of ‘' + S.show (t) + '’']);
}
const missingFields = t.keys.filter (
k => !(Object.prototype.propertyIsEnumerable.call (x, k))
);
if (missingFields.length > 0) {
return S.Left (missingFields.map (k => S.show (k) + ' field is missing from ' + S.show (t)));
}
for (const fieldName of t.keys) {
const fieldType = t.types[fieldName];
const fieldValue = x[fieldName];
if (!(fieldType._test ($.env) (fieldValue))) {
return S.Left ([
'Value of ' + S.show (fieldName) + ' field, ' + S.show (fieldValue) +
', is not a member of ‘' + S.show (fieldType) + '’',
]);
}
if (fieldType.keys.length) {
return validate (fieldType)
(S.foldMap (Object)
(k => ({ [k]: x[fieldName] }))
(fieldType.keys));
}
}
return S.Right (x);
}; I realize that my version relies on Sanctuary which will create a cyclic dependency but if the idea works out, we can convert it to an validate (BarFoo) ({foo: 'blue', bar: '2020-10-10', fez: 2})
// -> Right ({"$1": "2020-10-10"})
// test ran with x = 2020-10-10
validate (BarFoo) ({foo: 'blue', bar: '2020-29-10', fez: 2})
// -> Left (["Value of \"$1\" field, \"2020-29-10\", is not a member of ‘DateIso’"])
// test ran with x = 2020-29-10 Unfortunately I'm running out of time and are AFK the following week, so won't have time to do the next obvious steps.
But for sure there will be a part V and VI! ;) @davidchambers What do you think? |
I have a use case where I have a RecordType like this:
And I use this function to validate it:
newProductRequestBodyType
is a post request, where I want to validate that each field is valid. Is there a simple way, to provide an error message for each field, so that I can show the right error message to the form on the client?The
Left
just gives me the sent object back, but it doesn't say why the validation of the Recordtype failed, if that makes sense?It would be super-useful if the Left contained more details about the error.... Maybe even something I define myself when I create my RecordType.
(This issue was first mentioned on Gitter).
The text was updated successfully, but these errors were encountered: