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

Invalid handling of 201 response types #700

Closed
jgoelten opened this issue Mar 30, 2017 · 15 comments
Closed

Invalid handling of 201 response types #700

jgoelten opened this issue Mar 30, 2017 · 15 comments

Comments

@jgoelten
Copy link

hi @RSuter

I've already seen a few issues reported to this topic but I was not able to find a "best way to handle" it. In both c# and typescript generated code the 201 response type throws an exception instead of returning the result. Of course I can catch it and get the result extracted out of the exception but this is not a convenient approach.

        if (status === 200) {
            let result200: SampleEntity = null;
            let resultData200 = responseText === "" ? null : JSON.parse(responseText, this.jsonParseReviver);
            result200 = resultData200 ? SampleEntity.fromJS(resultData200) : new SampleEntity();
            return result200;
        } else if (status === 201) {
            let result201: SampleEntity = null;
            let resultData201 = responseText === "" ? null : JSON.parse(responseText, this.jsonParseReviver);
            result201 = resultData201 ? SampleEntity.fromJS(resultData201) : new SampleEntity();
            this.throwException("A server error occurred.", status, responseText, result201);
        } else if (status !== 200 && status !== 204) {
            this.throwException("An unexpected server error occurred.", status, responseText);
        }

On the controller of my asp.net core project I add an attribute on the post-method to tell swagger about the new response type. I am producing a 201 by calling the CreatedAtRoute(.....) method.

[ProducesResponseType(typeof(SampleEntity), (int)HttpStatusCode.Created)]

Do you have any suggestion how to handle it? I'd like to keep my 201 response type without the need of catching all exceptions for post requests. Is there a better way instead of manually modifying the generated service client after its creation?

Thanks for your help!

Jan

@RicoSuter
Copy link
Owner

The only way is to merge 2xx results with the same response type... Otherwise i dont see a solution because a method can only have one result type....

@RicoSuter
Copy link
Owner

... but this is not a simple task.

@RicoSuter
Copy link
Owner

... maybe it is if we just return instead of throw if response schema matches success schema and http code is 2xx...

@RicoSuter
Copy link
Owner

Duplicate: #541

@RicoSuter
Copy link
Owner

I think this is now properly supported (if success responses 2xx have the same type, then they are returned instead of only one and the others thrown). Please test this with the latest CI artifacts.

@jgoelten
Copy link
Author

I've re-generated all service clients with the latest ci-build. The generated code looks great, exactly what I expected it to be like. Thanks very much for your effort and fast deployment, awesome! Tomorrow I will test in on the dev staging area and provide you some feedback.

@jgoelten
Copy link
Author

@RSuter it is working as expected now. All responses with the same type are handled successfully. In my case the additional registered 201 response returns the resulting entity instead of throwing an exception. All workarounds where we had to get the result out of the swaggerexception are obsolet.

@RicoSuter
Copy link
Owner

Good to hear... Im still working on the blob handling and fixing some other issues then ill release v10... Hopefully next week.

Have a nice weekend.

@jgoelten
Copy link
Author

That sounds great! Looking forward for v10 👍 . The same to you, thanks!

@xaviergxf
Copy link

Hi @RSuter

Apparently swagger definition allows 200 to return message response type A and 201 to return response type B. How will this reconcile with c# client generation? Currently 201 throws an exception because there's no way a method can return 2 different object types. Do you see any improvement in the future?

Thanks

@RicoSuter
Copy link
Owner

Do you see any improvement in the future?

I dont really see a good solution to this - except change your api so that an operation has only one successful schema - everything other than that just confuses people and tooling :-)

@freedeveloperjagg
Copy link

Hi,
I have the following problem with the generation. The method is a POST, and return 201, that is normal for a POST, but the generation only accept as correct 200 and 204. I am missing something in the configuration?

@jeremyVignelles
Copy link
Collaborator

@freedeveloperjagg : Yes, you're probably missing the ProducesResponseType attribute above your controller method to declare the expected return type.

Also, please don't reply to closed issues, open your own issue, otherwise your question might be missed.

@kemmis
Copy link

kemmis commented Dec 31, 2020

FYI - you can apply default api conventions at the assembly level instead of adding ProducesResponseType on every controller method:

[assembly: ApiConventionType(typeof(DefaultApiConventions))]

https://docs.microsoft.com/en-us/aspnet/core/web-api/advanced/conventions?view=aspnetcore-5.0

@FaizulHussain
Copy link

I've applied the line "[assembly: ApiConventionType(typeof(DefaultApiConventions))]" on top of the namespace in the startup class but it didn't work. Even on the controller didn't work. The method version worked. I'm using asp.net core 5.
Any suggestions?

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

No branches or pull requests

7 participants