Let Restart resume polling after the action throws [patch]

[matt-edmondson]

Fixes #49

The documented recovery from an action that threw is RethrowExceptions() to observe the failure, then Restart() to resume. It could not complete. RestartAsync() opened with:

if (shouldPoll) { Stop(); await PollingTask.ConfigureAwait(false); }

ShouldPoll is only cleared by Stop(), so it is still true after the loop faults. The restart therefore awaited the already-faulted PollingTask and rethrew the same stale exception — from inside RestartAsync, before anything started a new loop. One exception in Action killed the instance permanently; every later Restart() just replayed it.

The fix, which is two things rather than one

1. A restart discards the old loop's outcome instead of awaiting it. A loop that faulted is replaceable exactly like one that ended normally, so the wait goes through a small WaitAndDiscardOutcomeAsync helper that observes the exception and moves on. This is the issue's suggested fix.

2. A finished ActionTask is cleared as part of restarting. This one is not in the issue, and fixing only the first half leaves the bug standing in a different place. TryRun throws on the tick that observes the fault and leaves ActionTask set — it clears the slot only on the non-faulted path:

if (ActionTask?.IsCompleted ?? false)
{
    if (ActionTask.Exception is not null)
    {
        throw ActionTask.Exception.GetBaseException();   // ActionTask still set
    }

    ActionTask = null;
}

So the new loop's very first TryRun re-observes the same faulted task and faults too. The restart would have swapped one dead loop for another, and the mutation test below shows exactly that.

Only a task that has already finished is cleared. That distinction is the whole reason the clearing lives in the restart rather than being an unconditional ActionTask = null: an action still running keeps its slot, so a restart mid-action cannot start a second one. The class's documented "prevents overlapping executions" guarantee is unchanged.

The concurrency note from the issue

Also addressed, since it is in the same method. RestartAsync read ShouldPoll, stopped, awaited, and assigned — re-taking Lock at each step rather than holding it across the awaits, which it cannot do. Two callers could each pass the checks and each start a loop, the second assignment orphaning the first, both then driving the same ActionTask field: the overlap the class says it prevents.

Restarts are now chained through a RestartGate task, so each waits for the previous one before it begins. That serializes them without holding a lock over an await and adds no disposable state to the class — a SemaphoreSlim would have made IntervalAction own a disposable and dragged a public IDisposable surface in with it, which is a bigger change than this bug is owed.

This half is not proven by a test, and I would rather say so than imply otherwise. ConcurrentRestartsLeaveTheInstancePolling fires eight concurrent restarts and asserts the instance is left polling and still stops cleanly — it exercises the path, but it passes on the old code too, because the only externally visible symptom of the race is a duplicate action invocation that no test can provoke without being timing-dependent. The argument for this part is structural, not empirical. Happy to drop it to a separate issue if you'd rather keep this PR to the fault path.

Tests

Three cases in IntervalActionTests:

test	asserts
RestartResumesPollingAfterActionThrows	the issue's acceptance criterion — action throws once, RethrowExceptions() then RestartAsync(), and the counter keeps climbing afterwards
RestartDoesNotRethrowTheStaleActionException	RestartAsync() itself completes rather than throwing, with an action that goes on throwing
ConcurrentRestartsLeaveTheInstancePolling	eight concurrent restarts leave one working instance (see the caveat above)

Confirmed the tests depend on the change by reverting each half separately and re-running:

mutation	result
await PollingTask restored in place of the discard	2 failed — RestartResumesPollingAfterActionThrows, RestartDoesNotRethrowTheStaleActionException
discard kept, ActionTask clearing removed	1 failed — RestartResumesPollingAfterActionThrows

Each half is independently load-bearing, which is why both are here.

Full suite green: 14 total, 0 failed (11 before, 3 added), run three times to check the timing-based cases are not marginal. Release build clean, zero warnings.

RethrowExceptionsThrowsException is unchanged and still passes — an action exception still faults the loop and is still reported once through RethrowExceptions(). This changes what happens after that, not whether it happens.

Not covered

An action that faults while polling is already stopped is not separately tested. It works — the restart clears the finished task the same way — but the failure is never surfaced through RethrowExceptions() in that window, which is pre-existing behaviour and not something this change should quietly alter.

🤖 Generated with Claude Code

https://claude.ai/code/session_01UscjStBJdW3uHm5NR289DX

---

Generated by Claude Code

[sonarqubecloud]

Quality Gate passed

Issues
5 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[matt-edmondson]

CI is green on d4b7995 — all 12 checks, including tests on ubuntu, windows and macos, CodeQL and SonarCloud.

Noting the "5 New issues" on the Sonar badge, since the count is more alarming than the contents. All five are the same rule, MSTEST0049 at INFO, on the awaits in the three new tests:

Consider using the overload that accepts a CancellationToken and pass 'TestContext.CancellationToken'

Leaving them as they are. The file has 18 such calls and 13 of them predate this PR, all written the same way — Sonar reports only the five because it scores new code. Threading TestContext.CancellationToken through my tests alone would make the file inconsistent with itself while fixing nothing this PR broke, and adopting it across all 18 is a change to the test suite's convention that belongs in its own PR rather than riding along with a bug fix.

The quality gate passed, with 100% coverage on new code and no security hotspots.

---

Generated by Claude Code