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

[Feature Request] Allow configuration of HttpClients for IDownstreamWebApi #1740

Closed
h3rmanj opened this issue May 24, 2022 · 9 comments
Closed
Labels
enhancement New feature or request feature request

Comments

@h3rmanj
Copy link
Contributor

h3rmanj commented May 24, 2022

Is your feature request related to a problem? Please describe.
I'd like to add Polly policies, described in these docs on how to implement resilient http requests.

However I can't really access the HttpClientBuilder when using the recommended way to call downstream apis.

There seems to be some incompatibility in whats recommended, and I'd like the gap to be closed.

Describe the solution you'd like
Some way to configure the HttpClients used by IDownstreamWebApi. I've proposed a solution in #1735 which allows the user to pass a configureHttpClientBuilder to AddDownstreamWebApi().

Additional context
This was partly pointed out in #891 but wasn't really addressed when closed.

@luismanez
Copy link

Not sure if this helps, but this is what I´m doing to achieve what you need. Basically, I´m overwriting the HttpClient used by AddDownstreamWebApi() and then adding my Polly stuff in the HttpClientBuilder returned. In my case, it also helps to change some headers, as I´m calling SharePoint REST API, and returned XML by default

services
                .AddHttpClient<IDownstreamWebApi, DownstreamWebApi>(client =>
                {
                    client.DefaultRequestHeaders.Add("Accept", "application/json");
                })
                .AddMyPollyStuff();

@h3rmanj
Copy link
Contributor Author

h3rmanj commented Jun 29, 2022

I didn't think of that, looks like a good solution/workaround in scenarios where one policy is enough.

In my case I call multiple downstream APIs, each with different needs, so I need to be able to specify a policy per API.

@bosteelsd
Copy link

We would really like this for our project too.

@h3rmanj
Copy link
Contributor Author

h3rmanj commented Nov 9, 2022

It's coming in v2 with the new DownstreamRestApi interface
#1735 (comment)

@jennyf19
Copy link
Collaborator

done in 2.5.0.

@EnricoMassone
Copy link

EnricoMassone commented Feb 23, 2025

I recently ran into the very same problem of registering a Polly-based HTTP message handler for the underlying HttpClient of IDownstreamApi. I have also asked a question about this in the Discussions section.

Just as a confirmation, is the suggested solution the one showed here by @h3rmanj ? If that is the case, it could be great to explain that in the library documentation. It seems it is not mentioned anywhere in the available documentation and examples for IDownstreamApi.

@h3rmanj
Copy link
Contributor Author

h3rmanj commented Feb 23, 2025

Can confirm, currently using this extension when registering our downstream clients. Works with both on-behalf-of and client credential scenarios.

/// <summary>
/// Extension methods to support downstream web API services.
/// </summary>
public static class DownstreamApiExtensions
{
    /// <summary>
    /// Adds a named downstream web API service related to a specific configuration section.
    /// </summary>
    /// <param name="services">Services.</param>
    /// <param name="serviceName">Name of the configuration for the service.
    /// This is the name used when calling the service from controller/pages.</param>
    /// <param name="configuration">Configuration.</param>
    /// <returns>The httpClientBuilder for the client to use.</returns>
    public static IHttpClientBuilder AddDownstreamApiClient(this IServiceCollection services,
        string serviceName,
        IConfiguration configuration)
    {
        services.AddDownstreamApi(serviceName, configuration);

        return services.AddHttpClient(serviceName);
    }
}

Agree docs should be added for this.

@EnricoMassone
Copy link

EnricoMassone commented Feb 24, 2025

Can confirm, currently using this extension when registering our downstream clients. Works with both on-behalf-of and client credential scenarios.

///


/// Extension methods to support downstream web API services.
///

public static class DownstreamApiExtensions
{
///
/// Adds a named downstream web API service related to a specific configuration section.
///

/// Services.
/// Name of the configuration for the service.
/// This is the name used when calling the service from controller/pages.
/// Configuration.
/// The httpClientBuilder for the client to use.
public static IHttpClientBuilder AddDownstreamApiClient(this IServiceCollection services,
string serviceName,
IConfiguration configuration)
{
services.AddDownstreamApi(serviceName, configuration);

    return services.AddHttpClient(serviceName);
}

}
Agree docs should be added for this.

@h3rmanj many thanks for the confirmation. I will do the same for my project.

Still wondering if this is the intended way to use the abstraction. This is working because the underlying HttpClient is registered with the same name used to register the downstream api. Although this is reasonable, this solution relies on an implementation detail. Your initially proposed solution seems much better to me.

If you want to be safer in terms of breaking changes, an alternative is manually getting an access token and then use it with an instance of HttpClient created by IHttpClientFactory and registered in the standard way with the IHttpClientBuilder. This is basically the approach used in this documentation. Of course, this implies not using IDownstreamApi in the first place.
In my humble opinion, the approach used by other authentication libraries like this one is overall better, since there is no additional abstraction built on top of HttpClient and this makes life simpler in scenarios like the one discussed here.

@EnricoMassone
Copy link

I finally filed a new issue (see #3258) for this topic @h3rmanj

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request feature request
Projects
None yet
Development

No branches or pull requests

5 participants