Current Behavior

At the moment, when an HTTP request receives an error response, the response is parsed using a ParsableFactory<IParsable> that's passed in as a dictionary of HTTP Status Code -> Factory.

At the moment, when the error response is successfully parsed, it's used to set an activity tag for telemetry, then checked whether it's an exception or not.

Desired Behavior

It would be useful for the parsed response to be bubbled up to the caller in the thrown ApiException so that this can be accessed in the calling code.

API Changes

This would depend on an API change in Microsoft.Kiota.Abstractions in ApiException to accommodate the error response value.

public class ApiException : Exception
{
    public ApiException() : base()
    public ApiException(string message) : base(message)
    public ApiException(string message, Exception innerException) : base(message, innerException)

    public int ResponseStatusCode { get; set; }
    public IDictionary<string, IEnumerable<string>> ResponseHeaders { get; set; }
+   public object? ResponseObject { get; set; }
}

I generated a client with Kiota, based on an OpenAPI spec, which does not specify error codes, hence errorMapping is empty in Kiota-generated client. The ApiException that Kiota throws contains just the status code and response headers. Is there any way to read the actual response body (as string) without modifying the Kiota-generated client code?

When the HttpClientRequestAdapter is resolved from a service provider using either transient or scoped lifetime and passing a HttpClient then it leaks memory. The leak is caused by the creation of an ActivitySource every time the HttpClientRequestAdapter is constructed but it's never disposed unless the HttpClientRequestAdapter created the HttpClient.

This problem was introduced in commit e44fbfb#diff-d9ae4f1159bbe7195bce06dbc06baeb882e3f3d4fcf280c3d31523bf8c0dada7

In a very simple test constructing 1 million HttpClientRequestAdapter objects which can easily happen in the use-case I have results in the following memory usage pattern. Once the test completes we have a large number of objects in gen2 of the GC.

    public static void DotPeekActivitySourceLeak()
    {
        HttpClient httpClient = new HttpClient();
        for (var i = 0; i < 1000000; i++)
        {
            using var adapter = new HttpClientRequestAdapter(new AnonymousAuthenticationProvider(), httpClient: httpClient);
        }
    }

Updating the HttpClientRequestAdapter constructor to register the ActivitySource in the same way as the HttpClient middleware implementations, since it's using the same ActivitySource name eliminates this leak.

activitySource = ActivitySourceRegistry.DefaultInstance.GetOrCreateActivitySource(obsOptions.TracerInstrumentationName);

Hey, I use Microsoft.Graph package for the dotnet/Scaffolding project. For the new .NET 8 release of the scaffolding package, the dependency constraint this project has on System.Text.Json being [6.0, 8.0) is causing issues since it causes the default .NET 8 project's dependency on System.Text.Json to downgrade causing an error (dependency downgrade). Unless you have a workaround you can suggest.

This issue is related to this ADR microsoft/kiota#3941

We need to think about building an AuthHandler middleware that can use our Kiota Auth Providers so that we can have native HttpClient experiences. Otherwise, we can't satisfy this ADR microsoft/kiota#3200

This issue occurs for Blazor WASM applications consuming an OpenAPI spec that does not supply a base URL. Blazor WASM does not support the System.Net.Http.HttpClientHandler class, so those apps must supply an HttpClient to the HttpClientRequestAdapter to work.

The issue is that the BaseAddress property on the supplied HttpClient object does not infer that as the HttpClientRequestAdapter's BaseUrl, and it must be set separately for requests to be sent appropriately.

How to reproduce:

1. Create a Blazor WASM application with .NET 7 using the empty template (For the sake of example, named "KiotaTestApp.FrontEnd")

2. Generate a client from an API spec that does not supply a Base URL. E.g. (kiota generate -d http://<url>/swagger/v1/swagger.json -n KiotaTestApp.FrontEnd.Client -o ./Client -l CSharp -c ApiClient

   Note: The dotnet-aspnet-codegenerator tool can be used to scaffold an API with an OpenAPI spec that reproduces the above scenario.

3. Note the following warning that appears in the console output:

   warn: Kiota.Builder.KiotaBuilder[0]
         OpenAPI warning: #/ - A servers entry (v3) or host + basePath + schemes properties (v2) was not present in the OpenAPI description. The root URL will need to be set manually with the request adapter.
   warn: Kiota.Builder.KiotaBuilder[0]
         No server url found in the OpenAPI document. The base url will need to be set when using the client.
   

4. Add the Kiota API client service setup in a Blazor WASM application's Program.cs file like so:

   // ...
   var authProvider = new AnonymousAuthenticationProvider();
   var client = new HttpClient { BaseAddress = new Uri("http://<url>") };
   var requestAdapter = new HttpClientRequestAdapter(authProvider, httpClient: client );
   builder.Services.AddScoped<ApiClient>(_ => new ApiClient(requestAdapter));
   // ...

5. In the Index.razor (or any other page) inject the client and create a method that sends an HTTP request with it

6. Launch the application, and view that requests are not sent to the HttpClient's BaseAddress

Workaround

Explicitly setting the BaseUrl after adding an HttpClient applies the parameter value, allowing the client to send requests.

// ...
var authProvider = new AnonymousAuthenticationProvider();
var client = new HttpClient { BaseAddress = new Uri("http://<url>") };
var requestAdapter = new HttpClientRequestAdapter(authProvider, httpClient: client );
requestAdapter.BaseUrl = "http://<url>";
builder.Services.AddScoped<ApiClient>(_ => new ApiClient(requestAdapter));
// ...

This method name confused me. How do folks feel like about renaming it to ThrowIfFailedResponse?

Streams returned from HttpClientRequestAdapter methods like SendPrimitiveAsync<Stream> cannot be used. The methods return disposed streams. I believe the culprit is the new DrainAsync method.

The default telemetry configuration results in creating and an ActivitySource and Activity for each request, and then disposing those objects after a response is returned. ActivitySource.Dispose calls SynchronizedList.Remove (https://github.com/dotnet/runtime/blob/main/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/ActivitySource.cs#L299) as shown in the Azure Profiler output where the lock is present (https://github.com/dotnet/runtime/blob/main/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/ActivitySource.cs#L402).

As we are doing this in each of the client middleware irrespective of the whether the telemetry is being listened to (e.g.

A temporary workaround is to add middleware to remove the default ObservabilityOptions created from the request. But the action below should probably be configurable and "opt-in"

cc @baywet

related to microsoft/kiota#4025
add a 3rd case in the error handling of the request adapter for XXX (must be the 3rd in the order, first sepcific code, then 4XX or 5XX depending on the first number, then XXX)

Maybe I am missing something but this is painful....

A key scenario is that someone wants to use Kiota "out of the box" but with one more MessageHandler. e.g. CompressionHandler because it isn't there by default yet.

To customize the message handling pipeline it is necessary to pass in a "custom" HttpClient. e.g.

var requestAdapter = new HttpClientRequestAdapter(new AnonymousAuthenticationProvider(), httpClient:client);

You can create your own HttpClient by doing

var client = KiotaClientFactory.Create();

This is great if you want to set custom headers that are sent with every request. Having said that, if you set an Accept header it will get overridden by RequestBuilders. That's not ideal.
However, if you want to add a message handler, you need to use the overload that accepts a messagehandler pipeline. I'm not sure why we dropped this terminology for "chain" as pipeline has been the term used since HttpClient was originally created.

You can create a customized set of handlers by calling the CreateDefaultHandlers method and adjusting it. e.g.

       var handlers = KiotaClientFactory.CreateDefaultHandlers();
        handlers.Insert(0, new CompressionHandler());

However, there is no obvious way to use that list. I tried this

var pipeline = KiotaClientFactory.ChainHandlersCollectionAndGetFirstLink(KiotaClientFactory.GetDefaultHttpMessageHandler(), handlers);

but unfortunately, we made ChainHandlersCollectionAndGetFirstLink accept "params". I had to Bing how to use a list to provide "params". So in the end, this works.

        var handlers = KiotaClientFactory.CreateDefaultHandlers();
        handlers.Insert(0, new CompressionHandler());
        var pipeline = KiotaClientFactory.ChainHandlersCollectionAndGetFirstLink(KiotaClientFactory.GetDefaultHttpMessageHandler(), handlers.ToArray());
        var client = KiotaClientFactory.Create(pipeline);
        var requestAdapter = new HttpClientRequestAdapter(new AnonymousAuthenticationProvider(), httpClient:client);

I would like us to consider adding an overload to Create() that accepts a list of handlers and optionally a non-default finalhandler. This would allow a developer to do the following.

        var handlers = KiotaClientFactory.CreateDefaultHandlers();
        handlers.Insert(0, new CompressionHandler());
        var client = KiotaClientFactory.Create(handlers);
        var requestAdapter = new HttpClientRequestAdapter(new AnonymousAuthenticationProvider(), httpClient:client);

I don't see a whole lot of value making our customers convert the list to a pipeline. We can do that as an implementation detail.

Related to microsoftgraph/msgraph-sdk-dotnet#1954

For calls to SendPrimitiveAsync<Stream> we should pass the HttpCompletionOption.ResponseHeadersRead to the HttpClient to avoid the loading of the entire stream buffer into memory as the requested stream may be unnecessarily large.
This could be done by passing the optional parameter once the return type is determined to be that of a Stream.
https://github.com/microsoft/kiota-http-dotnet/blob/main/src/HttpClientRequestAdapter.cs#L410

We can avoid this for other scenarios as typed return are not expected to run into this issue as the size is not expected to grow as much compared to an unknown Stream size.

originally reported in microsoft/kiota-http-python#179
We add those cases as unit tests and correct the implementation so it doesn't impact query parameters values

I originally posted this to the main https://github.com/microsoft/kiota repo, but I figured I should also ask here. Close whichever one you feel like doesn't make sense.

Original post

A requestInfo object may be created with just the raw url and thus have no template. The line above results in throwing and error if not provided

Add static anaylzers for Core and Abstraction libraries and cleanup the readmes to reflect this

This will mainly involves setting up Sonar cloud for the abstraction libs and fixing validation pipelines once the abstraction libs are moved to a separate repo.

AB#10960