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

oas3-schema rule output is hard to grok #210

Closed
TristanSpeakEasy opened this issue Dec 16, 2022 · 6 comments
Closed

oas3-schema rule output is hard to grok #210

TristanSpeakEasy opened this issue Dec 16, 2022 · 6 comments
Labels

Comments

@TristanSpeakEasy
Copy link
Contributor

The oas3-schema rule seems to do a lot of the high level validation of a document, but I find the output hard to digest and it doesn't help guide a user towards a solution.

For example:

(1:1)         | error    | OpenAPI specification is invalid: /properties/paths/$ref doesn't validate with '/definitions/Paths' | $                              
(1:1)         | error    | OpenAPI specification is invalid: /properties/tags/uniqueItems items at index 0 and 1 are equal     | $                              
(1:1)         | error    | OpenAPI specification is invalid: /properties/servers                                               | $

Here is some example output from a OpenAPI 3.0 document

  • The first error talks about /properties/paths/$ref which I am unsure what it relates to and then references /definitions/Paths which I believe is a Swagger reference instead of an OpenAPI 3 reference.
  • The second error seems to catch the duplicate tags I was asking for in this feature request: rule to detect duplicate tag definitions #206 but has no valid line number or path reference to the tags and the /properties/tags/uniqueItems notation is unclear to what it is referring to.
  • The third error is because the top level servers object is invalid but once again the notation is unclear and there are no valid line numbers or paths to find where the issue is.

Would be good to see the errors produce by this rule improved as it does seem to catch a lot of validation issues not present in other rules

@daveshanley
Copy link
Owner

I was waiting for this question to come up. The problem is the underlying data. It's deeply nested and kinda overly verbose, for example:

Screenshot_20221216_013632

So the question is: How do you want to boil this information down?

This recursive data goes down deep!

@TristanSpeakEasy
Copy link
Contributor Author

To be entirely honest this data doesn't look all that useful for really understanding what is wrong with an OpenAPI document.
I would want something like this to clearly state the problem, a potential solution and line number to find the issue and it looks like with the data shown it wouldn't be possible to give meaningful errors.

I imagine this is a 3rd party lib being used for JSONSchema validation? Are you using that on 3.0 and 3.1 documents equally? As it could give false negatives on a 3.0 document as they didn't fully conform to the JSON Schema specification

daveshanley added a commit that referenced this issue Dec 30, 2022
Schema errors are now flattened down to just the useful parts.
daveshanley added a commit that referenced this issue Dec 30, 2022
now schema violations are flattened, we can try and look them up from the JSONPath.
daveshanley added a commit that referenced this issue Dec 30, 2022
Schema errors are now flattened down to just the useful parts.
daveshanley added a commit that referenced this issue Dec 30, 2022
now schema violations are flattened, we can try and look them up from the JSONPath.
@daveshanley
Copy link
Owner

daveshanley commented Dec 30, 2022

In version v0.0.48, All of those deeply nested errors have been flattened down to the items that have no more causes (i.e. the actual nodes that throw the error and not the stack above it). The validation errors at the bottom of the stack actually do have useful data in there and a usable path that is extracted.

Try it out, and see how you feel about the validation messages now for schema failures.

@daveshanley
Copy link
Owner

@TristanSpeakEasy ping

@TristanSpeakEasy
Copy link
Contributor Author

Ah sorry I missed this will try it out tomorrow

@TristanSpeakEasy
Copy link
Contributor Author

Thanks @daveshanley this looks a lot better 👍 some of the errors returned don't make sense in some situations but I may open separate bugs for these

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

No branches or pull requests

2 participants