Nswag: Bug in Error serialization openapi2csclient

Created on 17 Jun 2019  路  19Comments  路  Source: RicoSuter/NSwag

my yaml:

    Error:
      type: object
      properties:
        code:
          $ref: '#/components/schemas/ErrorCode'
        description:
          type: string
    Errors:
      type: object
      properties:
        errors:
          type: array
          items:
            $ref: '#/components/schemas/Error'
    ErrorCode:
      description: Enum of errors
      type: string
      enum:
        - NOTFOUND
        - GENERAL_EXCEPTION

service I am calling returns this:

{
    "errors": [
        {
            "code": "GENERAL_EXCEPTION",
            "description": "03396370-ddc4-41c4-ae03-ed07249a9d1b - Not supported yet."
        }
    ]
}

I cannot get serialized object Errors in proper format with current error handling. This is current generated c# code

     if (status_ == "500") 
                        {
                            var objectResponse_ = await ReadJsonObjectResponseAsync<Errors>(response_, headers_).ConfigureAwait(false);
                            throw new ApiException<Errors>("Error", (int)response_.StatusCode, objectResponse_.Text, headers_, objectResponse_.Object, null);
                        }

objectResponse_ contains Errors object which is what I get from service I am calling but Text property is empty and the Errors object is not seriliazed correctly

this used to work in my previously generated code:

                       if (status_ == "500") 
                        {
                            var responseData_ = response_.Content == null ? null : await response_.Content.ReadAsStringAsync().ConfigureAwait(false); 
                            var result_ = default(Errors); 
                            try
                            {
                                result_ = Newtonsoft.Json.JsonConvert.DeserializeObject<Errors>(responseData_, _settings.Value);
                            } 
                            catch (System.Exception exception_) 
                            {
                                throw new SwaggerException("Could not deserialize the response body.", (int)response_.StatusCode, responseData_, headers_, exception_);
                            }
                            throw new SwaggerException<Errors>("Errors", (int)response_.StatusCode, responseData_, headers_, result_, null);
                        }

Most helpful comment

@rars Oh, I see there's a new NSwag template Client.Class.Constructor that you can override (so you don't need any other *.liquid templates) to set the property - that's probably the best approach.

All 19 comments

objectResponse_ contains Errors object which is what I get from service I am calling ... the Errors object is not seriliazed correctly

So is Errors object correct or not?

/cc @Jehoel

objectResponse_ contains Errors object which is what I get from service I am calling ... the Errors object is not seriliazed correctly

So is Errors object correct or not?

there is no problem with yaml but Exception handling has changed (simplified) previously I did get Errors object in same format as above json example so it bubbled up through generated code in same format I was calling service

now I get
"Errors\n\nStatus: 500\nResponse: \n"
as output from generated code so I lost code and description info from the service I call

So you are only missing the plain response string?

So you are only missing the plain response string?

not really I would expect the Errors object or any yaml defined for error handling to be fully populated with all properties I expect from service I call. It can be anythign not just code or description/

this used to work in previous version of error handling. The very last code you can see in original post

What is in objectResponse_.Object? By default objectResponse_.Text is empty for better performance (this is expected).

What is in objectResponse_.Object? By default objectResponse_.Text is empty for better performance (this is expected).

it contains populated Errors object

So it is correctly deserialized?

Sorry I dont understand your problem - if you need the response text in the exception you need to set ReadResponseAsString to true...

@RicoSuter thanks you can close the issue setting ReadResponseAsString to true fixed my problem

@RicoSuter where should ReadResponseAsString be set to true? Is there an Nswag.json config setting to set this to true by default instead?

On the generated client class

@RicoSuter thanks. Do you have a nice example of how to set this with dependency injection though? I've been using something along the lines of:

void RegisterClients(IServiceCollection serviceCollection)
{
    services.AddHttpClient<IBooksClient, BooksClient>(
        httpClient => httpClient.BaseAddress = "https://books.com/api");`
}

using Microsoft.AspNetCore.Http.Extensions. How would you alter the above to set that property or is there an alternative better way of doing that?

@rars Please post your code where your application instantiates and uses an NSwag-generated client class object.

Microsoft.AspNetCore.Http.Extensions is only concerned with HttpClient - not NSwag-generated clients.

@Jehoel thanks. I've created the following small repo: https://github.com/rars/client-di-demo
Hopefully that shows what sort of setup I have. Where would you suggest setting the property in that context?

@rars

There are several options.

The simplest is to set ReadResponseAsString in a partial method - which requires adding partial class declarations in another file and keeping it in-sync with the NSwag-generated clients. The ProcessResponse partial method is the best place to set ReadResponseAsString as it's always called right before deserialization:

public partial class BooksClient
{
    partial void ProcessResponse(HttpClient client, HttpResponseMessage response)
    {
        this.ReadResponseAsString = false;
    }
}
public partial class VideosClient
{
    partial void ProcessResponse(HttpClient client, HttpResponseMessage response)
    {
        this.ReadResponseAsString = false;
    }
}
public partial class TeaClient
{
    partial void ProcessResponse(HttpClient client, HttpResponseMessage response)
    {
        this.ReadResponseAsString = false;
    }
}
// etc

Another approach, which uses MEDI, and is documented by Microsoft as the prescribed way for customizing registered typed-clients is to use ITypedHttpClientFactory.

Unfortunately because the ReadResponseAsString property isn't attached to any superclass or interface we can't use a common factory method for all of your typed clients - also you'll need to reimplement ITypedHttpClientFactory - which is a pain. And I won't go into details here.

I think the best approach here is to override the Liquid template that NSwag is using to generate a client class constructor that sets the this.ReadResponseAsString = false;

...or just subclass all of the typed clients and set the property there:

class BooksClient2 : BooksClient
{
    public BooksClient( HttpClient httpClient ) : base( httpClient )
    {
        this.ReadResponseAsString = false;
    }
}

void RegisterClients(IServiceCollection serviceCollection)
{
    services.AddHttpClient<IBooksClient, BooksClient2>(
        httpClient => httpClient.BaseAddress = "https://books.com/api");`
}

...which is about the same amount of effort as using partial classes.

@RicoSuter I think the option to use ReadResponseAsString should be a configurable option in NSwag client generation options - as well as adding an optional interface ITypedClient that exposes common members to external consumers, such as the ReadResponseAsString property.

@rars Oh, I see there's a new NSwag template Client.Class.Constructor that you can override (so you don't need any other *.liquid templates) to set the property - that's probably the best approach.

Thanks @Jehoel, that worked.

Was this page helpful?
0 / 5 - 0 ratings