fix(threading): widen RunWithTimeout<T1,TResult> retry handler to catch TaskCanceledException (#285)

[drmoisan]

Summary

TimeOutTask.RunWithTimeout<T1, TResult> armed its timeout with a CancellationTokenSource and awaited Task.Run(..., combinedToken.Token), but guarded its retry ladder with catch (TimeoutException). Timer-driven cancellation of that Task.Run surfaces as TaskCanceledException, which shares no base with TimeoutException other than Exception, so the retry ladder was unreachable for the exact case it exists to serve. The exception instead reached the general handler, where it was logged as a generic error and either rethrown (strict: true) or swallowed into a default return (strict: false).

Every one of the four sibling RunWithTimeout overloads already catches TaskCanceledException for the identical pattern. This overload was the outlier.

Closes #285.

The change

Two source files, 24 added and 6 removed lines in production, 40 added lines in test.

UtilitiesCS/Threading/TimeOutTask.cs — the handler becomes an additive filter:

catch (System.Exception e) when (e is TaskCanceledException || e is TimeoutException)

TimeoutException is retained because a wrapped delegate may raise it directly and two existing tests depend on that retry. System.Exception is spelled out rather than written bare because Microsoft.Office.Interop.Outlook, imported at line 9 of the file, also declares a type named Exception, so a bare Exception is CS0104-ambiguous. Every one of the ten general handlers already in this file is written the same way.

The filter deliberately does not catch OperationCanceledException. The combined token is never handed to the wrapped delegate, so a bare OperationCanceledException can only originate from an unrelated caller token and must propagate rather than be retried. Caller-token cancellation already arrives as TaskCanceledException and is re-thrown by the first statement in the clause body, token.ThrowIfCancellationRequested().

A trailing optional parameter Func<int, CancellationTokenSource>? timeoutSourceFactory = null was added to the public wrapper and the private implementation and threaded through the retry recursion, mirroring the seam already present on the Func<TResult> sibling. It makes the defect testable with no wall-clock wait and is behaviour-preserving when defaulted.

UtilitiesCS.Test/Threading/TimeOutTask_OverloadCoverageTests.cs — one appended test, RunWithTimeout_FuncT1TResult_ShouldRetryAfterTaskCanceledException. It injects an already-cancelled source for attempt 0 and a live source for attempt 1, so the outcome is fixed before any scheduling decision is taken. No Task.Delay, Thread.Sleep, or Thread.SpinWait; it runs in 55 ms.

Behavioural consequence, stated precisely

This makes currently-dead retry logic live. That is the intended outcome, but it is a real behavioural change at the two production call sites, both of which pass maxAttempts: 3 and strict: false:

• UtilitiesCS/OutlookObjects/Conversation/ConversationHelper.Formatting.cs line 80 (timeoutMs: 1000)
• UtilitiesCS/OneDriveHelpers/OneDriveDownloader.cs line 139

Worst-case latency in the affected case rises from roughly one second to roughly four, while the failure rate falls.

The scope of that timeout is narrower than it first appears, and this PR corrects a claim in spec.md that overstated it. The token given to Task.Run(() => function(arg1), combinedToken.Token) is observed only before the work item is dequeued, and function is a Func<T1, TResult> with no token parameter, so it cannot observe cancellation once running. A conversation.GetTable() call that has already begun is therefore never cancelled by the timeout and raises no exception at all. The retry ladder this change makes live is reached when the work item is still queued at the deadline — thread-pool saturation — not when the COM call itself is slow. Making the timeout cover a running delegate is a separate defect, already recorded under the spec's Non-Goals.

Verification

Fail-before / pass-after evidence is a real failing run, not an exception dossier.

Gate	Result
Regression test before the fix	Failed: 1, escaping System.Threading.Tasks.TaskCanceledException
Regression test after the fix	Passed: 1, Failed: 0
dotnet tool run csharpier check .	clean, 1565 files
Analyzer build (/t:Rebuild, EnableNETAnalyzers, EnforceCodeStyleInBuild)	0 errors, 5 warnings against a baseline of 5
Nullable build (/t:Rebuild, TreatWarningsAsErrors, no Nullable=enable)	0 errors
UtilitiesCS.Test	4771 passed, 0 failed, 0 skipped (baseline 4770 / 0 / 0)
QuickFiler.Test	1272 passed, 0 failed, 0 skipped (baseline 1272 / 0 / 0)

Both Phase 0 baseline-failure sets were empty, so the zero-failure result is literal rather than the product of a subtraction. The changed state machine UtilitiesCS.TimeOutTask.<RunWithTimeout>d__6<T1, TResult> reads line-rate="1" branch-rate="1" post-change at complexity="10", against complexity="4" at baseline, so every branch the change introduced is executed. The changed assembly UtilitiesCS reads 89.21% line and 83.04% branch.

Both at-risk pre-existing tests were verified unmodified by diff against the merge base: TimeOutTask_AdditionalTests.cs produced no diff output at all, and the diff to TimeOutTask_OverloadCoverageTests.cs contains zero deletion lines.

All 12 acceptance criteria in spec.md are checked, each against a named evidence artifact.

Review outcome

feature-review produced policy-audit, code-review, and feature-audit artifacts at 2026-09-01T09-10, all committed under the feature folder. Zero blocking findings, so no remediation cycle was run. One review finding was acted on in this PR: the spec.md timeout-scope correction described above.

Non-blocking findings deferred to follow-up work, all on the same production file:

• The new timeoutSourceFactory parameter transfers CancellationTokenSource disposal ownership to the callee and this is not documented on the public API.
• No test covers the retry-exhaustion arm reached specifically via TaskCanceledException.
• The seam now exists on 2 of 9 RunWithTimeout overloads.
• UtilitiesCS/Threading/TimeOutTask.cs is 1011 lines against the 500-line ceiling. It was already 993 at the merge base; this change adds 18. Splitting it would require paths outside this change's declared scope boundary.

Two repository-wide conditions were recorded and are not introduced by this change: the canonical artifacts/csharp/coverage.xml is absent, and raw merged repository-wide coverage reads 70.84% line against an 85% threshold. That denominator carries the test assembly, six third-party packages, and six first-party assemblies whose own test assemblies were not part of this run.

Scope

git diff --name-only against the merge base lists only UtilitiesCS/Threading/TimeOutTask.cs, UtilitiesCS.Test/Threading/TimeOutTask_OverloadCoverageTests.cs, paths under docs/features/active/2026-07-09-timeouttask-runwithtimeout-exception-type-mismatch-285/, and four .claude/agent-memory/ files. No .csproj, .props, or .targets was touched, and no generated coverage output was committed.

🤖 Generated with Claude Code