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

Required properties error path does not contain the required property name #91

Closed
asaf opened this issue Jan 14, 2021 · 6 comments · Fixed by #102
Closed

Required properties error path does not contain the required property name #91

asaf opened this issue Jan 14, 2021 · 6 comments · Fixed by #102
Assignees

Comments

@asaf
Copy link

asaf commented Jan 14, 2021

Hey,

Having a JSON schema with required properties generates an error such:

PropertyPath: "/"
Message: 'X value is required'

I understand that the error is actually on the object level but losing the context of the X field makes it very difficult to display the error under the 'X' field.

Does it make sense to keep the property key that is required in the error somehow?

Thanks,
Asaf

@aakhan1
Copy link
Contributor

aakhan1 commented Jan 16, 2021

Hi @asaf. I reported a similar issue on this #81. I tend to agree. The property name that is the source of the exception should be made available. I created my own version for the time being until this gets addressed.

@carolynvs
Copy link
Contributor

carolynvs commented Jan 20, 2021

I am confused by what the desired behavior is as well. On v0.1.1, the ProperyPath was set to the child that violated the schema. After upgrading to v0.2.0, the path is now set to the parent.

I'm not sure if this change was intended or if a fix (I only had to move a single line of code) would be welcome?

@peterhellberg
Copy link

I just noticed this issue as well (and that there hasn’t been a tagged release in quite some time).

It would be good if the behavior was documented if working as intended.

@Arqu
Copy link
Contributor

Arqu commented Mar 25, 2021

I'll aim to review the spec and look into it by the end of the week if time permits to move this along.

@Arqu Arqu self-assigned this Mar 25, 2021
@asaf
Copy link
Author

asaf commented Apr 18, 2021

@Arqu any news on this? thanks!

@Arqu
Copy link
Contributor

Arqu commented Apr 18, 2021

I've looked into it and apparently neither is really up to spec anymore. As the output format is not really correct per spec.
Taking the more useful route, I'd say the better way would be to show the property path and not just the parent path until this is patched.

For anyone interested, the KeyError structure needs to be updated (and surrounding logic) to contain (at a minimum) keywordLocation, instanceLocation & error which would contain both options and satisfy the basic output format per spec. This is more looking ahead to the 2020 spec as for the 2019-09 it's not clearly defined and only mandates the "errors" field.

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