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

Allow developers to inspect headers #2963

Closed
papegaaij opened this issue Jul 19, 2023 · 16 comments · Fixed by microsoft/kiota-http-python#255
Closed

Allow developers to inspect headers #2963

papegaaij opened this issue Jul 19, 2023 · 16 comments · Fixed by microsoft/kiota-http-python#255
Assignees
Labels
enhancement New feature or request generator Issues or improvements relater to generation capabilities.
Milestone

Comments

@papegaaij
Copy link
Contributor

I'm looking for a way to access the response headers for a response that also returns a value. A ResponseHandler does give access to the native response, but it also disables the default response handling. It seems none of the languages provide a way to access these headers via the abstractions layer. Maybe it would be possible to have a RequestOption that collects the response headers. The response headers could then be read from this option after the request.

The reason I need access to the response headers is because our API returns at most 100 items at a time. The Content-Range response header will show me which items were returned, so I can construct the Range header for the next request.

@baywet baywet self-assigned this Jul 20, 2023
@baywet baywet added this to the Backlog milestone Jul 20, 2023
@baywet
Copy link
Member

baywet commented Jul 20, 2023

Assuming you're referring to dotnet here (other languages work in a similar fashion) it can be achieved through a middleware handler that'd observe but not bypass the response pipeline.

@sebastienlevert was due to create an issue a while ago for a new layer called "Observer". Today we have an ability through a ResponseHandler option to bypass the response pipeline entirely. This is great but puts a lot of burden on the consumer if they just want to "look at the request/response". The idea of that spec was to provide the ability to observe (or maybe even mutate) the request and the response through an option that'd provide two callbacks.

Seb: I think we can repurpose that issue, just update the title/tags/milestone and add the spec below.

@papegaaij
Copy link
Contributor Author

.net, Go and Java all have middleware handlers, but they are not abstracted away from the actual implementation. For example, the Go version operates directly on the net/http package and the Java version uses OkHttp interceptors.

Another issue is that these handlers operate at a global level. They must be registered in the RequestAdapter. This makes it very hard to use them for single requests.

If the Kiota abstraction layer would provide a ResponseHeaderCollectorOption request option, I could do something like this (in Java):

ResponseHeaderCollectorOption responseHeaders = new ResponseHeaderCollectorOption();
var info = info().get(c -> c.options.add(responseHeaders)).get();
resonseHeaders.get("Content-Range");

@baywet baywet added enhancement New feature or request needs more information and removed question labels Jul 20, 2023
@baywet baywet assigned sebastienlevert and unassigned baywet Jul 20, 2023
@sebastienlevert
Copy link
Contributor

I tried to find the issue I thought I created. I'll be linking the POC here and repurpose this issue! I 100% agree with you @papegaaij, this should be easy! I'll comment the high-level design and we'll see for your comments!

@sebastienlevert
Copy link
Contributor

So here is the POC: https://github.com/sebastienlevert/kiota-demos/tree/main/github/csharp

This is based on 2 new classes ResponseInspectorHandler that plays a role in the middleware chain and the ResponseInspectorOption that allows to define which headers to inspect.

  • ResponseInspectorOption: Option object allowing to set if the headers need to be inspected HeadersToInspect and which headers should be inspected InspectHeaders. It also offers a Headers collection that represent the inspected (and now available) headers.
  • ResponseInspectorHandler: Receives the ResponseInspectorOption and based on its config (if the headers are to be inspected, the headers to inspect) would add the necessary headers in the ResponseInspectorOption.
// Set up the client and the request adapter and injects the response inspector handler
var authProvider = new AnonymousAuthenticationProvider();
var handlers = KiotaClientFactory.CreateDefaultHandlers();
handlers.Add(new ResponseInspectorHandler());

var firstHandler = KiotaClientFactory.ChainHandlersCollectionAndGetFirstLink(KiotaClientFactory.GetDefaultHttpMessageHandler(), handlers.ToArray());

if (firstHandler == null)
  throw new InvalidOperationException("No handlers were provided.");

var responseInspectorOption = new ResponseInspectorOption
{
  HeadersToInspect = new[] { "Link" }
}

var repos = await client.Orgs["microsoftgraph"].Repos.GetAsync(
    request =>
      {
        request.Options.Add(responseInspectorOptions);
      }
);

Console.WriteLine(responseInspectorOptions.Headers["Link"]);

@sebastienlevert sebastienlevert changed the title Accessing response headers Allow developers to inspect responses Jul 21, 2023
@sebastienlevert sebastienlevert modified the milestones: Backlog, Kiota v1.6 Jul 21, 2023
@sebastienlevert
Copy link
Contributor

Added it as part of Kiota 1.6 for now.

@papegaaij
Copy link
Contributor Author

@sebastienlevert Yes, that's very similar to what I proposed. IMHO the handler should be part of the default handlers and maybe even eliminated by moving the code to the http implementation. I think it should not be necessary to enumerate the headers you want to inspect. The option can simply collect all headers.

@sebastienlevert
Copy link
Contributor

It's about the cost of running this for all requests. That's why we want to keep it a separate option. Now the face that the middleware could be part of the chain by default is something that could be considered, especially if it does nothing until we pass an option. We'll discuss and conclude on our design but we'll take your feedback into consideration!

@papegaaij
Copy link
Contributor Author

I think the cost of copying the headers for every request is actually negligible compared to the total request handling, but there's no point in doing it either. The option not only gives the developer a way to explicitly ask for the response headers, but the option is also the container in which these headers can then be stored. This option therefore serves a dual purpose: it enables inspection of the response and it gives the developer easy access to the headers.

I think it would be better if no separate handler is required to get the functionality, even if this handler is part of the default set. For example, in our SDK, we chose not to use all handlers. We do not want the retry and redirect handlers. Also, we needed a custom handler for a custom request header. Therefore, we do not use the default set of handlers, but specify the handlers we want ourselves. Putting support for this option in a handler would require changes to our code to enable it, and I doubt we are the only ones in this situation. I think this might lead to surprised developers and bug reports. Given the fact that the option is required to enable this behavior, the overhead should be almost non-existent.

What I meant with 'The option can simply collect all headers', is that when this option is added to the request, it should collect all response headers for that single request. The developer should not have to specify which response headers to collect. In your example, you explicitly specified HeadersToInspect = new[] { "Link" }. This would require the developer to specify the name of the header twice: once to enable inspection and a second time to actually get the value. Also, it would make it impossible to inspect all headers without knowing in advance which headers will be present (for example for logging).

@sebastienlevert sebastienlevert changed the title Allow developers to inspect responses Allow developers to inspect headers Jul 24, 2023
@sebastienlevert
Copy link
Contributor

Your point on all headers is valid and something I'll consider. Sometimes a developer might not know that a headers is returned and might miss on critical information.

We had a discussion about this and we think that a mix between our vision and your suggestion would be a good candidate. While having it at the http level would be easier, bringing cross-cutting concerns there would add a ton of maintenance and burden for Kiota. We believe having is as a middleware / handler will provide the right level of functionality and also the right level of configuration.

We would provide the handler as part of the default chain and you would provide an option to request to inspect headers from request and/or response. We would still be using the option to fill the ResponseHeaders and RequestHeaders as we want to make sure we make it easy for async scenarios. We could, in the future, offer the ability to override the callback we'd use to rollout your own and capture the request / response header values.

// Set up the client and the request adapter and injects the response inspector handler
var authProvider = new AnonymousAuthenticationProvider();
var adapter = new HttpClientRequestAdapter(authProvider);
var client = new GitHubClient(adapter);

var headersInspectorOption = new HeadersInspectorOption
{
  InspectResponseHeaders = true;
  InspectRequestHeaders = true;
}

var repos = await client.Orgs["microsoftgraph"].Repos.GetAsync(
    request =>
      {
        request.Options.Add(headersInspectorOption);
      }
);

Console.WriteLine(headersInspectorOption.ResponseHeaders["Location"]);
Console.WriteLine(headersInspectorOption.RequestHeaders["SdkVersion"]);

Thanks for your feedback on this one, it will make a huge difference for developers using Kiota!

@baywet baywet assigned baywet and unassigned sebastienlevert Jul 24, 2023
@baywet baywet added generator Issues or improvements relater to generation capabilities. and removed needs more information labels Jul 24, 2023
@baywet
Copy link
Member

baywet commented Aug 11, 2023

Quick update here: as I was working on the dotnet implementation, I decided to reuse the headers class we have in abstractions for consistency.

@baywet
Copy link
Member

baywet commented Aug 14, 2023

Update: now implemented in 4 canonical languages. No PR in kiota itself closing.

@LukeButters
Copy link

LukeButters commented May 13, 2024

Does an example exist on how to inspect the response headers for dotnet? Perhaps it is done by supplying HeadersInspectionHandlerOption

@andrueastman
Copy link
Member

@LukeButters In C# this should looks something like

            var headersInspectionHandlerOption = new HeadersInspectionHandlerOption()
            {
                InspectResponseHeaders = true // specific you wish to collect reponse headers
            };
            //add the option and make the request with it
            var user = graphClient.Users["user-id"].GetAsync(requestConfiguration => requestConfiguration.Options.Add(headersInspectionHandlerOption));
            //use the key to get the Location header.
            var locationHeader = headersInspectionHandlerOption.ResponseHeaders["Location"];

MicrosoftDocs/openapi-docs#90 has been created to track to add this to the docs.

@rm-code
Copy link

rm-code commented Jun 26, 2024

@LukeButters Did you get it to work in .Net? The example code by @andrueastman doesn't capture anything for me:

image

@andrueastman
Copy link
Member

@rm-code Any chance you can share the sample code you're using? Did you pass the option to the relevant GetAsync method? Also, can you share how you initialize you client?

@tgreensill
Copy link

I had the same issue. The HeaderInspecitonHandler was missing from my HttpClient because I was using DI and HttpClientFactory to create my clients but forgot to register the handler. Changing the service registration code to this fixed the issue for me:

services.AddHttpClient(nameof(SomeClient)).AddHttpMessageHandler(() => new Microsoft.Kiota.Http.HttpClientLibrary.Middleware.HeadersInspectionHandler());

Using KiotaClientFactory.Create() to create the HttpClient instead of the HttpClientFactory also worked for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request generator Issues or improvements relater to generation capabilities.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

7 participants