Skip to content

Comments

feat: retry policies with exponential backoff and exception filtering#4949

Merged
thomhurst merged 2 commits intomainfrom
feat/retry-policies
Feb 19, 2026
Merged

feat: retry policies with exponential backoff and exception filtering#4949
thomhurst merged 2 commits intomainfrom
feat/retry-policies

Conversation

@thomhurst
Copy link
Owner

Closes #4890. Adds BackoffMs, BackoffMultiplier, RetryOnExceptionTypes to [Retry].

@claude
Copy link
Contributor

claude bot commented Feb 19, 2026

Code Review

Overall this is a clean, well-structured feature addition. The API design is intuitive and the documentation is thorough. Two issues found:


Issue 1: Public API snapshot tests not updated (CLAUDE.md Rule 2 violation)

The PR adds several new public API members to TUnit.Core but does not update the .verified.txt snapshot files in TUnit.PublicAPI/. These files currently show the old RetryAttribute API surface without the new properties, and the snapshot tests will fail on CI.

New public members missing from all four .verified.txt files:

  • RetryAttribute.BackoffMs
  • RetryAttribute.BackoffMultiplier
  • RetryAttribute.RetryOnExceptionTypes
  • DiscoveredTestContext.SetRetryBackoff(int, double)
  • TestDetails.RetryBackoffMs
  • TestDetails.RetryBackoffMultiplier
  • ITestConfiguration.RetryBackoffMs
  • ITestConfiguration.RetryBackoffMultiplier

Per CLAUDE.md: "Changes to source generator output or public APIs require running snapshot tests. Commit .verified.txt files."

Fix: Run the snapshot tests, review the generated .received.txt files, and accept them:

# From TUnit.Core.SourceGenerator.Tests or the relevant test project:
# Linux/macOS:
for f in *.received.txt; do mv "$f" "${f%.received.txt}.verified.txt"; done
# Then commit the updated .verified.txt files

Issue 2: Integer overflow in exponential backoff delay calculation

In RetryHelper.cs:

var delayMs = (int)(backoffMs * Math.Pow(multiplier, attempt));

if (delayMs > 0)
{
    await Task.Delay(delayMs, testContext.CancellationToken).ConfigureAwait(false);
}

Math.Pow returns a double. When backoffMs * Math.Pow(multiplier, attempt) exceeds int.MaxValue (2,147,483,647), the (int) cast in an unchecked context (C# default) produces an implementation-defined value — typically int.MinValue (-2,147,483,648) on .NET x64.

With BackoffMs = 1000 and the default BackoffMultiplier = 2.0, this happens at attempt 22 (1000 × 2²² = 4,194,304,000 > int.MaxValue). At that point delayMs becomes negative, the if (delayMs > 0) guard evaluates to false, and exponential backoff silently stops working — retries continue with no delay instead of increasing delays.

While attempt 22 is extreme, smaller BackoffMs values with a higher multiplier (e.g., BackoffMs = 500, BackoffMultiplier = 3.0) overflow much sooner.

Recommended fix — cap the computed delay to avoid the overflow:

private static async Task ApplyBackoffDelay(TestContext testContext, int attempt)
{
    var backoffMs = testContext.Metadata.TestDetails.RetryBackoffMs;

    if (backoffMs <= 0)
    {
        return;
    }

    var multiplier = testContext.Metadata.TestDetails.RetryBackoffMultiplier;
    var rawDelayMs = backoffMs * Math.Pow(multiplier, attempt);
    
    // Cap to int.MaxValue to avoid undefined behavior from unchecked cast
    var delayMs = (int)Math.Min(rawDelayMs, int.MaxValue);

    if (delayMs > 0)
    {
        await Task.Delay(delayMs, testContext.CancellationToken).ConfigureAwait(false);
    }
}

Or use TimeSpan.FromMilliseconds which accepts a double directly, avoiding the cast entirely:

var delay = TimeSpan.FromMilliseconds(rawDelayMs);
await Task.Delay(delay, testContext.CancellationToken).ConfigureAwait(false);

(TimeSpan.FromMilliseconds accepts a double and handles large values gracefully, and Task.Delay(TimeSpan, CancellationToken) accepts TimeSpan up to ~24.8 days.)

…filtering

Enhance the existing [Retry] attribute with built-in retry policy support:
- BackoffMs: initial delay between retries (default 0 = no delay)
- BackoffMultiplier: multiplier for exponential backoff (default 2.0)
- RetryOnExceptionTypes: only retry on specific exception types (Type[])

The backoff delay is applied in RetryHelper before each retry attempt,
growing exponentially as BackoffMs * BackoffMultiplier^attempt. The
exception filtering is integrated into the base ShouldRetry method,
preserving full backward compatibility with existing custom subclasses.
@thomhurst thomhurst merged commit c0a521c into main Feb 19, 2026
11 of 14 checks passed
@thomhurst thomhurst deleted the feat/retry-policies branch February 19, 2026 09:37
thomhurst added a commit that referenced this pull request Feb 19, 2026
The merged PRs #4929 (hardcoded constants) and #4949 (retry policies)
added new public API surface to TUnit.Core but the DotNet8_0, DotNet9_0,
and DotNet10_0 verified snapshot files were not updated, causing
Core_Library_Has_No_API_Changes test failures in all PR CI pipelines.
thomhurst added a commit that referenced this pull request Feb 19, 2026
* fix: update Core public API snapshots for all frameworks

The merged PRs #4929 (hardcoded constants) and #4949 (retry policies)
added new public API surface to TUnit.Core but the DotNet8_0, DotNet9_0,
and DotNet10_0 verified snapshot files were not updated, causing
Core_Library_Has_No_API_Changes test failures in all PR CI pipelines.

* fix: update analyzer tests for CancellationToken parameter location

PR #4956 changed the CancellationTokenMustBeLastParameter diagnostic
to point at the CancellationToken parameter instead of the method name.
Update 3 test expectations to match the new diagnostic location.
@claude claude bot mentioned this pull request Feb 22, 2026
1 task
This was referenced Feb 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: built-in retry policies (exponential backoff, exception-type filtering)

1 participant