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

[.NET]: How do I react to unexpected status codes? Or, how do I read the response itself? #3744

Closed
KennethHoff opened this issue Nov 18, 2023 · 20 comments
Assignees
Milestone

Comments

@KennethHoff
Copy link

KennethHoff commented Nov 18, 2023

I've been trying out kiota for the past few hours and while the initial setup was really nice (easily the best one I've seen so far), I've looked through every sort of documentation I could find - the actual docs, GitHub issues and discussions (for both this repo and the various dotnet-specific ones) as well as looking through the raw code - even resorting to ChatGPT. All of which resulted in, for all intents and purposes, a waste of time. I could not understand how to do something as simple as reading the HttpResponseMessage. There's absolute no way this is not possible, but I couldn't find it, so here I am.

It's abstracted to the nth degree yet something as simple as seeing what's happening is seemingly impossible (Again, probably isn't.. just hyperbole)

The closest I've seen is #2963, but that's just accessing some minor part of the request, and not the entirety (Side note: If you're accessing the HttpResponseMessage anyways, why didn't you just add the whole thing at once back when #2963 was solved?)

I can only assume there's something I'm missing here, because there's no way nobody has ever thought about something as simple as this before, but considering I've spent literal hours trying to find some documentation, at least this issue will serve as some (hopefully) easy-to-find documentation on this 👍

Additional information

Upon calling the endpoint and getting an unexpected error, I get the following from kiota, but I cannot for the life of me figure out what it means:
The server returned an unexpected status code and no error factory is registered for this code: 500

What is an error factory and how do I register it?

This sounds like more-or-less exactly what I want; A way to respond to unexpected status codes.

Why do I want it?

After saying all that, I guess I should probably tell you why I want it...

I'm creating a BFF (Backend-for-Frontend) solution where I'm effectively just passing values from a third-party through the .NET app and giving the results to the front end, and in doing so I need to give some reason for why something failed - other than simply a status code. That value naturally comes from the response message in some way (As of now, I haven't looked into it too much, but maybe it's located in the body or something.. Doesn't really matter to this issue I don't believe).

Proposed solution

Assuming this is already possible, improve the documentation to illustrate how to read the response. I am assuming "registering an error factory" is a part of it, but after hours of searching for it or other solutions, it's coming up blank.

Alternative Designs

Assuming somehow that doing this is not possible, my first instinct tells me that this is the most logical return type for the generated client methods:

var result = await generatedClient.SomeEndpoint.PostAsync(body, null, ct);

if (!result.Response.IsSuccessStatusCode)
{
	// Handle bad times
}
// Handle good times

Failing that, at least being able to do this would be nice, but then it would be fail-state-only which feels weird.

try 
{
	await generatedClient.SomeEndpoint.PostAsync(body, null, ct);
}
catch (ApiException apiException)
{
	if (!apiException.Response.IsSuccessStatusCode)
	{
		// Handle bad times
	}
	// Handle good times (?? that somehow threw an exception? :shrug: )
}
// Handle good times, albeit without a ResponseMessage to work with.
@KennethHoff KennethHoff changed the title [.NET]: How do I read the response itself? [.NET]: How do I react to unexpected status codes? Or, how do I read the response itself? Nov 18, 2023
@gpltaylor
Copy link

OK after some diggin, technically it's not the fault of this Package. The root cause comes from Microsoft.Kiota.Http.HttpClientLibrary.

This throws away the content/body of the response if it can't be mapped to a errorMapping IParsable.

I am still digging to see if I can create a custom IParsable and attach it to the status code..

At the moment, the only way I can see the Response is to remove the Nuget package for Microsoft.Kiota.Http.HttpClientLibrary and use the code. Then put a breakpoint on jetbrains://rd/navigate/reference?project=KiotaExploded&path=kiota-http-dotnet/src/HttpClientRequestAdapter.cs:ThrowIfFailedResponse

NOTE: This is odd behaviour IMO. In some cases APIs return valuable information about validation, 500 errors stack traces and other goodies during development. This lib assumes a perfect payload and a perfect API.. I've never see such a thing :)

@gpltaylor
Copy link

OK, IMO this can't be done with the current codebase...

In order to handle unique (real life) error responses we need to be able to override the "errorMapping" used within the requestBuilder. Changing this code is a bad idea, as this is generated.

I have been able to do a PoC by overriding the Error Mapper for 400 (very bad code, it's just a PoC).

        public async Task<Type_3?> PostAsync(Type_2 body, Action<FullQuoteRequestBuilderPostRequestConfiguration>? requestConfiguration = default, CancellationToken cancellationToken = default) {
#nullable restore
#else
        public async Task<Type_3> PostAsync(Type_2 body, Action<FullQuoteRequestBuilderPostRequestConfiguration> requestConfiguration = default, CancellationToken cancellationToken = default) {
#endif
            _ = body ?? throw new ArgumentNullException(nameof(body));
            var requestInfo = ToPostRequestInformation(body, requestConfiguration);
            var errorMapping = new Dictionary<string, ParsableFactory<IParsable>> {
                {"400", ErrorResponse.CreateFromCusomDiscriminatorValue},
                {"401", ErrorResponse.CreateFromDiscriminatorValue},
                {"404", ErrorResponse.CreateFromDiscriminatorValue},
                {"405", ErrorResponse.CreateFromDiscriminatorValue},
                {"406", ErrorResponse.CreateFromDiscriminatorValue},
                {"415", ErrorResponse.CreateFromDiscriminatorValue},
                {"500", ErrorResponse.CreateFromDiscriminatorValue},
            };
            return await RequestAdapter.SendAsync<Type_3>(requestInfo, Type_3.CreateFromDiscriminatorValue, errorMapping, cancellationToken).ConfigureAwait(false);
        }

NOTE:

 {"400", ErrorResponse.CreateFromCusomDiscriminatorValue},

Then I created my own Error Response and handler based on the unique response from this API I am integrating with.

        /// <summary>
        /// Custom error response
        /// </summary>
        /// <param name="parseNode">The parse node to use to read the discriminator value and create the object</param>
        public static ErrorResponse CreateFromCustomDiscriminatorValue(IParseNode parseNode) {
            _ = parseNode ?? throw new ArgumentNullException(nameof(parseNode));
            var err = new ErrorResponse()
            {
                // messageez??? What the hell
                AdditionalData = new Dictionary<string, object>(new KeyValuePair<string, object>[] {
                    new KeyValuePair<string, object>("code", parseNode.GetChildNode("code").GetStringValue()),
                    new KeyValuePair<string, object>("message", parseNode.GetChildNode("messageez").GetStringValue())
                })
            };

            return err;
        }

What I propose is that we allow custom ErrorMappings to override/extend the defaults.
Something like

var result = await client.FullQuote.PostAsync(body, x => {
            x.Headers.Add("Client-Id", "my-key"); 
            x.Headers.Add("Client-Secret", "Bob"); 
            .AddErrorMapping("400", CreateFromCustomDiscriminatorValue);
        });

NOTE: During my development the API is changing all the time (Multiple times per day). They are not a fixed "real boy" APIs but a system under high development. Therefore I am re-generating the Client all day long. This means I loose this code and have to reset. The Classes are not partial and I can't get extension methods to work...

Does anyone have any ideas.. am I missing something obvious?

@KennethHoff
Copy link
Author

KennethHoff commented Nov 19, 2023

Edit: See my next comment for newer findings.


I figured something out. IResponseHandler is able to intercept the response and change the output of the message. For methods that don't actually return anything - like SendNoContentAsync - the only real thing you can do is side-effects (including throwing exceptions) however.

Therefore, doing something like this allows you to "intercept" the content message and replace the ApiException message:

await generatedClient.SomeEndpoint.PostAsync(body, configuration =>
{
	configuration.Options.Add(new ResponseHandlerOption
	{
		ResponseHandler = new NoContentResponseHandler(),
	});
}, ct);

where NoContentResponseHandler is:

class NoContentResponseHandler : IResponseHandler
{
	public async Task<TModelType?> HandleResponseAsync<TNativeResponseType, TModelType>(TNativeResponseType response, Dictionary<string, ParsableFactory<IParsable>>? errorMappings)
	{
		if (response is not HttpResponseMessage httpResponseMessage)
		{
			throw new InvalidOperationException("The response is not an HttpResponseMessage");
		}

		if (!httpResponseMessage.IsSuccessStatusCode)
		{
			return await HandleErrorResponseAsync<TModelType>(httpResponseMessage, errorMappings);
		}

		return default;
	}

	private static async Task<TModelType> HandleErrorResponseAsync<TModelType>(HttpResponseMessage httpResponseMessage, Dictionary<string,ParsableFactory<IParsable>>? errorMappings)
	{
		var content = await httpResponseMessage.Content.ReadAsStringAsync();

		throw new ApiException(content);
	}
}

That said, I have not tested this in any real capacity just yet, so I don't know how for example this interacts with the built-in ErrorMappings; Whether I have to manually execute them, or if they're executed before the IResponseHandler kicks in (... or what the errorMappings are in the first place...). The built-in implementation does not seem to do anything other than assigning some properties which are then accessed in NativeResponseWrapper which itself is seemingly not used 🤷. I couldn't find anything about them anywhere in the docs, so.. ye.. This documentation is in dire need of improvement (Which, considering it's a Microsoft product is far from a surprise).

In context of this, I have a few questions for the kiota team:

  1. Is it possible to apply these options globally on the Client itself, or would I have to manually add them everytime?
  2. Is all of this such a hassle because my OpenApi Schema is bad? Given a perfect schema (whatever that means), would I get a perfect client? (again, whatever that means). What can I tell my API author in order to improve this experience?
  3. Is this how you are supposed to handle errors? Because "side-effect oriented design" (which I just coined) is a very poor DX.
  4. What is the ErrorMappings dictionary? How do I use it? Is it supposed to be used? It was added in adds error handling support #1100 but it didn't describe what it does - at least not in a way I can understand.

@KennethHoff
Copy link
Author

KennethHoff commented Nov 19, 2023

I just thought I might've overlooked something so I went out of bed to check on how it worked and there's a fundamental flaw with the "solution" I found; An IResponseHandler takes over the entire response flow.

Yes, it gives you access to partially affect the error path... but you lose the happy path along the way.

The following is the response flow of the SendAsync method on HttpClientRequestAdapter(The other model-returning methods are, for all intents and purposes, identical to this). As you can see, specifying a responseHandler more-or-less bypasses the entire reason you used Kiota in the first place.

image

It skips that entire response handling flow and goes straight to your IResponseHandler implementation. I guess that is theoretically fine for a SendNoContentAsync - which is what I was testing with - but unless you recreate the entire happy path along the way - which obviously makes no sense - it's still fundamentally flawed.

The following is the SendNoContentAsync on HttpClientRequestAdapter. As you can see, specifying a responseHandler doesn't really affect the happy path - by virtue of it not really having one - but it still technically bypasses it in its entirety. The fact it didn't really have a happy path to begin with is probably why I didn't think about it before just now.. And I was so happy with finally figuring something out 😭

image

This goes all the way back to the errorMappings. That seems to have the most potential here, but without any way of actually adding some of those - without manually editing the generated code - we're not any closer than I was at this time yesterday.

I guess we could technically create a custom implementation of HttpClientRequestAdapter that "intercepts" all the calls and adds in our custom errorMappings, but that cannot possibly be what Microsoft intended when they designed this project.

@gpltaylor
Copy link

gpltaylor commented Nov 20, 2023

I agree. I think adding the ability to add a ErrorMappings is a logically way forward. I was hoping an admin would have jumped on this thread to help give direction.

I will see if I can put a PR together and see if we can figure out how to add a custom ErrorMapping/IParsable but I don't have a lot of time.

This also only allows us to see the response.. In a perfect world I want to be able to store the Request and Response.
I would normally do this by raising events within this the code, but there is no Mediator or Event pattern in this code so I don't think that PR would be approved. Maybe allowing Deligates to be passed in, and then over written.

So it looks like there are two issues here

  1. Validation of the response fails if the structure of the schema response does not match that of the ErrorResponse.
  2. If validation fails we do not get the body of the response
  3. It's not possible to "see" the request of response content

Story
As a user working for a large bank, when I perform an API call, the request AND response should be recorded within Elia.
** Elia: An auditing platform

Question: Is this possible with the current codebase?
Answer: I don't think so

@KennethHoff
Copy link
Author

I think I'll give up on Kiota for now, and maybe return if/when we ever get a response on this issue. In the best case scenario I can simply revert back to the old state, but I'm guessing this issue won't be fixed in the coming days, and I can't spend any more time on this than I already have. Unfortunate, as the API interface Kiota generates (the fluent one) is a lot better than the mess that NSwag generates.

@gpltaylor
Copy link

OK, for clarity, on reviewing the Swagger definitions for the Return type; the response didn't match the expected format :(
This is my bad. The integration documents I am working of is wrong (I told you my world was rough)

I fixed the integrations Swagger document and removed some random required fields that the API does not return.
Once I did this the Error response was returned 🙄 🐑 ish.

However, I still can't find any way to record the raw Request and Response without pushing all the traffic via a Proxy.

I await help from admins and people in the know...

@baywet
Copy link
Member

baywet commented Nov 20, 2023

Hi everyone 👋,
Thanks for your interest in kiota, for reaching out and for the great feedback on this thread.
I'll try to reply to every point without forgetting anything.

Documentation

We know we have a painful gap in our public documentation today, this is mostly due to a staffing issue for technical writers we've been trying to resolve for months on end now. In the meantime we're trying to collect all those gaps, so don't hesitate to create issues that are focused on one point at a time in the dedicated repository

General design

The general design of kiota clients is the application developers should not have to "deal with HTTP" in most cases anymore. This assumes the API description has enough information.
The errors classes responses (4XX, 5XX) will be thrown as exceptions, exceptions generated from the API description.

Errors and factories

The exception thrown from the client in case of an error class response depend on what's described by the OpenAPI description. The logic follows this order, (let's assume we received a 503 from the service):

  1. if a 503 response is documented, use the generated type for that.
  2. else if a 5XX response is documented, use the generated type for that.
  3. else throw the generic API exception you've seen earlier.
  4. for cases 1 & 2, if the payload cannot be parsed into the generated type, throw the generic API exception as well.

In any case, all generated exceptions derive from API exception, and that has fields for the return status code as well as the responses headers. We didn't include the response body mostly out of a performance concern.

If you want to include more errors/factories, simply edit the description to document additional response codes/classes.

Cross cutting concerns at the HTTP level

One thing this thread hasn't mentioned yet, is the middleware stack. For concerns like "save any request/response to a third party service for audit purposes", you can implement a middleware handler, and add it to the stack, see the retry handler as an example and let me know if you have further questions on this aspect.

Tracing

The clients implement Open Telemetry tracing, so if you want more details over what's going on, you can turn on the diagnostics sources for Microsoft.Kiota.Http.HttpClientLibrary, Microsoft.Kiota.Abstractions and Microsoft.Kiota.Authentication.Azure (if you're using this authentication provider).

Accessing the native request/response ad-hoc

Again, the design philosophy is that you don't need to access the native http requests/responses, and improve the OpenAPI description instead as it'll benefit not only the client you're generating, but also the other clients/tools in your toolchain.
Should you still need to do so, there are multiple ways.

IReponseHandler

You've already mentioned this one, you can implement that interface and pass that to a request through the response handler option. When you use that option, kiota will expect you to handle everything from the response to throwing an exception/returning the deserialized value in the implementation. This is designed this way to avoid reading the response multiple times and creating performance bottlenecks.

var myHandlerOption = new ResponseHandlerOption {
   Handler = myHandlerInstance,
};
var result = await client.Foo.Bar.GetAsync(x -> x.Options.Add(myHandlerOption));

The NativeResponseWrapper is a remnant of an earlier design, it only exists in dotnet, and we should consider marking it obsolete IMHO.

HeadersInspectionHandlerOption

this option allows you to observe the request and response headers only. Here is how you use it. We didn't copy the request/response body to avoid performance issues here as well.

var observeOptions = new HeadersInspectionHandlerOption {
   InspectRequestHeaders = true,
   InspectResponseHeaders = true,
};
var result = await client.Foo.Bar.GetAsync(x -> x.Options.Add(observeOptions));
// read the headers and do something

Response turnaround

For information, most of (Microsoft part of) the team is based either in Kenya, East or West coasts. Expect most replies to thread to happen during work days on those time zones.

Hopefully I addressed all questions/concerns, don't hesitate to follow up here.

@baywet baywet self-assigned this Nov 20, 2023
@baywet baywet added this to the Backlog milestone Nov 20, 2023
@KennethHoff
Copy link
Author

KennethHoff commented Nov 20, 2023

Thanks for the in-depth reply.

Am I right in understanding then that the only thing I really should have to tweak in order to get what I want is the OpenApi schema itself? In retrospect that actually makes a lot of sense, but it didn't really occur to me that it was something I - as a consumer of the API - was "allowed to do".

Based on that, is this line of thinking correct?

A story of API consumption

I get an OpenAPI schema from some company - let's call the company Company A, and the schema Schema A. It has the following possible returns: 204 and 400.

I generate my client (Client A) using the CLI, and hook it up using my language of choice (Which for me would be C#), and realize that it can also return a 401. I update my copy of Schema A (Now referred to as Schema B) to add this information (And call Company A and tell them to fix Schema A). I then regenerate the client using Schema B (now Client B).

Later on, I get a 500 w/ a response body from one of the endpoints. I do the same again; Update Schema B (Now Schema C) with information regarding 500 with the fact that it can contain a response body. I regenerate the client (Client C) and call Company A again.

Rinse and repeat until the client reflects the schema, and the schema reflects the server. No manual peeking into the HttpResponseMessage or custom re-implementations of HttpClientRequestAdapter or anything C# at all, really.


Is this the DX story of Kiota? Because if it is, then that sounds very reasonable... Off to OpenApi-schema-learning-school I go!

@KennethHoff
Copy link
Author

After some rudimentary testing, simply fixing the OpenAPI schema seems to do the trick. It also feels a lot less hacky than anything else I can think of, which is perfect.

Sometimes you don't really think about the obvious solutions and instead try find a way around the problem instead.

Looks like Kiota is back on the menu, boys!

@gpltaylor
Copy link

Am I right in understanding then that the only thing I really should have to tweak in order to get what I want is the OpenApi schema itself? In retrospect that actually makes a lot of sense, but it didn't really occur to me that it was something I - as a consumer of the API - was "allowed to do".

yes I had that head-desk moment

@baywet thanx for the great response.
Pointing me in the right direction for the middleware is great. I could see the comments in the code but couldn't find out how to implement middleware. This is defo needed within the core Docs.

I am going to try an add a Middleware that records the request and response. I think I should be able to pull this off.
Also a Kiota Middleware Repository may be a cools idea...

Also to finish, I realised from the Docs that the whole Microsoft.Kiota.Http.HttpClientLibrary can be replaced.. So that also solves my problem; but I like Middleware idea better.

Thanx all!! My team really love this product BTW

@baywet
Copy link
Member

baywet commented Nov 20, 2023

yes, this is the thinking behind kiota. Instead of having to write convoluted code to handle tangent scenarios (after all, the goal is to have the call succeed) in every and each application that depends on the API, let's fix the API description so it benefits everyone, at every stage. And if you think about it, it's a fault of the API owners to have implemented another response code, but not documented it in their descriptions.

Now, we acknowledge the loop to get the description fixed can be shorter or longer depending on the scenario:

  • When you own the API you're generating a client for: minutes.
  • When your organization owns the API you're generating a client for: hours.
  • When you rely on another organization: variable, could be days, could be months.

We don't have a clear solution for the 3rd case, hopefully API producers care about their APIs and the associated description because it is somehow tied to revenue. But a workaround, after reaching out and not getting satisfactory/timely resolution, could be to copy the description locally, make a couple of edits, commit it to your repo (so you can repeat the generation) and using that instead of the vendor provided description.

Yes, please create the issue for the missing docs topics, you wouldn't know how much it helps to have a customer complaining constructively sometimes :)

@gpltaylor
Copy link

Just for reference and not directly related to Kiota core package but this answers the question about getting access to the Request.

NOTE: As above, if the Swagger response document is correct you will get the response from Kiota - Still think the Pure response should be added to the default error handler

Implementing Middleware

Implement middleware to access the pure request.

Middleware class

Create your middleware class and add your business logic

public class SaveRequestHandler : DelegatingHandler
{
    protected override async Task<HttpResponseMessage> SendAsync(
        HttpRequestMessage request, CancellationToken cancellationToken)
    {
        string jsonContent = await request.Content.ReadAsStringAsync();
        Console.WriteLine($"Request: {jsonContent}");

        return await base.SendAsync(request, cancellationToken);
    }
}

Register Middleware

Create a Middleware delegate array however, make sure to use the existing Middleware already implemented within Kiota.HttpClient. This includes Retry, redirect handling and more.

var delegates = KiotaClientFactory.CreateDefaultHandlers();
delegates.Add(new SaveRequestHandler());

Next we create a Delegate chain so middleware is registered in the right order

var handler =
    KiotaClientFactory.ChainHandlersCollectionAndGetFirstLink(
        KiotaClientFactory.GetDefaultHttpMessageHandler(),
        delegates.ToArray());

Finally create the HttpClient and then the Adapter

var httpClient = new HttpClient(handler!);
var adapter = new HttpClientRequestAdapter(authProvider, httpClient:httpClient);
var client = new PostsClient(adapter);

I will spend some time cleaning this up and adding to the Docs - then PR

Thanx

@gpltaylor
Copy link

@KennethHoff you happy to mark this ticket as done?

@gpltaylor
Copy link

As noted, I've added some docs to the Learn website so we can help others who had the same issues.

currently here:
https://learn.microsoft.com/en-gb/openapi/kiota/middleware

@KennethHoff
Copy link
Author

@gpltaylor

Doesn't seem like it's added to the navigation yet? So you can't really get to it yet.

@gpltaylor
Copy link

I noticed that this morning. It’s was working last night.
I think a merge conflict has removed it from the index file.
I will add a new PR to add it back

@baywet
Copy link
Member

baywet commented Nov 22, 2023

Hey everyone,
Thanks for bringing this up.
We forgot to update one of the files, the page was effectively in the breadcrumb, but not in the table of contents.
Just put together this to address the issue.
MicrosoftDocs/openapi-docs#41

@lvde0
Copy link

lvde0 commented Apr 5, 2024

Hey, sorry for reviving this thread, but I am currently struggling to "customize" the generic ApiException. My OpenAPI spec returns simple strings for error status codes:

"400": {
  "description": "Bad Request",
  "content": {
    "text/plain": {
      "schema": {
        "type": "string"
      }
    },
    "application/json": {
      "schema": {
        "type": "string"
      }
    },
    "text/json": {
      "schema": {
        "type": "string"
      }
    }
  }
},

Ideally I could embed these into the Message of the ApiException. It doesn't seem like Kiota is generating any error mappings or anything. How would I accomplish this?

@LukeButters
Copy link

Here is an example of throwing an error with the content of the response when an error code is received:

public class ThrowErrorWithContentWhenNonSuccessExitCodeIsRecievedRequestHandler : DelegatingHandler
    {
        protected override async Task<HttpResponseMessage> SendAsync(
            HttpRequestMessage request, CancellationToken cancellationToken)
        {
            var response = await base.SendAsync(request, cancellationToken);
            if (response.IsSuccessStatusCode)
            {
                return response;
            }

            var reader = new StreamReader(await response.Content.ReadAsStreamAsync(cancellationToken));
            var text = await reader.ReadToEndAsync(cancellationToken);
            throw new Exception($"While making request {request.Method} {request.RequestUri}, a non success http status code: {response.StatusCode.ToString()} was recieved. The content was: {text}");
        }
    }

Ideally it would be possible to only read the entire content as a string when we don't know what to do with the error code, however kiota doesn't support that :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

5 participants