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

Handling different response content types for same status code #4309

Closed
HaydnDias opened this issue Mar 8, 2024 · 10 comments
Closed

Handling different response content types for same status code #4309

HaydnDias opened this issue Mar 8, 2024 · 10 comments
Labels
generator Issues or improvements relater to generation capabilities.
Milestone

Comments

@HaydnDias
Copy link

HaydnDias commented Mar 8, 2024

Hello! We have a situation where a call to a an endpoint can return a 403 of application/json or text/html, the json coming from the api, and the html coming from an api gateway/proxy, this could be the case for other HTTP status codes, however I can't seem to get Kiota to handle both of these, have posted an example json below.

Unfortunately what ends up happening is kiota throws a System.InvalidOperationException with the message Content type text/html does not have a factroy registered to be parsed, this is less than ideal because it doesn't even give us a base ApiException with the status code for us to fall back to.

Any advice on things I can do to handle this nicely would be much appreciated, aware I can likely handle it by using my own request adapters/handlers, but would like to keep things as simple as possible using the default implementation.

Command used to generate example client:
kiota generate -l CSharp -c ExampleClient -n ExampleApi -d example.yaml -o ./Generated --cc --co

openapi: 3.0.3
info:
  title: Example
  description: Example
  version: 1.0.0
servers:
  - url: https://example.com/api
paths:
  /helloworld:
    get:
      summary: Example
      description: Example
      operationId: example
      responses:
        '403':
          description: Forbidden
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/ApiError'
            text/html:
              schema:
                $ref: '#/components/schemas/HtmlError'
components:
  schemas:
    ApiError:
      required:
        - error_code
        - error_message
      type: object
      properties:
        error_code:
            type: string
        error_message:
            type: string
    HtmlError:
      type: string
      format: binary

Generated method:

public async Task<Stream> GetAsync(Action<RequestConfiguration<DefaultQueryParameters>> requestConfiguration = default, CancellationToken cancellationToken = default) {
	var requestInfo = ToGetRequestInformation(requestConfiguration);
	var errorMapping = new Dictionary<string, ParsableFactory<IParsable>> {
		{"403", ApiError.CreateFromDiscriminatorValue},
	};
	return await RequestAdapter.SendPrimitiveAsync<Stream>(requestInfo, errorMapping, cancellationToken).ConfigureAwait(false);
}
@baywet
Copy link
Member

baywet commented Mar 8, 2024

hi @HaydnDias,
Thanks for reaching out and for using kiota.
Can you pass the body of the "toGetRequestInformation" method please?
Kiota should be generating an accept request header with priorities, and the default settings should make json a higher priority than text/html.
Given that context, the API should reply with json according to HTTP standards.

@baywet baywet added question generator Issues or improvements relater to generation capabilities. labels Mar 8, 2024
@baywet baywet added this to the Backlog milestone Mar 8, 2024
@HaydnDias
Copy link
Author

Hey @baywet! Thanks for the speedy response, I've included the entire Builder class to save potential back and fourth.

// <auto-generated/>
using ExampleApi.Models;
using Microsoft.Kiota.Abstractions.Serialization;
using Microsoft.Kiota.Abstractions;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Threading.Tasks;
using System.Threading;
using System;
namespace ExampleApi.Helloworld {
    /// <summary>
    /// Builds and executes requests for operations under \helloworld
    /// </summary>
    public class HelloworldRequestBuilder : BaseRequestBuilder {
        /// <summary>
        /// Instantiates a new HelloworldRequestBuilder and sets the default values.
        /// </summary>
        /// <param name="pathParameters">Path parameters for the request</param>
        /// <param name="requestAdapter">The request adapter to use to execute the requests.</param>
        public HelloworldRequestBuilder(Dictionary<string, object> pathParameters, IRequestAdapter requestAdapter) : base(requestAdapter, "{+baseurl}/helloworld", pathParameters) {
        }
        /// <summary>
        /// Instantiates a new HelloworldRequestBuilder and sets the default values.
        /// </summary>
        /// <param name="rawUrl">The raw URL to use for the request builder.</param>
        /// <param name="requestAdapter">The request adapter to use to execute the requests.</param>
        public HelloworldRequestBuilder(string rawUrl, IRequestAdapter requestAdapter) : base(requestAdapter, "{+baseurl}/helloworld", rawUrl) {
        }
        /// <summary>
        /// Example
        /// </summary>
        /// <param name="cancellationToken">Cancellation token to use when cancelling requests</param>
        /// <param name="requestConfiguration">Configuration for the request such as headers, query parameters, and middleware options.</param>
#if NETSTANDARD2_1_OR_GREATER || NETCOREAPP3_1_OR_GREATER
#nullable enable
        public async Task<Stream?> GetAsync(Action<RequestConfiguration<DefaultQueryParameters>>? requestConfiguration = default, CancellationToken cancellationToken = default) {
#nullable restore
#else
        public async Task<Stream> GetAsync(Action<RequestConfiguration<DefaultQueryParameters>> requestConfiguration = default, CancellationToken cancellationToken = default) {
#endif
            var requestInfo = ToGetRequestInformation(requestConfiguration);
            var errorMapping = new Dictionary<string, ParsableFactory<IParsable>> {
                {"403", ApiError.CreateFromDiscriminatorValue},
            };
            return await RequestAdapter.SendPrimitiveAsync<Stream>(requestInfo, errorMapping, cancellationToken).ConfigureAwait(false);
        }
        /// <summary>
        /// Example
        /// </summary>
        /// <param name="requestConfiguration">Configuration for the request such as headers, query parameters, and middleware options.</param>
#if NETSTANDARD2_1_OR_GREATER || NETCOREAPP3_1_OR_GREATER
#nullable enable
        public RequestInformation ToGetRequestInformation(Action<RequestConfiguration<DefaultQueryParameters>>? requestConfiguration = default) {
#nullable restore
#else
        public RequestInformation ToGetRequestInformation(Action<RequestConfiguration<DefaultQueryParameters>> requestConfiguration = default) {
#endif
            var requestInfo = new RequestInformation(Method.GET, UrlTemplate, PathParameters);
            requestInfo.Configure(requestConfiguration);
            requestInfo.Headers.TryAdd("Accept", "application/json");
            return requestInfo;
        }
        /// <summary>
        /// Returns a request builder with the provided arbitrary URL. Using this method means any other path or query parameters are ignored.
        /// </summary>
        /// <param name="rawUrl">The raw URL to use for the request builder.</param>
        public HelloworldRequestBuilder WithUrl(string rawUrl) {
            return new HelloworldRequestBuilder(rawUrl, RequestAdapter);
        }
        /// <summary>
        /// Configuration for the request such as headers, query parameters, and middleware options.
        /// </summary>
        [Obsolete("This class is deprecated. Please use the generic RequestConfiguration class generated by the generator.")]
        public class HelloworldRequestBuilderGetRequestConfiguration : RequestConfiguration<DefaultQueryParameters> {
        }
    }
}

@baywet
Copy link
Member

baywet commented Mar 8, 2024

thanks for the additional information.
See requestInfo.Headers.TryAdd("Accept", "application/json"); the API should respond with json if it's capable of doing so, over HTML. Do you own the API and can you investigate why it's not following HTTP standards?

@HaydnDias
Copy link
Author

HaydnDias commented Mar 8, 2024

We don't own the api unfortunately, this is for a 3rd party, of which we have many that do a similar thing, the API does return JSON, but there are instances where the proxy/api gateway rejects the requests rather than the API, due to a variety of reasons, in our case we notice it most with firewall/ip filtering, but sometimes it's nginx/apache errors which do return html, this functionality is also supported by the OpenApi spec afaik? (feel free to correct me if not)
https://swagger.io/docs/specification/describing-responses/
https://swagger.io/specification/#response-object

@baywet
Copy link
Member

baywet commented Mar 8, 2024

It's perfectly valid from an OAS perspective. Kiota delineates between structured and non structured data types.
In the former case, it'll project models and handle deserialization, etc..
In the later case, it'll consider everything a stream, and let the caller decide what to do with it.
In case both are present, the non-structured types will be ignored.

I still believe the proxy shouldn't be doing what it's doing according to HTTP though.

You're kind of caught between mis aligned things are the moment. If you don't care about the HTML value, you can probably just implement IParseNode, you only need to return null in GetObjectValue. And register that for text/html as a workaround.

@HaydnDias
Copy link
Author

The HTML could be useful, but honestly the status code is the most important from our POV, do you have any examples you could point me towards for your suggestion? Alternatively may just have to use our own response handler which would be a shame.

Does Kiota support multiple different structured data types for the same response code but different content type headers?

@baywet
Copy link
Member

baywet commented Mar 8, 2024

because you'll be returning null, then the request adapter will throw an ApiException instead, which will contain the status code and response headers. If that's all you need, you should be good with that.

yes if the schema is the same across the different content types. No if the schema is different. We have an internal design doc to solve for that but didn't get any customer demand for it to date.

@HaydnDias
Copy link
Author

For the former point, do you have an example of how/where to implement IParseNode and register it for text/html for this example?

For the second point, glad to see something is in progress, I think this kind of thing will become more in demand as Kiota grows in usage, I think I would like to see more flexibility around exceptions/error mappings, and have seen a lot of discussions around this on github issues, we ended up having to manually fix/modify openapi specs to make them usable as there are sadly a lot of bad specs out there, and whilst it's a solution, being able to handle this in code would be nicer.

@HaydnDias
Copy link
Author

Hey, apologies for delay in getting back, took a look at this yesterday and got a solution I am happy with!

For anyone else who comes across this thread and would a bit more detail:

I created a new IParseNodeFactory and IParseNode based on the existing TextParseNodeFactory and TextParseNode.

Modified the ValidContentType in the new IParseNodeFactory to be text/html, and made it return the GetRootParseNode return the new IParseNode.

In the newly created IParseNode, rather than return null/default for the GetObjectValue<T> method, I actually checked the type being requested, and if it was our ApiError, I case the item to our error type, and then set the error message to the Text property on the IParseNode, thus giving us the html on the error property, and allowing us to re-use existing error handling.

public T GetObjectValue<T>(ParsableFactory<T> factory) where T : IParsable
{
    var item = factory(this);
    if (item is ApiError { ErrorMessage.Length: 0 } apiError)
        apiError.ErrorMessage = Text;
    return item;
}

To have this IParseNodeFactory added to the Client on generation, you pass the fully qualified class name into the generate command using https://learn.microsoft.com/en-gb/openapi/kiota/using#--deserializer---ds, however, if you already have a generated client with a kiota-lock.json file, you can add the fully qualified class name into this kiota-lock.json in the deserializers section.

Thanks to @baywet for pointing me in the right direction.

@github-project-automation github-project-automation bot moved this from Todo to Done in Kiota Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
generator Issues or improvements relater to generation capabilities.
Projects
Archived in project
Development

No branches or pull requests

2 participants