feat: add validationErrors to schema mismatch errors#641
feat: add validationErrors to schema mismatch errors#641MoLow wants to merge 3 commits intofastify:mainfrom
Conversation
ivan-tymoshenko
left a comment
ivan-tymoshenko
left a comment
There was a problem hiding this comment.
I like the idea, but I don't uunderstand the purpose of accumulating errors. Why we can't throw them if validation fails without accumulating?
with |
|
Oh, I see. Can you modify the |
MoLow
force-pushed
the
add-validation-errors
branch
from
fe701e4 to
04d0b3c
Compare
|
@ivan-tymoshenko @mcollina is there anything else missing to get this merged? |
| else throw Object.assign(new TypeError(\`The value of '${schemaRef}' does not match schema definition.\`), { | ||
| validationErrors: errors | ||
| }) |
There was a problem hiding this comment.
Why not use cause and avoid the Object.assign?
| else throw Object.assign(new TypeError(\`The value of '${schemaRef}' does not match schema definition.\`), { | |
| validationErrors: errors | |
| }) | |
| else throw new TypeError(\`The value of '${schemaRef}' does not match schema definition.\`, { | |
| cause: errors | |
| }) |
Co-authored-by: Uzlopak <aras.abbasi@googlemail.com>
| return this.ajv.validate(schemaRef, data) | ||
| validate (schemaRef, data, errors) { | ||
| const valid = this.ajv.validate(schemaRef, data) | ||
| if (this.ajv.errors && Array.isArray(errors)) errors.push(...this.ajv.errors) |
There was a problem hiding this comment.
could this be faster?
| if (this.ajv.errors && Array.isArray(errors)) errors.push(...this.ajv.errors) | |
| Array.isArray(errors) && Arrry.prototype.push.apply(errors, this.ajv.errors) |
| t.fail('should throw') | ||
| } catch (err) { | ||
| t.equal(err.message, 'The value of \'#\' does not match schema definition.') | ||
| t.same(err.validationErrors, [ |
There was a problem hiding this comment.
This would be then:
| t.same(err.validationErrors, [ | |
| t.same(err.cause, [ |
| t.fail('should throw') | ||
| } catch (err) { | ||
| t.equal(err.message, 'The value of \'#/properties/data\' does not match schema definition.') | ||
| t.same(err.validationErrors, [ |
There was a problem hiding this comment.
dito
| t.same(err.validationErrors, [ | |
| t.same(err.cause, [ |
There was a problem hiding this comment.
i think the rationale here was to call it as fastify with ajv implementation:
Uzlopak
left a comment
Uzlopak
left a comment
There was a problem hiding this comment.
I hope my remarks make sense :)
Looking forward
5 participants
when using a schema that includes
anyOforoneOfand it fails to find a match the error is pretty worthless (in my case it is usually'The value of \'#\' does not match schema definition.')and it is not trivial to debug since stack trace points to anonymous functions composed at runtime.
this sets the validation errors on the error before throwing it. if the performance penalty is too big I suggest it should be behind a
verboseoption