Skip to content

TrackedSession completes on ANY satisfied condition, not ALL — multi-condition tracked sessions return early #3824

Description

@jeremydmiller

TrackedSession.IsCompleted() short-circuits on the first satisfied condition instead of requiring
all of them, so any tracked session that registers more than one condition can return while the work it
was told to wait for is still running.

public bool IsCompleted()
{
    if (!_executionComplete) return false;

    if (_conditions.Any(x => x.IsCompleted()))   // <-- ANY
    {
        return true;
    }

    if (!_envelopes.All(x => x.IsComplete()))
    {
        return false;
    }

    return !_conditions.Any() || _conditions.All(x => x.IsCompleted());  // <-- ALL, unreachable
}

The All(...) on the last line is dead code whenever any single condition is satisfied. With one
condition Any and All are identical, which is why this survived: nearly every session registers just
one.

Why it matters beyond our own suite

Wolverine.Tracking is public test-support API — this is what users call in their own test suites. A
user chaining

.WaitForMessageToBeReceivedAt<Thing>(hostA)
.WaitForMessageToBeReceivedAt<Thing>(hostB)

gets a session that returns when either host handles the message, then asserts against handlers that
may still be running. The failure mode is a test that passes locally and fails under load — the worst
kind to debug, because the assertion names a null value and points nowhere near the cause.

There is already a workaround in the codebase that implies this was hit before and routed around rather
than fixed. WaitForExecutionOf<T> deliberately merges repeat calls into one condition object, and its
own doc comment says so:

Multiple calls combine into a single condition that requires every registered count to be reached.

That is only necessary because multiple conditions do not AND together.

How it surfaced

Wolverine.RabbitMQ.Tests.end_to_end has been tagged [Trait("Category", "Flaky")] — excluding 20
tests
from CI. Its two persistent failures, use_fan_out_exchange and
use_direct_exchange_with_binding_key, each chain three WaitForMessageToBeReceivedAt calls for a
fan-out exchange.

The in-file triage had it half right: both tests pass alone, fail in-class, and fail in ~500ms on a null
ColorHistory, nowhere near their 30s timeout. It then inferred the cause — that
WaitForMessageToBeReceivedAt is satisfied by MessageFailed, so a message that arrived and then failed
ended the session early — and recorded that the next step was to dump the session rather than infer.

Doing that dump refutes the inference:

PROBE status=Completed exceptions=0
Sent ColorChosen to rabbitmq://exchange/exchange-...
Received ColorChosen at rabbitmq://queue/messages-...-20e23   (node 5321b23d)
Received ColorChosen at rabbitmq://queue/messages-...-21e23   (node 7d6a0afc)
Received ColorChosen at rabbitmq://queue/messages-...-22e23   (node ee1d1ae4)
Started execution of ColorChosen                              (node ee1d1ae4)
Finished execution of ColorChosen                             (node ee1d1ae4)
ColorChosen was marked as successful.                         (node ee1d1ae4)

Zero exceptions, nothing failed, and the message succeeded — at exactly one of the three receivers. The
session ended there because that one node's condition was enough to satisfy Any. Alone on an idle
machine all three receivers finish within the same millisecond, so the race is invisible; in-class, under
load, the other two lag and their ColorHistory is still null when the assertion runs.

Fix

Require all conditions, keeping the existing "conditions are the exit criteria" shortcut so the only
behaviour that changes is the multi-condition case:

if (_conditions.Any())
{
    return _conditions.All(x => x.IsCompleted());
}

return _envelopes.All(x => x.IsComplete());

Blast radius

Small, and measured rather than assumed:

  • 163 WaitForMessageToBeReceivedAt call sites repo-wide; exactly one file chains more than one in a
    single session — the two tests this fixes.
  • One documentation sample (TestingSupportSamples.cs) mixes WaitForMessageToBeReceivedAt with
    WaitForExecutionOf. Its own comments describe wanting both, which the old behaviour did not honour —
    the fix makes it match its documentation.
  • CoreTests: 2246 passed, 0 failed.
  • Wolverine.RabbitMQ.Tests.end_to_end: 20/20 on three consecutive runs, previously 18/20.

The [Trait("Category", "Flaky")] tag comes off that class, returning 20 tests to CI.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions