notion-sdk-net icon indicating copy to clipboard operation
notion-sdk-net copied to clipboard

HttpClient should be initiated through HttpClientFactory / .AddHttpClient<>

Open mrcunninghamz opened this issue 6 months ago • 1 comments

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

mrcunninghamz avatar May 29 '25 20:05 mrcunninghamz