Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Property names are missing in error text #222

Closed
kutyepov opened this issue May 10, 2021 · 3 comments · Fixed by #223
Closed

Property names are missing in error text #222

kutyepov opened this issue May 10, 2021 · 3 comments · Fixed by #223

Comments

@kutyepov
Copy link
Contributor

kutyepov commented May 10, 2021

From version v2.4.1 onwards, property names are missing in the response body. This is due to a breaking change in ajv@8:

property name is removed from "propertyName" keyword error message (it is still available in error.params.propertyName).
more details here

This issue was introduced in #190 where ajv@7 was upgraded to avj@8. Specific example of missing property name can be seen in updated test in that PR.


I am happy to raise a PR to fix this issue, but need some guidance on what would be the best approach to tackle it.
So far, I found an approach that works.
If the following is added to setValidatorCompiler in index.js:

  instance.setSchemaErrorFormatter(function (errors, dataVar) {
    return new Error(ajv.errorsText(errors))
  })

then the error text will include property name.
However, I am not sure if this is the right place to do so. Potentially, this might need to be addressed in fastify itself since it has its own ajv instance and therefore it might have the same issue. Fastify@3 is using ajv@6, so their schemas work fine.

@seriousme
Copy link
Owner

Hi,

thanks for asking!

I think AJV will have a reason for excluding the property name in version 8.
(esp. since its mentioned explictly ;-))
My suspicion is because of potential information leakage.
The example Fastify provides for setSchemaErrorFormatter also aims at providing generic error texts.

Fastify will be migrating to AJV@8 around end Q2, 2021 (to be released as Fastify 4) and I am hoping to let go of the custom AJV instance in the next major release (hence my PR on ajv-formats ;-) ) while also supporting Fastify 4.

So even if we add it now, but it is probably gone in the next major release.

Kind regards,
Hans

@seriousme
Copy link
Owner

Closed a bit too soon, sorry :-(
Published as 2.5.1 on NPM

Thanks,
Hans

@kutyepov
Copy link
Contributor Author

Yea, hopefully this will get fixed with fastify v4. Happy to undo this change when v4 is released.

Thank you for merging and releasing this change so quickly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants