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

chore: Better JSON validation error messages #214

Merged
merged 2 commits into from
Nov 6, 2023

Conversation

aish-where-ya
Copy link
Contributor

@aish-where-ya aish-where-ya commented Nov 2, 2023

Feature or Problem

Better error messages for JSON validation.

Currently, when manifests are validated, the deserialization and validation is internally handled by serde and it prints out error messages that specify the exact location of the error in the file. Here are some of the errors thrown by serde:

Should be able to parse: spec.components[5]: missing field `contract` at line 97 column 7

Should be able to parse: spec.components[0]: invalid type: unit value, expected struct ActorProperties at line 10 column 7

Serde handles a majority of the cases in the Manifest schema but in the case of Traits, if there are any missing or incorrect properties that do not fall under type Linkedef or Spreadscaler, serde deserializes to Custom TraitProperty. Upon which the validation falls through to the OAM validation schema. Due to the limited features of the jsonschema crate, it pinpoints to the correct area with the problematic code but does not tell the user the exact problem with the schema.

This PR takes the best it can of the ValidationError provided by jsonschema and gives error messages as follows:

Validation Error : 
Should be able to parse object at: spec/components/ at index: 0 
Please check for missing or incorrect elements

Validation Error : 
Should be able to parse object at: spec/components/ at index: 0 
Should be able to parse object at: spec/components/ at index: 1 
Please check for missing or incorrect elements

Related Issues

#195

Release Information

next

Consumer Impact

NA

Testing

manually tested

Unit Test(s)

Modified test_manifest_validation

Acceptance or Integration

NA

Manual Verification

manually verified

brooksmtownsend
brooksmtownsend previously approved these changes Nov 3, 2023
Copy link
Member

@brooksmtownsend brooksmtownsend left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes the best of what we got! 😄 This should help just narrow down the source of the error

));
}
return Err(anyhow!("Validation Error : \n{}", error_message));
return Err(anyhow!(
"Validation Error : \n{}Please check for missing or incorrect elements",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: we can remove the space between Error :

Signed-off-by: aish-where-ya <[email protected]>
@brooksmtownsend brooksmtownsend merged commit 571adff into wasmCloud:main Nov 6, 2023
5 checks passed
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 this pull request may close these issues.

2 participants