Specified non-successful status codes should throw HttpOperationException<> with specific schema about autorest

[stevenkuhn]

@matt-gibbs Thanks for the quick reply, Matt. I added those error responses to my Swagger file because I want the users of my API to know specifically what a specific error looks like (an unhandled exception error will be different than a validation error). I read that section of the specification as well and found it vague concerning errors. Quoting from the swagger specification:

However, it is expected from the documentation to cover a successful operation response and any known errors.

I personally assumed that this meant there would only be one successful response and that all other defined responses would be errors. But I think treating any defined 2xx responses as success and 4xx and 5xx as errors would be the best option. There could be an argument whether 1xx and 3xx should be treated as successes or errors.

The default can be used a default response object for all HTTP codes that are not covered individually by the specification.

The Responses Object MUST contain at least one response code, and it SHOULD be the response for a successful operation call.

I think throwing for default is great and definitely something worth preserving. What I would like to suggest is the following:

1. Keep the default response the way it is (in my case throwing HttpOperationException<ErrorResource>).
2. If a defined Swagger response is 2xx, then return the referenced schema (in my case, UserResource).
3. If there are multiple 2xx responses defined with _different_ schemas, then return object.
4. If a defined Swagger response is 4xx or 5xx then throw HttpOperationException<> with the referenced schema (in my case HttpOperationException<ErrorResource> for 400 and HttpOperationException<ValidationResource> for 422). You already generate separate code paths for different "successful" status codes, so something similar could be done for different "error" status codes. A common base type for errors or adding vendor extensions should not be necessary.
5. If a defined Swagger response is 4xx or 5xx and a schema _is not_ provided, then throw HttpOperationException (non-generic).

On another note, awesome work on this this project! I've been able to automatically generate and build an API client project using the OWIN TestServer (to generate the swagger file) and AutoRest.exe (to generate the client code). I then reference that client project in my integration tests to test against my API locally. It's been working great! My suggestions above would help me with testing the appropriate error responses.

[stevenkuhn]

I also would like to point out that because those generated operations aren't virtual, I can't simply create another class that derives from the generated class and overrides those methods.

I've also tried to use PostSharp to intercept the method call to add the logic I want, but since HttpOperationResponse<> does not derive from a non-generic HttpOperationResponse class, I'm finding it very difficult to simply pull out the HttpOperationResponse<> from the Task<HttpOperationResponse<>> return value so I can get at the HttpResponseMessage value inside it.

[matt-gibbs]

@chartek, Thank you Steven. We're trying to model the behavior described in the Swagger spec.
https://github.com/swagger-api/swagger-spec/blob/master/versions/2.0.md#responses-object
The defined operation responses are "successful" with the default "implying" an error. So the return type for the known codes has just the common base type of object. I agree this is not ideal. I'm going to leave this issue open for us to discuss some options. I think the distinction of throwing for default is good and something we'd like to preserve. Perhaps treating 2xx as success and 4xx as error would be an improvement and allow the common base type to be segmented better. The 201 would have a UserResource return type and the 400 and 422 and default could throw a common base type. Still not ideal, but might be a reasonable improvement without resorting to adding vendor extensions to the Swagger.

[stankovski]

@chartek We may consider this in the future. For now this is by design.

[nour-s]

Hello guys, sorry to comment on old issue, but I couldn't find any reference for this to a recent issue.
Has this been resolved?