HttpRequestMessage retries issue and HttpClient lifetime

[PatrykPlewaOfficial]

Background

HttpRequestMessage instance is reused during retries which throws an exception.
HttpClient lifetime is equal to the lifetime of the entire CrowdinApiClient.

Issue

When retries mechanism is enabled

System.InvalidOperationException
  Message=The request message was already sent. Cannot send the same request message multiple times.
  Source=System.Net.Http
  StackTrace:
   at System.Net.Http.HttpClient.CheckRequestMessage(HttpRequestMessage request)
   at System.Net.Http.HttpClient.CheckRequestBeforeSend(HttpRequestMessage request)
   at Crowdin.Api.CrowdinApiClient.<>c__DisplayClass90_0.<SendRequest>b__0()
   at Crowdin.Api.Core.Resilience.RetryService.<ExecuteRequestAsync>d__2`1.MoveNext()
   at Crowdin.Api.Core.Resilience.RetryService.<ExecuteRequestAsync>d__2`1.MoveNext()
   at Crowdin.Api.CrowdinApiClient.<SendRequest>d__90.MoveNext()
   at Crowdin.Api.Languages.LanguagesApiExecutor.<ListSupportedLanguages>d__5.MoveNext()
   at Crowdin.Api.CrowdinApiClient.<WithFetchAll>d__89`1.MoveNext()
   at Program.<>c__DisplayClass0_0.<<<Main>$>g__GetSupportedLanguages|5>d.MoveNext()

Test environment

• .NET 6 runtime
• Crowdin.Api v2.11.1

Reproduction

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net6.0</TargetFramework>
    <Nullable>enable</Nullable>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Crowdin.Api" Version="2.11.1" />
    <PackageReference Include="Newtonsoft.Json" Version="13.0.2" />
  </ItemGroup>

</Project>

var credentials = new CrowdinCredentials
{
    AccessToken = "secret",
    Organization = "test-org",
    BaseUrl = "http://localhost:58190" // some invalid Url on purpose, to make it fail
};

var client = new CrowdinApiClient(
    credentials,
    retryService: new RetryService(new RetryConfiguration
    {
        RetriesCount = 5,
        WaitIntervalMilliseconds = 1500
    })
);

var languages = await CrowdinApiClient
.WithFetchAll((limit, offset) => client.Languages.ListSupportedLanguages(limit, offset), amountPerRequest: 500);

Workaround

The only workaround for this moment to make retries work is to define external policy and wrap the entire process of CrowdinApiClient instance creation and sending a single request as one.

Solution

I've come up with a solution for this issue in this PR.
Creating a new instance of the HttpClient and a new instance of the HttpRequestMessage.
That solved the issue.

It's a breaking change due to changes in the constructor of the CrowdinApiClient.

It also resolves another issue. The HttpClient instance should be used only for a very small period of time. In the suggested solution the HttpClient instance is created before every API call.

An additional benefit is that now the developers can use IHttpClientFactory to create new instances of the HttpClient.

Testing

This code can't be really unit tested. I only managed to test it locally with API calls.

Future improvements

The signature of the retry method can be improved.

From

public interface IRetryService
    {
        Task<T> ExecuteRequestAsync<T>(Func<Task<T>> func);
    }

Into

public interface IRetryService
    {
        Task<HttpResponseMessage> ExecuteRequestAsync(Func<Task<HttpResponseMessage>> func);
    }

This will allow more advanced users to define their own Retry services e.g. using Polly thanks to the HttpResponseMessage publicly exposed in the method's signature.

[innomaxx]

Hi @PatrykPlewaOfficial,

Thank you for contribution to this project! I left a comment for your PR, please check it out.