[Bug]: Timeout strategy does not propagate correct CancellationToken

[DaRosenberg]

Describe the bug

This is similar to the stale issue #934, but since it's been 4 years and we are now talking about v8 resilience pipelines, I am choosing to file a new issue.

When a timeout strategy is present in a pipeline, Polly replaces the ResilienceContext.CancellationToken with an internal timeout token before invoking the user callback. It also registers the caller's original token so that cancelling the caller token cancels the internal timeout token. If the caller token is cancelled, downstream code observes cancellation on Polly's internal token and throws an OperationCanceledException carrying that internal token. Polly then propagates that original exception without restoring or translating the token back to the caller's original token.

This makes it difficult for calling code to distinguish caller-initiated cancellation from other failures (for example timeouts) by inspecting OperationCanceledException.CancellationToken.

Consider the following very common pattern:

try
{
	await pipeline.ExecuteAsync(callback, callerCancellationToken);
}
catch (OperationCanceledException ex) when (ex.CancellationToken == callerCancellationToken)
{
	throw;
}
catch (Exception ex)
{
	throw new ApplicationException("Operation failed.", ex);
}

In this case the cancellation really did come from callerCancellationToken, but the exception filter fails because the exception carries Polly's internal token instead. This behavior leaks an implementation detail of the timeout strategy into the exception observed by the caller. The caller's cancellation token caused the operation to cancel, but the escaping exception does not carry that token.

That is especially problematic for libraries that need to distinguish cancellation from genuine operation failures. A very common pattern is to let caller cancellation pass through unchanged while wrapping other exceptions with domain-specific context. If token identity is not preserved across Polly's timeout strategy, that pattern becomes unreliable even though the cancellation originated upstream.

The correct behavior would be for timeout strategies to translate upstream-triggered internal-token cancellation back into an OperationCanceledException associated with the previous/caller token, while continuing to throw TimeoutRejectedException when the timeout itself fires.

Expected behavior

If cancellation originates from the token passed to ExecuteAsync(..., cancellationToken), the OperationCanceledException escaping ExecuteAsync should carry that same caller token, or Polly should otherwise document that callers must not rely on OperationCanceledException.CancellationToken identity after executing through strategies such as timeout.

Note: Timeouts internal to Polly are different. If the timeout strategy itself causes cancellation while the caller token is not cancelled, the current behavior of throwing TimeoutRejectedException makes sense. The surprising case is caller-token cancellation routed through Polly's internal linked timeout token.

Actual behavior

When caller cancellation causes Polly's internal timeout token to be cancelled, the exception escaping ExecuteAsync incorrectly carries the internal callback token rather than the caller token passed to ExecuteAsync.

Steps to reproduce

Here's a minimal test (using TUnit) that reproduces the issue.

using Polly;

public class PollyCancellationTokenTests
{
	[Test]
	public async Task CallerCancellationThroughTimeoutStrategyPropagatesOriginalTokenAsync()
	{
		var pipeline =
			new ResiliencePipelineBuilder()
				.AddTimeout(TimeSpan.FromMinutes(1))
				.Build();

		using var cts = new CancellationTokenSource();

		var ex = await Assert
			.That(async () =>
			{
				await pipeline.ExecuteAsync(async cancellationToken =>
				{
					await cts.CancelAsync(); // Simulate cancellation request from upstream caller
					cancellationToken.ThrowIfCancellationRequested(); // Simulate cancellation response from downstream code
				}, cts.Token);
			})
			.Throws<OperationCanceledException>();

		await Assert
			.That(ex.CancellationToken)
			.IsEqualTo(cts.Token)
			.Because("CancellationToken that triggered the cancellation should be on the resulting exception.");
	}
}

The final assertion fails because ex.CancellationToken is not equal to cts.Token.

Expected to be equal to System.Threading.CancellationToken
but received System.Threading.CancellationToken

at Assert.That(ex.CancellationToken).IsEqualTo(cts.Token)

Polly version

8.6.6

.NET Version

net10.0

[martincostello]

This is by design for the same rationale as explained by #934 (comment).

For the strategy/policy to function correctly Polly needs to create its own CancellationTokenSource, which is then linked to yours so external cancellation is respected, so that things work as expected with respect to cancellation being signalled when it should (e.g. the user does not provide their own token, but still has a timeout configured) or if the strategy is wrapped within a different strategy (e.g. hedging where parallel operations should be cancelled once a result is available, or a different timeout strategy).

To get the behaviour you want (based just on looking at your repro code), instead of inspecting OperationCanceledException.CancellationToken you would inspect CancellationTokenSource.IsCancellationRequested instead, which would be true in this scenario showing that you cancelled it, rather than Polly.

[DaRosenberg]

Hi @martincostello — thanks for the response.

I did read that explanation. In it, you base the rationale partly on the (incorrect) claim that HttpClient behaves the same way, but I believe this was correctly refuted by the subsequent reply. As noted by the reply, HttpClient takes explicit care to give propagate the caller's original cancellation token back to the caller, even if it uses combined tokens internally for more orchestrated cancellation logic.

It is true that calling code could work around this bug by filtering on CancellationTokenSource.IsCancellationRequested instead, but this is semantically and behaviorally not the same. It's entirely feasible that my caller requested cancellation after whatever condition arose that caused Polly to cancel (for example the timeout), and for entirely different reasons. In some cases that matters. The workaround you are proposing would obscure this distinction, and always assume that my caller's cancellation is the thing that caused Polly's cancellation, just because my caller also happened to request cancellation (possibly at a later point in time).

The correct behavior of library code is to always propagate the caller's cancellation token back out if and only if the caller's token is what actually caused the cancellation, which is why HttpClient works that way.

[DaRosenberg]

By the way, if you agree, we'd be happy to take a stab at contributing a PR with a fix and related test coverage.

Whether it should be considered a breaking change is an interesting question in this case, but I would personally lean towards regarding this as non-breaking and allowing it in a patch. (It's difficult to imagine customer code currently relying on not getting their own cancellation token back.)

[martincostello]

My main point was that this isn't a bug as it was designed intentionally to do what it does today.

I'm open to the possibility of changing it if you'd like to investigate and propose a way to change the behaviour, but I have concerns that doing so in a way that works correctly for all scenarios across all strategies regardless of how deeply they nest/wrap might either might not be possible (i.e. it only works some of the time), or would require a large amount of special casing and/or logic to do correctly in many places across the codebase.

[DaRosenberg]

Ah ok, yeah sorry I didn't think so much about whether this is a "bug" in that sense, I'm just thinking about what's desirable/expected from a consumer code perspective.

Yes, I share your concern.

I'll do some investigating and see what I come up with.

[DaRosenberg]

@martincostello At the risk of having missed something, I think this was relatively easy to do. Only two of the strategies were impacted as far as I could tell. As noted in the PR, this fix doesn't cover third-party strategies (by choice to keep the change minimal). Take a look and let me know what you think.

#3094

[github-actions]

Thanks for creating this issue @DaRosenberg - the associated changes have been published as part of version 8.7.0 📦, which is now available from NuGet.org 🚀