microsoft / kiota-http-dotnet

Kiota http provider implementation for dotnet with HttpClient
https://aka.ms/kiota/docs
MIT License
34 stars 16 forks source link

Adding a handler causes System.InvalidOperationException: 'The inner handler has not been assigned.' to be thrown on request execution #267

Open IanKemp opened 1 week ago

IanKemp commented 1 week ago

The following code throws the aforementioned exception with Microsoft.Kiota.Http.HttpClientLibrary version 1.4.3 (latest as of this writing) when issuing a request against myClient. Am I doing something wrong, or is this a bug?

var kiotaClient = KiotaClientFactory.Create(
    new MyDelegatingHandler());

MyAuthenticationProvider authenticationProvider = new();
HttpClientRequestAdapter requestAdapter = new(
    authenticationProvider: authenticationProvider,
    httpClient: kiotaClient)
{
    BaseUrl = "https://example.com",
};

GeneratedKiotaClient myClient = new(requestAdapter);

Stack:

System.Net.Http.DelegatingHandler.SetOperationStarted()
System.Net.Http.DelegatingHandler.SendAsync(System.Net.Http.HttpRequestMessage, System.Threading.CancellationToken)
MyDelegatingHandler.SendAsync(System.Net.Http.HttpRequestMessage, System.Threading.CancellationToken)
Microsoft.Kiota.Http.HttpClientLibrary.Middleware.HeadersInspectionHandler.SendAsync(System.Net.Http.HttpRequestMessage, System.Threading.CancellationToken)
Microsoft.Kiota.Http.HttpClientLibrary.Middleware.RedirectHandler.SendAsync(System.Net.Http.HttpRequestMessage, System.Threading.CancellationToken)
Microsoft.Kiota.Http.HttpClientLibrary.Middleware.RetryHandler.SendAsync(System.Net.Http.HttpRequestMessage, System.Threading.CancellationToken)
Microsoft.Kiota.Http.HttpClientLibrary.Middleware.UriReplacementHandler<TUriReplacementHandlerOption>.SendAsync(System.Net.Http.HttpRequestMessage, System.Threading.CancellationToken)
System.Net.Http.HttpClient.SendAsync.__Core|83_0(System.Net.Http.HttpRequestMessage, System.Net.Http.HttpCompletionOption, System.Threading.CancellationTokenSource, bool, System.Threading.CancellationTokenSource, System.Threading.CancellationToken)
Microsoft.Kiota.Http.HttpClientLibrary.HttpClientRequestAdapter.GetHttpResponseMessage(Microsoft.Kiota.Abstractions.RequestInformation, System.Threading.CancellationToken, System.Diagnostics.Activity, string, bool)
Microsoft.Kiota.Http.HttpClientLibrary.HttpClientRequestAdapter.SendAsync<ModelType>(Microsoft.Kiota.Abstractions.RequestInformation, Microsoft.Kiota.Abstractions.Serialization.ParsableFactory<ModelType>, System.Collections.Generic.Dictionary<string, Microsoft.Kiota.Abstractions.Serialization.ParsableFactory<Microsoft.Kiota.Abstractions.Serialization.IParsable>>, System.Threading.CancellationToken)

I do not receive the same exception if I follow the example directly.

IanKemp commented 1 week ago

What works is using the other overload of Create:

var handlers = KiotaClientFactory.CreateDefaultHandlers();
handlers.Add(new MyDelegatingHandler());

var kiotaClient = KiotaClientFactory.Create(handlers);

// all other code same as first comment
MyAuthenticationProvider ...

The issue is that Create(HttpMessageHandler?, IRequestOption[]?) is IMO a bit of a footgun: it doesn't automagically add GetDefaultHttpMessageHandler() as the InnerHandler of the passed-in handler. Yes, the method does explicitly say that the parameter is the final handler, but I made the assumption (which I don't think is unwarranted) that if I wanted to do nothing more than add a single handler into the chain, I should probably be calling the method that takes a single handler. Perhaps a CreateWith(params HttpMessageHandler[] handlers) method that inserts the provided handlers between the default ones and the final GetDefaultHttpMessageHandler() one?

I also think that the linked example should be updated to the code given in this comment, as it's both more concise and IMO self-explanatory.

svrooij commented 1 week ago

This seems related to #268 ?

And since you're trying to do authentication, why not make a factory that creates the right IAuthenticationProvider. The authentication provider interface has one method to implement which is: Task AuthenticateRequestAsync(...), has access to all the request info and can attach additional headers.

microsoft-github-policy-service[bot] commented 4 days ago

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment.