fix: display prop for additionalProperties error

[handrews]

This is trivial, but since it was not pre-approved I'll have no objection whatsoever if it's shot down

It's adding a single variable interpolation to an error template, so a proposal seemed overkill as I'd already solved it. But I can go through the steps if desired, and fully intend to for anything less trivial.

What issue does this pull request resolve?

With "additionalProperties": false, the existing errror message was always:

~/schemas$ ajv compile -m draft-04/hyper-schema -s foo
schema foo is invalid
error: schema is invalid: some.path should NOT have additional properties
~/schemas$ 

which can be hard to track down in very large objects.

What changes did you make?

This changes it to include the name of the property that triggered the error, which was already available in the error object:

~/schemas$ ajv compile -m draft-04/hyper-schema -s foo
schema foo is invalid
error: schema is invalid: some.path should NOT have additional property "thisIsWrong"
~/schemas$ 

Is there anything that requires more attention while reviewing?

I could not find any tests that verified error message wording. I looked by grepping the submodule for NOT since it appears in several messages including ones with interpolated variables.

(but the test suite and coverage are super-impressive)

[epoberezkin]

@handrews thank you.

I am sorry, I am going to say no...

That's been suggested several times. I don't like this idea for two reasons:

• it may be a breaking change for some users.
• more important: it would create a precedent where validated data is used in error messages, creating a vulnerability (e.g., when ajv is used to validate API data/parameters and error messages are logged).

As you say, it's already in params object so in the application you can modify messages in any way you need. ajv-errors package will allow to modify messages as well - templating is not there yet though.

[handrews]

@epoberezkin no problem! The vulnerability concern is an excellent point.

Would the application-level change work in ajv-cli? Perhaps under a flag if there is a security concern? Could perhaps be done as an integration with ajv-errors?

Otherwise I'll have to re-implement part of that (there is no way I'll be able to get schema authors to put up with a schema compile commit hook that doesn't say what actually caused the error). Which is not your problem but... ick.

I actually was originally trying to do this in ajv-cli originally but I couldn't figure out where to catch the error. My first experience with the node debugger was not enjoyable :-P JavaScript seems nearly unrecognizable compared to the last time I used it (when jquery was cutting edge...)

[epoberezkin]

Thank you.

ajv-cli already reports error objects, so you can see additional property in params. Is it not good enough?

The idea of ajv-errors is to define errors in the schema itself - not sure you're going to like it :)

[handrews]

What do you mean that I can see the property in params? My use case is non-programmers (in addition to programmers) such as product managers or tech writers editing schemas. A commit hook will keep non-compiling schemas from being checked in, but if the output from the actual command line doesn't pinpoint where the error occurs I'll have a revolt on my hands :-) Telling them to use the npm debugger won't fly, if that's what you mean.

I can, of course write my own tool for this, but that seems like a lot of work for better logging. Although perhaps by re-wrapping most of ajv-cli somehow...

[handrews]

Also because of this use case, it uses a modified-meta-schema with additionalProperties: false everywhere to catch typos and the like. Otherwise I avoid that usage like the plague.

[epoberezkin]

What I meant was that if you run

ajv -s test/schema.json -d test/invalid_data.json

(in ajv-cli folder) you will get this output:

test/invalid_data.json invalid
[ { keyword: 'required',
    dataPath: '[0].dimensions',
    schemaPath: '#/items/properties/dimensions/required',
    params: { missingProperty: 'height' },
    message: 'should have required property \'height\'' } ]

which looks friendly enough for people who can edit schemas - I didn't suggest debugging.

BUT, "compile" command outputs schema validation errors as strings only, it doesn't support --errors option that is supported for commands "validate" and "test".

There is ajv-validator/ajv-cli#10 that is exactly about it - to allow to output errors from schema validation in different format, that I think would solve your issue.

[handrews]

@epoberezkin ah, now I see. I should have been more clear: in this case I am only running "ajv compile", and hadn't gotten around to even trying data validation yet. However, I could obviously run "ajv validate" by passing the meta-schema in -s and the schema in -d. In which case that output is definitely sufficient! I had just assumed that they used the same output format, thanks for walking me through it :-)

Also will keep an eye on that ajv-cli issue. Fantastic ecosystem here!

[epoberezkin]

Because of some inconsistencies in uri format draft-4 meta-schemas use a bit different "uri" format when compiled (to allow uri-references). For draft-06 "validate" will work, but not for draft-04 schemas when they use "id" with uri-reference.

The issue has been recently raised json-schema-org/JSON-Schema-Test-Suite#177 - no idea why I decided to just just dance around it, probably because JSON-schema org was quite quiet then.

[handrews]

Hmm... yeah, we should sort that out. I like Julian's view that it's a meta-schema bug for "id" and should be treated as "uri-ref" in that case. But I'm willing to bet that the vast majority of people did not catch that distinction and just treating draft-04 "uri" as "uri-reference" would be the more practical approach. (now I need to go read that issue and see if I just contradicted myself)

In any event I'm not currently using "id" except at the root schema level, even when not using a custom meta-schema, and those are always absolute, so this will still work for my use case. Thanks for the heads up.