Skip to content

HttpClient should be initiated through HttpClientFactory / .AddHttpClient<> #462

@mrcunninghamz

Description

@mrcunninghamz

Describe the bug
The current implementation of the RestClient has a coupled HttpClient instantiation in line 186. To be fair in the ServiceCollectionExtension the whole notionclient is a singleton which mitigates issues that you could run into.

My main issue that I was having was that I wanted to have more control of the httpclient and be able to add resilient policies to it via the .AddPolicyHandler extensions.

Additional context
In my project I basically copied the RestClient code, but made the logger and HttpClient a dependency that will get injected. ClientOptions is no longer a dependency and instead it comes in through the IOptions pattern and uses the configuration to setup the httpclient.

public static IHttpClientBuilder AddNotionClient(
        this IServiceCollection services, IConfiguration configuration)
    {
        
        // Register HttpClient for NotionService
        services.Configure<ClientOptions>(configuration.GetSection("Notion"));

        var httpClientBuilder = services.AddHttpClient<IRestClient, NotionBaeRestClient>((srv, httpClient) =>
        {
            var options = srv.GetService<IOptions<ClientOptions>>()?.Value ?? throw new Exception("Notion client options not configured");

            httpClient.BaseAddress = new Uri(options.BaseUrl);
            httpClient.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", options.AuthToken);
            httpClient.DefaultRequestHeaders.Add("Notion-Version", options.NotionVersion);
            
        })
        .ConfigurePrimaryHttpMessageHandler(() => new LoggingHandler { InnerHandler = new HttpClientHandler() });

        services.AddSingleton<IUsersClient, UsersClient>();
        services.AddSingleton<IDatabasesClient, DatabasesClient>();
        services.AddSingleton<IPagesClient, PagesClient>();
        services.AddSingleton<ISearchClient, SearchClient>();
        services.AddSingleton<ICommentsClient, CommentsClient>();
        services.AddSingleton<IBlocksClient, BlocksClient>();
        services.AddSingleton<IAuthenticationClient, AuthenticationClient>();
        services.AddSingleton<INotionClient, NotionClient>();
        
        return httpClientBuilder;
    }

my appsettings.local.json file looks like this

{
    "Notion" : {
        "AuthToken" : "<APITOKEN>",
        "BaseUrl" : "https://api.notion.com/",
        "NotionVersion" : "2022-06-28"
    }
}

in my Program.cs file i am using the extension like this

builder.Services.AddNotionClient(builder.Configuration)
    .AddPolicyHandler(Policy.BulkheadAsync<HttpResponseMessage>(10, Int32.MaxValue))
    .AddPolicyHandler((provider, _) => GetRetryOnRateLimitingPolicy(provider));

I can fork and make a PR if you are interested in this. If not, no hard feelings here, my needs are pretty specific but I wonder if anyone else who wants to do retries to get around api rate limits or simply have a bit of retry / resilience could benefit from this..

If you would like to review my code and how its implemented / being used in my context my project can be found here NotionBae MCP Server

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions