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

Enable option to model cloudError for responses even if not present in swaggers #224

Closed
vladbarosan opened this issue Mar 6, 2018 · 10 comments
Assignees

Comments

@vladbarosan
Copy link

No description provided.

@vladbarosan
Copy link
Author

Even though the responses for swagger specs do not define it, a lot of operations return cloud error. To minimize noise in the beginning, we can enable an option to model this even if it is not in the swagger.

@vladbarosan vladbarosan self-assigned this Mar 6, 2018
@amarzavery
Copy link
Contributor

While we resolve swaggers in the spec resolver, we can add a step to add "default" response code with the schema for CloudError if the default response is not present.

Second step would be to add logic in validateResponse to validate any response code that is not defined in the responses to validate against default

@amarzavery
Copy link
Contributor

This needs to happen for both example and live validation

@amarzavery
Copy link
Contributor

We can decide the logic for validating against default. That is any response code above 399 should be validated against default or should we validate any response code that we do not find a definition for to validate against default. I think we should do the former as the latter can be classified as a separate error ( response code not defined in swagger, since swagger should match what is happening on the wire).

@vladbarosan
Copy link
Author

@amarzavery I was thinking for > 399 since they should at least return an error code if they return a cloud error.

@amarzavery
Copy link
Contributor

yes. We are saying the same thing. In other words greater than or equal to 400.

@vladbarosan
Copy link
Author

@amarzavery yes I was agreeing :)

@vladbarosan
Copy link
Author

@amarzavery only thing is if we want to make that validation depending on the received status code we will need to make the change in Sway. Not sure if we want to do that there. I would rather do it in oav and maybe from here to try to do a check.

and regarding doing it for examples, i would leave it off there, if we can catch when they author the examples that they don't define a default section lets make them do it.

@amarzavery
Copy link
Contributor

and regarding doing it for examples, i would leave it off there, if we can catch when they author the examples that they don't define a default section lets make them do it.

A little confused with what you are saying. If I am thinking correctly then the PR that you just sent will also do model validation against default schema for responses not specified in x-ms-examples, correct ?

The only thing is we should then modify the logic to not throw an error when they provide an example for a response status code in x-ms-examples and that response status code is not defined in the swagger spec.

We should simply rely on sway to validate this against the default. Which will obviously fail, since the schema for say 201 will not match that of default. It would be the right error IMO, which will make them think to correctly define the response codes in swagger. In this way the logic for live and model validation would remain the same.

@vladbarosan
Copy link
Author

vladbarosan commented Mar 15, 2018

A little confused with what you are saying. If I am thinking correctly then the PR that you just sent will also do model validation against default schema for responses not specified in x-ms-examples, correct ?

So the implicit modelling is optional and off by default ( it will be needed to be passed as as paramter to be enabled ( which is not happening for anything here ) from the api validation service we will pass the option to enable that. So for anything else nothing changes.

The only thing is we should then modify the logic to not throw an error when they provide an example for a response status code in x-ms-examples and that response status code is not defined in the swagger spec.

Not sure what you mean by that. RIght now if this happens for examples ( i.e. a status code is used that is not defined in the swagger ) an error is thrown ?

We should simply rely on sway to validate this against the default. Which will obviously fail, since the schema for say 201 will not match that of default. It would be the right error IMO, which will make them think to correctly define the response codes in swagger. In this way the logic for live and model validation would remain the same.

If for the previous question the answer is that no error is thrown, than this is what is going to happen, Sway will return an error saying INVALID_STATUS_CODE.

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

No branches or pull requests

2 participants