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

Use of RetryDelegatingHandler with IHttpClientFactory #1026

Closed
krispenner opened this issue Jul 19, 2020 · 2 comments · Fixed by #1030
Closed

Use of RetryDelegatingHandler with IHttpClientFactory #1026

krispenner opened this issue Jul 19, 2020 · 2 comments · Fixed by #1030
Labels
status: work in progress Twilio or the community is in the process of implementing type: twilio enhancement feature request on Twilio's roadmap

Comments

@krispenner
Copy link

Issue Summary

The RetryDelegatingHandler is not used when using IHttpClientFactory. In order to manually register it, it requires that the base DelegatingHandler class have it default ctor called which is not the case with the current implementation.

This is what I'm trying to do...

Steps to Reproduce

Services
    .AddSingleton(new ReliabilitySettings(2, TimeSpan.FromSeconds(1), TimeSpan.FromSeconds(30), TimeSpan.FromSeconds(2)))
    .AddTransient<RetryDelegatingHandler>()
    .AddHttpClient(nameof(SendGrid))
    .AddHttpMessageHandler<RetryDelegatingHandler>()
    .ConfigureHttpClient(httpClient =>
    {
        httpClient.Timeout = TimeSpan.FromSeconds(60);
    });

Changes that would fix this is to call base default ctor instead of creating a new HttpClientHandler.

Change this line to the below:

public RetryDelegatingHandler(ReliabilitySettings settings)
// remove this line
//  : this(new HttpClientHandler(), settings)
{
    // add this line
    this.settings = settings;
}

Was there a specific reason to create an HttpClientHandler? If so, maybe need a InjectableRetryDelegatingHandler class specific for this use case.

Or is there any other option for this? The only other option I can think of is to make my own retry handler or use Polly but I would prefer to use the one created specifically by SendGrid for SendGrid. Seems reasonable to expect this to work.

Exception/Log

The 'InnerHandler' property must be null. 'DelegatingHandler' instances provided to 'HttpMessageHandlerBuilder' must not be reused or cached. Handler: 'SendGrid.Helpers.Reliability.RetryDelegatingHandler'
@childish-sambino
Copy link
Contributor

I think the refactor suggested is fine. The HttpClientHandler can be created 1 level up (which is what's done when dealing with an IWebProxy). I'll work on an update.

@krispenner
Copy link
Author

Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: work in progress Twilio or the community is in the process of implementing type: twilio enhancement feature request on Twilio's roadmap
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants