Repository navigation
Backport #8516 to v1.5: WithinAsync failure swallow + the ClusterClientSpec phase it unmasked - #8529
Merged
Aaronontheweb merged 2 commits intoSep 11, 2026
Conversation
…deadline wins (akkadotnet#8516) * Fix akkadotnet#8483: WithinAsync no longer swallows the block's failure WithinAsync races the block against Task.Delay(max + 200ms). When the delay won that race the block's Task was never observed: a faulted block lost its exception and the test carried on as if the block had succeeded. The elapsed time check could not catch it either, because the default epsilon (max(0.15 * max, 50ms)) covers the 200ms of slack for any max above ~1.4s. When the delay wins, the block is now observed: - Completed - awaited, so a faulted or cancelled block rethrows with its original stack trace, and a block that finished as the deadline fired still yields its result. - Still running - reported as a failure that names the elapsed time and the maximum allowed duration, instead of returning as if the block had passed. The success path, the min path, the epsilon defaults and every timeout constant are untouched. Only the discarded outcome becomes visible. Tests cover both halves of the race, the unchanged success path, and the exception that a block throws before the deadline. The synchronous Within overloads never had the defect - the action runs inline inside the delegate invocation, so it throws before any race exists - and a test pins that. * De-flake ClusterClientSpec server-restart phase: real rebind, real rendezvous The `reestablish connection to receptionist after server restart` phase failed on three nodes across two transports in the first CI run under the honest `WithinAsync` (build 131110): classic `second`, artery `client` and `third`, all with "Block was still running after 00:01:00.2, exceeding the maximum allowed duration of 00:01:00". The old primitive returned `default` and marched on, so the phase had been fake-passing. Three defects, all now fixed: 1. The restarted server bound the wrong port under Artery. The phase pinned the replacement system with `akka.remote.dot-netty.tcp.port` only, which is inert when the suite runs on Artery, so the fresh system took a random `canonical.port`: node `third` came back on 43295 after dying on 34091 and the client spent the rest of the phase re-sending GetContacts to an address nothing was listening on. Use `StartNewSystemAsync`, which pins host:port on both transports. 2. `EnterBarrier("reconnection-verified")` could never rendezvous. `TestConductor.Shutdown` terminates the target's whole `Sys`, and the conductor client lives in `Sys`, so the restarted node had no conductor left: it never logged `entering barriers reconnection-verified` at all. On the client the conductor had already dropped that role, so the same barrier passed in 1ms (18:23:24.218 -> .219) without synchronizing anything - while the server sat in a barrier ask it could not complete. `StartNewSystemAsync` attaches a fresh conductor, so the barrier is now a real rendezvous, and it is what keeps the restarted system alive until the client has proven reconnection. 3. Neither `EventFilter` carried a budget, so both fell back to `RemainingOrDefault` - the umbrella's remaining time. Filter and umbrella then expired together and "Block was still running" won the race, hiding which log line never arrived. Both filters now take bounds derived from the client's own cadences: 10s for "Lost contact" (heartbeat-interval 1s + acceptable-heartbeat- pause 3s + one tick + the conductor round trip) and 20s for "Connected to" (retry-gate-closed-for 5s + 3 x establishing-get-contacts-interval 3s). The 60s umbrella is gone. Barriers must not run inside a Within, whose `RemainingOr(barrier-timeout)` starves the rendezvous exactly when the phase has run long; the trailing `after-N` barriers in the other phases move outside their umbrellas for the same reason, and the startup phase - five barriers around one convergence poll - drops its umbrella entirely in favour of an explicit `AwaitCount` budget. Also converts the phase's one remaining synchronous `ExpectMsg` to `ExpectMsgAsync`. Backport note (v1.5) Both defects were confirmed present on v1.5 before the pick. `TestKitBase_Within.cs` was byte-identical to the commit's parent on dev, so the swallow is the same code. The `ClusterClientSpec` restart phase carried the same hand-rolled `ActorSystem.Create`, the same `reconnection-verified` barrier after `TestConductor.ShutdownAsync`, the same 60s umbrella and the same budget-less `EventFilter`s. Scoping of the dev-only claims above: the CI build number and the per-node port and timestamp evidence come from dev's run and were not re-observed here. v1.5 has no Artery, so restart-phase defect 1 (the replacement system taking a random `canonical.port` because only `akka.remote.dot-netty.tcp.port` was pinned) cannot occur on this branch - the classic lane is the only transport and that key is live. `StartNewSystemAsync` is still the correct call on v1.5: it pins host and port for dot-netty and, more importantly here, attaches a fresh conductor, which is what defect 2 needs. Defects 2 and 3 are transport-independent and apply in full. (cherry picked from commit 94c1949)
Aaronontheweb
added
tests
akka-testkit
Aaronontheweb
enabled auto-merge (squash)
September 9, 2026 01:53
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport of #8516 to the v1.5 maintenance line — a straight
cherry-pick -xof dev's94c194933, zero conflicts, provenance trailer kept. Both defects were verified present on v1.5 before picking.What v1.5 gets
The
WithinAsyncfailure swallow, fixed. v1.5'sTestKitBase_Within.cswas byte-identical to dev's pre-fix file: when the deadline won the race, a failing block's exception was discarded and the test passed. Now a finished block rethrows and a still-running block fails loudly. Fail-first was re-demonstrated on v1.5's own pre-pick code: both new tests fail withno exception was thrown, the swallow verbatim.The one spec that fake-passed under it, fixed. v1.5's
ClusterClientSpecrestart phase has the same transport-independent defect dev had:TestConductor.ShutdownAsynckills the node's conductor client, so the restarted system never re-attached and thereconnection-verifiedbarrier was a no-op — all hidden by a 60s umbrella over ~7s of real work. Running the unfixed spec under the fixed primitive reproduced the still-running failure on the first attempt, on v1.5's classic lane (nodethird, the restarted server). The artery random-port half of dev's defect does not apply here (no artery on v1.5); the dot-netty port pin is live on this branch, andStartNewSystemAsyncexists with the same contract.Validation on v1.5
Akka.TestKit.TestsfullClusterClientSpecclassic ×6 + ×2 under CPU load (loadavg 21–29)ClusterClientHandoverSpec×2Akka.API.Tests-warnaserroron touched projectsRecon findings (reported, not fixed, per v1.5 policy)
Two failures appeared during the recon; both reproduce identically on the pre-pick primitive, so neither is an unmasking:
TcpIntegrationSpecIPv6 DNS case — environmental (v4-firstlocalhostresolution against a v6-only listener).DistributedPubSubDeadLetterSpec— order-dependent in full-suite runs on both primitives; passes in isolation.Notes
AKKA_MNTR_TRANSPORT=artery, a mechanism v1.5 does not have. Left verbatim per cherry-pick fidelity; trimming those three lines is a one-liner if preferred.Backport note (v1.5)scoping dev-only claims above the provenance line.RELEASE_NOTES.mduntouched.