class-validator
class-validator copied to clipboard
feat: validateOrReject should throw instance of Error
validateOrReject throws an array of errors that does not have a stack.
That is bad in case such error is not being caught and results in 500 Internal Server Error.
Is it possible to add something like validateOrError that would throw an instance like this instead:
class ObjectInvalid extends Error {
errors: ValidationError[]
}
I do not understand why you need another method. In case you use validateOrReject you know promise is rejected and you have to handle it. Otherwise you will use validateSync
Not all of the rejects are being caught and processed. It means that if the error is displayed in console or sent to exception notification software, you will have no error stack available.
Example jest error when validateOrReject throws:
161 | });
162 |
> 163 | it('fails', async () => {
| ^
164 | await validateOrReject(new Reseller())
165 | });
166 | });
You may see that jest can only tell the test where the error occurred not the line number and backtrace.
When you see something like this in console:
Failed: Array [
ValidationError {
...
},
]
You have no idea where to fix it.
Any linter will tell you that throwing something that doesn't inherit Error class is a bad idea:
https://palantir.github.io/tslint/rules/no-string-throw/ https://eslint.org/docs/rules/no-throw-literal
BTW, this is first time I see someone throws an Array. IMO it is even more terrible idea than throwing a string because when you see it in console you don't even realize what had happened.
Sure I understand now. But it is breaking change. We have to bundle this to next major release.
You can avoid breaking change by intoroducing a different method like validateOrThrow and maybe deprecating the old one as I suggested initially.
Sure why not. Let's introduce new validateOrThrow method and mark validateOrReject as deprecated.
@NoNameProvided @vlapo Any release date on this?
I don't understand, the stack trace will be useless to you because it will originate inside class-validator and not your code.
You can already check if your received error is an instance of the ValidationError and process it in your error handler in a single place.
@NoNameProvided the problem is that we don't get an instance of ValidationError but an array which contains an object of ValidationError instance
Well then you can check via
Array.isArray(error) && error.every(e => e instanceof ValidationError)
right?
Nevertheless, as mentioned in other places, the error format will be reworked, but I don't want to start ot monkey-patch it before that.
@NoNameProvided I guess I can do so, and I definitely will, but what I think is something like this:
- Executing validation
- An error is raised
- I get the error from promise rejection which contains a single error per time (no array)
This could be really useful for an API consumer.
Also, it would be:
Array.isArray(err) && err.every(e => e instanceof ValidationError);
I don't understand, the stack trace will be useless to you because it will originate inside class-validator and not your code.
Having adequate stacktraces is the core feature of any programming language. JS is the only place I saw where it is sometimes broken. Most stacktraces include both a library and application code because they are stack traces. This is only a problem of promises that lose a part of stacktrace under special conditions because of async call, but it should not be the case if you do it right.
The reason you need stacktrace is when you use the library to validate data that is not a user input. Like you are reading data from other API or there is just some tables in your DB where data need to follow certain convention and you communicate to your team what kind of format is required.
On the high level, you need to distinguish the user input error and the application internal error that it doesn't follow its own data conventions. You need stacktraces to locate where second type of error occur and how to fix it.
Typical example: a fake data for your tests that is not a user input, but need to be validated like a user input before running the actual test.
I still think that the most smooth way of developing a different error format is a different method like validateOrThrow that will allow to change the format without introducing an instant breaking change.
Nevertheless, as mentioned in other places, the error format will be reworked, but I don't want to start ot monkey-patch it before that.
Can you please give a link to the source discussion on that?
I agree on this since some packages do check errors with error instanceof Error and if not, they just wrap it up like new Error(error). Which invokes ValidationError.prototype.toString.
In my case, apollo-server does. When I trying to access error in formatError, the error.originalError property is an direct instance of Error so unable to access ValidationError properties.
Any updates on this?