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

FakeTimeProvider.Advance fails to advance delays (at least in the case of Polly) #4912

Closed
rcollette opened this issue Jan 27, 2024 · 2 comments · Fixed by #5169
Closed

FakeTimeProvider.Advance fails to advance delays (at least in the case of Polly) #4912

rcollette opened this issue Jan 27, 2024 · 2 comments · Fixed by #5169
Assignees
Labels
area-fundamentals bug This issue describes a behavior which is not expected - a bug. work in progress 🚧

Comments

@rcollette
Copy link

Description

When using FakeTimerProvider in unit tests, retries configured in a ResiliencyPipeline are not executed.

The library maintainer believes the issue lies in FakeTimerProvider
App-vNext/Polly#1932

Reproduction Steps

Please see the minimal unit test repo
https://github.com/rcollette/PollyRetryFakeTimeProviderIssue

Expected behavior

Delays used in Poly should be advanced.

Actual behavior

They don't appear to be triggered.

Regression?

No response

Known Workarounds

No response

Configuration

.NET 8.0.101
Polly 8.2.1

Other information

I tried to debug down into the WakeWaiters/InvokeCallback/TimerCallback code but was not seeing why this was not triggering.

@dariusclay
Copy link
Contributor

@geeknoid @martintmk So the issue here can be resolved in unit tests by ensuring that awaited tasks which need to be advanced by the time provider are configured with ConfigureAwait(true) to ensure execution continues on the captured context. Polly provides a way to pass in the ResilienceContext to use this behavior.

@dariusclay
Copy link
Contributor

@rcollette if you use the below code to execute the pipeline, the tests will succeed. Continuing on captured context is important here in the unit tests so that advancing the time synchronizes with the delays in the ExecuteAsync task.

    public async Task<int> PollyRetry(double taskDelay, double cancellationSeconds)
    {
        CancellationTokenSource cts = new(TimeSpan.FromSeconds(cancellationSeconds), timeProvider);
        Tries = 0;

        // get a context from the pool and return it when done
        var context = ResilienceContextPool.Shared.Get(
            // ensure execution continues on captured context 
            continueOnCapturedContext: true, 
            cancellationToken: cts.Token);

        var result = await _retryPipeline.ExecuteAsync(
            async _ =>
            {
                Tries++;

                // Simulate a task that takes some time to complete
                await Task.Delay(TimeSpan.FromSeconds(taskDelay), timeProvider);

                if (Tries <= 2)
                {
                    throw new InvalidOperationException();
                }

                return Tries;
            },
            context);

        ResilienceContextPool.Shared.Return(context);

        return result;
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-fundamentals bug This issue describes a behavior which is not expected - a bug. work in progress 🚧
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants