Potential error issue and question about error handling
iranicus opened this issue · 6 comments
Hey there,
I've been playing around with ts-interface-checker and the builder in hopes of putting together a demo for some colleagues to push the idea of using a runtime json validator for the data we receive in some API's to ensure we get what we expect, in the right format and check for any extra properties received. I had a go with the check, checkStrict, test and testStrict functions and upon using the check functions when passing data with multiple invalid properties like an extra property or an invalid data type I noticed that the VError thrown in the console reports only the first one it finds and not all errors. Having fixed the first invalid property sure enough when the code was re-ran it then threw another VError for the next one. I'm not sure if this is the intended functionality or not as I expected to see it report on all errors so I wanted to raise it here.
I was also wondering if it is possible to currently receive the results from a check as an object so I can extract what I need to put into an error message or log since what I am looking to do is run the testStrict function, check the boolean returned then if false, run the checkStrict function for the data which would return an object with the results of the offending properties. So far I have just put the checkStrict in a try catch and caught the VError to access the message property which is ok but not ideal.
The second request is easier. E.g. we could add an interface such as validate(value) and validateStrict(value) which return null on success and an error message on error.
The first one is harder. A big goal is performance, so for the fast test() functions (which just return true or false), we want to return early as soon as we know the object is invalid. Unfortuately, the code is organized (intenionally) to have a single implementation both for the fast test() functions and the slow check() functions. So allowing to either exit early or not would involve some bigger refactorings.
In this case I would be just happy receiving the results in an object then with the checker results so I can run the testStrict() first to see if there are in fact any issues, then if there is I can run the checkStrict() to obtain an object with the error details ideally containing the offending properties and the problems. For instance i know currently if a property is missing it reports value.propertyName is missing, so if there were multiple issues then I would be able to access both the these in two parts (if possible) with a structure something like an array of JSON objects containing the property name and the message (in this example is the "is missing" message, or could be just "missing"). This would then allow me to iterate through the results and execute logic depending on the parsed message, at the moment I have been using a try catch to catch the VError where I have been accessing the message of the error which I have been able to parse but the problem here was that it only reported the first property discrepancy it encountered (thus the first point I mentioned) which would be solved with the returned object approach.
Could you check out this pull request and let me know what you think: #4 ? It adds Checker.validate() and Checker.strictValidate() methods that return null on success and detailed error on error. See the new test case for an example of the return value. It still stops on the first error though -- that's harder to change.
@dsagal Just had a go with it now, looks good, good job, the only thing indeed would be a way to return a report of multiple errors at once with the Checker.validate() and Checker.strictValidate() to avoid repetition after fixing the first error that it finds.
v1.0 now returns multiple errors from validate() (#47) so I think this can be closed.