fix: fixed timeout retry strategy was stopping retries too early when retry deadline passed

[anitarua]

Work towards https://github.com/momentohq/dev-eco-issue-tracker/issues/1214

The fixed timeout retry strategy was not properly handling the case when a retry deadline was exceeded--instead of resetting the deadline and trying again, it was returning after only one retry attempt with a Cancelled error.

This PR makes sure that there are retries until the overall client timeout has passed.

Updated the "long delays" test to ensure that it retries at least twice and that the average time between retries is approximately equal to the retry timeout. This is because the first request should experience the long delay, attempt a retry, then hit the retry deadline and cancel that retry attempt rather than wait the entire longDelay period.

Also refactored so that CalculateRetryDeadline exists only on FixedTimeoutRetryStrategy (the only strategy where it's relevant) and removed the GetResponseDataReceivedTimeoutMillis getter. There have been no recent dotnet sdk releases so this shouldn't be a breaking change and is a cleaner implementation.

Renamed ResponseDataReceivedTimeoutMillis to RetryTimeoutMillis for clarity

[Copilot]

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

tests/Integration/Momento.Sdk.Tests/Retry/FixedTimeoutRetryStrategyTest.cs:176

• [nitpick] The jitter bounds in this test (0.85 - 1.15 multiplier) differ from those in other tests (0.9 - 1.1). Please confirm this discrepancy is intentional.

var minDelay = RETRY_DELAY.TotalMilliseconds * 0.85;

[malandis]

Great last change. I just recommend commenting on the fixed timeout retyr strategy to indicate the version of determine when to retry without the overall deadline won't be invoked/is a trivial implementation.

[anitarua]

@nand4011 @malandis

To recap the state of this PR:

• The last sdk release contains IRetryStrategy with only DetermineWhenToRetryRequest() defined on it. We want to avoid breaking changes to this interface, which means avoiding adding anything new.
• I had already added something new (GetResponseDataReceivedTimeoutMillis()) in a previous PR but this has not been released yet, so we still have the opportunity to fix it by removing GetResponseDataReceivedTimeoutMillis() from IRetryStrategy and its implementations.
• This PR creates a new interface IDeadlineAwareRetryStrategy : IRetryStrategy instead for the FixedTimeoutRetryStrategy. The new interface defines a CalculateRetryDeadline() method and uses a DetermineWhenToRetryRequest() that accepts overallDeadline as a parameter.
  • This version of DetermineWhenToRetryRequest() will reset the deadline if the overall deadline has not yet passed, which is the correct behavior for FixedTimeoutRetryStrategy.
• The RetryMiddleware calls the appropriate DetermineWhenToRetryRequest() based on what type of retry strategy interface it receives.
• Finally, this PR adds a new test and fixes a couple of previously written fixed timeout retry strategy tests.

End result should be cleaner implementation of FixedTimeoutRetryStrategy and bug fix for incorrect behavior (deadline was not properly overwritten with each retry when using this strategy).