NSwag: Regression from 13.6.2 to 13.7.0

C# is C#, and you can’t return two totally unrelated types from your methods

Hey @jeremyVignelles whilst I agree with your statement in general, I believe that’s not really what’s being asked here.

What I think we’re all looking for, myself included (and @mattwhitfield, correct me if I’m wrong) is for the NSwag client generator to understand null (for nullable types of course) when the response HTTP status code is 204.

As @mattwhitfield described in his scenario, his API either returns 202 with an entity, or 204 without an entity.

Right now, the API client throws an exception on 204 rather than returning null to the caller. This should be possible. If not the default option, it should at least be configurable.

Thanks @mattwhitfield for your detailed answer. I agree that NSwag should not have to make any decision for those kind of cases, but C# is C#, and you can’t return two totally unrelated types from your methods.

The default behavior there will be that NSwag will pick one result as the return type, but the other one will throw, which is not consistent with the fact that this is also a success.

That said, for these kind of use cases, I’d advise that you use the WrapDTO option which should be able to do just that, though I didn’t test it myself.

https://github.com/RicoSuter/NSwag/issues/3038#issuecomment-688517075 - here it says

Do you have any good reason to return both a 200 and a 204 ? Yes : Please tell us why?

We have an event processing system. When a client sends an event, it may be routed, in which case we send back a 202 and you get a routing code, or it may be 204 because it was not routed, and there is no routing code to send. Both are ‘OK’ outcomes. We ended up having to change our code to always return 202 and return a model, which IMHO is less RESTful.

I think if someone has bothered to explicitly document that their API returns a 202 and a 204, then NSwag should just pick that up without passing judgement or requiring them to reimplement their API.

@r-rc-christopher I think that does indeed work around the runtime error calling the service. However, the generated API client response type will be Task<Payload> even if client is generated with nullable reference types enabled (so it looks like it’s not null). This leads to confusion on callers as they wouldn’t expect null.

Btw. instead of removing HttpNoContentOutputFormatter, it can also be configured to not do this status code changing (nut sure what else it does, tbh):

services.AddMvc(options => 
{
    var noContentFormatter = options.OutputFormatters.OfType<HttpNoContentOutputFormatter>().FirstOrDefault();
    if (noContentFormatter != null)
    {
        noContentFormatter.TreatNullValueAsNoContent = false;
    }
});

Hi, I’ve been thinking about it, but I haven’t been able to “merge” the return type due to the complexity of the code base (don’t know what I’m breaking and where). I’ve been thinking about this, about #1259 and about oneOf/anyOf scenarii ( #1903 #1721 #2991). I am creating a library to support that kind of type union, that you could find here : https://github.com/jeremyVignelles/ooak

The goal would be to merge all success status code in either one type, one nullable type (200|201 for example) or using a TypeUnion (would require a dependency) if the scenario is more complex. At the time, I have no idea what the path would be to introduce such change, so if you want to have a look…

It’s also possible to work around this by stopping ASP.NET core from auto-generating 204 responses for null results by removing the HttpNoContentOutputFormatter as in the snippet below.

services.AddMvcCore(options =>
{
    options.OutputFormatters.RemoveType<HttpNoContentOutputFormatter>();
})

Another problem is

throw new ApiException("Response was null which was not expected.", status_, objectResponse_.Text, headers_, null);

Model says it’s a NRT, but attributes may condradict with it but I didn’t find any information about how do I specify that response may be null in some cases but can’t in anothers. I’ve only found an option to set the default, but nothing about overriding it on per-handle basis. For example, I have this handler that returns MyData? and another which returns MyData. I’d like to express it via attributes so NSwag would generate an appropriate spec but apparently I couldn’t find any way to do so.

Any workaround/insights for this? cc @RicoSuter @jeremyVignelles

Somehow the problem didn’t get fixed with 13.7.0.

Here is an endpoint:

#nullable enable
class MyData {}

/// <response code="204" nullable="true"></response>
[HttpGet("Foo/MyData")]
[ProducesResponseType(typeof(MyData), StatusCodes.Status200OK)]
[ProducesResponseType(StatusCodes.Status204NoContent)]
[ProducesResponseType(typeof(Dictionary<string, string[]>), StatusCodes.Status400BadRequest)]
[ProducesResponseType(typeof(MessageModel), StatusCodes.Status401Unauthorized)]
[ProducesResponseType(typeof(MessageModel), StatusCodes.Status404NotFound)]
public async Task<MyData?> MyEndpoint(Guid orderId) =>
	null;

Here is schema produced:

"Foo/MyData": {
  "get": {
	"tags": [	],
	"summary": "",
	"operationId": "Foo_MyEndpoint",
	"parameters": [],
	"responses": {
	  "200": {
		"description": "",
		"content": {
		  "application/json": {
			"schema": {
			  "$ref": "#/components/schemas/MyData"
			}
		  }
		}
	  },
          "204": {
            "description": "",
            "content": {
              "application/json": {
                "schema": {
                  "nullable": true
                }
              }
            }
          },
	  "400": {
		"description": "",
		"content": {
		  "application/json": {
			"schema": {
			  "type": "object",
			  "additionalProperties": {
				"type": "array",
				"nullable": true,
				"items": {
				  "type": "string",
				  "nullable": true
				}
			  }
			}
		  }
		}
	  },
	  "401": {
		"description": "",
		"content": {
		  "application/json": {
			"schema": {
			  "$ref": "#/components/schemas/MessageModel"
			}
		  }
		}
	  },
	  "404": {
		"description": "",
		"content": {
		  "application/json": {
			"schema": {
			  "$ref": "#/components/schemas/MessageModel"
			}
		  }
		}
	  }
	}
  }
},

And this is code generated:

public async System.Threading.Tasks.Task<ApiResponse<MyData>> Foos_GetMyDataAsync(System.Threading.CancellationToken cancellationToken)
{
	var urlBuilder_ = new System.Text.StringBuilder();
	urlBuilder_.Append("Foo/MyData");

	var client_ = _httpClient;
	var disposeClient_ = false;
	try
	{
		using (var request_ = new System.Net.Http.HttpRequestMessage())
		{
			request_.Method = new System.Net.Http.HttpMethod("GET");
			request_.Headers.Accept.Add(System.Net.Http.Headers.MediaTypeWithQualityHeaderValue.Parse("application/json"));

			PrepareRequest(client_, request_, urlBuilder_);
			
			var url_ = urlBuilder_.ToString();
			request_.RequestUri = new System.Uri(url_, System.UriKind.RelativeOrAbsolute);

			PrepareRequest(client_, request_, url_);

			var response_ = await client_.SendAsync(request_, System.Net.Http.HttpCompletionOption.ResponseHeadersRead, cancellationToken).ConfigureAwait(false);
			var disposeResponse_ = true;
			try
			{
				var headers_ = System.Linq.Enumerable.ToDictionary(response_.Headers, h_ => h_.Key, h_ => h_.Value);
				if (response_.Content != null && response_.Content.Headers != null)
				{
					foreach (var item_ in response_.Content.Headers)
						headers_[item_.Key] = item_.Value;
				}

				ProcessResponse(client_, response_);

				var status_ = (int)response_.StatusCode;
				if (status_ == 200)
				{
					var objectResponse_ = await ReadObjectResponseAsync<MyData>(response_, headers_, cancellationToken).ConfigureAwait(false);
					if (objectResponse_.Object == null)
					{
						throw new ApiException("Response was null which was not expected.", status_, objectResponse_.Text, headers_, null);
					}
					return new ApiResponse<MyData>(status_, headers_, objectResponse_.Object);
				}
				else
				if (status_ == 204)
				{
					string responseText_ = ( response_.Content == null ) ? string.Empty : await response_.Content.ReadAsStringAsync().ConfigureAwait(false);
					throw new ApiException("A server side error occurred.", status_, responseText_, headers_, null);
				}
				else
				if (status_ == 400)
				{
					var objectResponse_ = await ReadObjectResponseAsync<System.Collections.Generic.IDictionary<string, System.Collections.Generic.ICollection<string>>>(response_, headers_, cancellationToken).ConfigureAwait(false);
					if (objectResponse_.Object == null)
					{
						throw new ApiException("Response was null which was not expected.", status_, objectResponse_.Text, headers_, null);
					}
					throw new ApiException<System.Collections.Generic.IDictionary<string, System.Collections.Generic.ICollection<string>>>("A server side error occurred.", status_, objectResponse_.Text, headers_, objectResponse_.Object, null);
				}
				else
				if (status_ == 401)
				{
					var objectResponse_ = await ReadObjectResponseAsync<MessageModel>(response_, headers_, cancellationToken).ConfigureAwait(false);
					if (objectResponse_.Object == null)
					{
						throw new ApiException("Response was null which was not expected.", status_, objectResponse_.Text, headers_, null);
					}
					throw new ApiException<MessageModel>("A server side error occurred.", status_, objectResponse_.Text, headers_, objectResponse_.Object, null);
				}
				else
				if (status_ == 404)
				{
					var objectResponse_ = await ReadObjectResponseAsync<MessageModel>(response_, headers_, cancellationToken).ConfigureAwait(false);
					if (objectResponse_.Object == null)
					{
						throw new ApiException("Response was null which was not expected.", status_, objectResponse_.Text, headers_, null);
					}
					throw new ApiException<MessageModel>("A server side error occurred.", status_, objectResponse_.Text, headers_, objectResponse_.Object, null);
				}
				else
				{
					var responseData_ = response_.Content == null ? null : await response_.Content.ReadAsStringAsync().ConfigureAwait(false); 
					throw new ApiException("The HTTP status code of the response was not expected (" + status_ + ").", status_, responseData_, headers_, null);
				}
			}
			finally
			{
				if (disposeResponse_)
					response_.Dispose();
			}
		}
	}
	finally
	{
		if (disposeClient_)
			client_.Dispose();
	}
}

Which clearly states that it throws for 204 when it shouldn’t.

Versions used:

NSwag.AspNetCore Version=“13.10.7” - to generate openapi.json NSwag.MSBuild Version=“13.10.7” - to generate client using openapi.json

This needs to be tested if asp allows that - otherwise we can build a custom processor which does that (second point in the list).

Related to #1259 in that both should be fixed at the same time IMO.