ajv-cli icon indicating copy to clipboard operation
ajv-cli copied to clipboard

Option for error reporting of invalid schemas

Open epoberezkin opened this issue 9 years ago • 10 comments
trafficstars

epoberezkin avatar Mar 22 '16 23:03 epoberezkin

@epoberezkin this is to track making the output of compile the same format as validate, correct?

It turns out I'm (more or less) going to need this, because errors in -r schemas are compiled rather than validated (obviously) and errors there end up preventing me from getting to the validation errors. Admittedly this is because I'm being lazy in my script and just passing all of our schemas as -r no matter which one is being validated, since pulling out the actual dependencies seemed annoying and the whole thing still runs fast enough. So I could fix my script to be intelligent, but if this isn't too hard to fix in ajv-cli I'd rather contribute a PR.

Any pointers on where to get started? When I was trying to debug this before I had trouble figuring out where I could intercept the error and access its details. No doubt due to my rusty JS skills and I'm fine with doing more debugging, but any advice would be much appreciated :-)

handrews avatar May 04 '17 17:05 handrews

@handrews that is correct.

Currently ajv-cli calls ajv.compile that internally calls ajv.validateSchema, so error reporting of invalid schemas happens in ajv rather than here: https://github.com/jessedc/ajv-cli/blob/master/commands/compile.js#L42

To report errors here (and control the output) you'll have to explicitly call ajv.validateSchema here and format errors using this: https://github.com/jessedc/ajv-cli/blob/master/commands/util.js#L50

The option has to be whitelisted here: https://github.com/jessedc/ajv-cli/blob/master/commands/compile.js#L10 in the same way as it is done here: https://github.com/jessedc/ajv-cli/blob/master/commands/validate.js#L10

You'll have to use ajv with option validateSchema: false and call validateSchema method manually in all cases here as well: https://github.com/jessedc/ajv-cli/blob/master/commands/ajv.js

It's quite a few changes for relatively trivial feature unfortunately...

Another approach would be to control how ajv itself logs schema validation errors (https://github.com/epoberezkin/ajv/blob/master/lib/ajv.js#L182) via some option (e.g. validateSchema can have additional values it accepts: 'log-json', 'log-js', 'log-line', or have another option e.g. schemaErrorsFormat) and simply pass --errors option from here.

epoberezkin avatar May 04 '17 20:05 epoberezkin

I probably prefer the first approach - error formatting should not be in Ajv...

epoberezkin avatar May 04 '17 20:05 epoberezkin

I probably prefer the first approach - error formatting should not be in Ajv...

Makes sense. Thanks so much for the detailed guide! That will cut down on time tremendously. Since I've got a (to me) compelling use case, I think it's probably worth the effort for me to work through it. But now I understand why it's been open for over a year :-)

handrews avatar May 04 '17 21:05 handrews

Actually the best and the easiest approach would be to add errors array to the error object here: https://github.com/epoberezkin/ajv/blob/master/lib/ajv.js#L183

And log errors based on the option here: https://github.com/jessedc/ajv-cli/blob/master/commands/ajv.js#L28

Then you won't have to change the flow everywhere...

In which case I suggest using a different option say --schema-errors to format these errors to keep it backwards compatible, as it would affect validate and test as well (which is fine).

epoberezkin avatar May 04 '17 21:05 epoberezkin

OK yeah that would work. Particularly if we can use -e as a short option since in the sort of situation I'm dealing with, while it will be scripted as a test, I'll be encouraging everyone to run it manually while developing schemas and they'll want to pass that every time. (or some other letter if e is already in use).

handrews avatar May 04 '17 21:05 handrews

-e can be a separate shortcut option that defines both --errors and --schema-errors at the same time. It would be weird if -e meant --schema-errors only. Let's have it as a separate PR.

epoberezkin avatar May 04 '17 21:05 epoberezkin

OK cool. So the plan is:

  • ajv PR to add the errors array (do you want me to file an ajv issue or just reference this one?)
  • ajv-cli PR to to add --schema-errors
  • ajv-cli PR to ~~change~~ add -e to enable both --errors and --schema-errors instead of only --errors

handrews avatar May 04 '17 21:05 handrews

to change -e

Rather to add it - there is no shortcut for --errors at the moment, so it's still backwards compatible.

epoberezkin avatar May 04 '17 22:05 epoberezkin

so it's still backwards compatible.

had kinda wondered about it. OK, good plan :-)

handrews avatar May 04 '17 22:05 handrews