Skip to content

Harden archive polling helpers against transient empty reads - #960

Open
eduardosmaniotto wants to merge 1 commit into
MUnique:masterfrom
eduardosmaniotto:fix/flaky-archive-polling-tests
Open

eduardosmaniotto wants to merge 1 commit into
MUnique:masterfrom
eduardosmaniotto:fix/flaky-archive-polling-tests

Conversation

@eduardosmaniotto

Copy link
Copy Markdown
Contributor

Problem

MUnique.OpenMU.Tests.NetworkObservationTests.TrafficOfAnObservedAccountIsArchivedAsync
failed randomly in the Azure pipeline with
System.ArgumentOutOfRangeException: Index was out of range.
The packets (and the session listing itself) are written by a
background task, so both can be transiently empty — but
WaitForPacketsAsync indexed sessions[0] unguarded and, on
timeout, returned a session with fewer packets than requested,
which then blew up at the caller's PacketList[0] with a index error.

Fix

In WaitForPacketsAsync (both NetworkObservationTests and its
copy-paste twin in PacketArchiveTest, which had the identical
latent crash):

  • keep polling while the session listing is empty instead of
    indexing into it;
  • throw a descriptive TimeoutException (Timed out after 10 s waiting for N archived packet(s)) when the budget is genuinely
    exhausted, instead of returning a short session.

No production code touched, no behavior change on the success path,
10 s budget unchanged (matches the repo's existing convention).

Tests

  • NetworkObservationTests: 5/5 passed, repeated runs.
  • PacketArchiveTest: 14/14 passed, repeated runs.

WaitForPacketsAsync in NetworkObservationTests and PacketArchiveTest
indexed sessions[0] unguarded and returned a possibly short session,
so a slow background writer task produced a random
ArgumentOutOfRangeException instead of a meaningful result. Both
helpers now keep polling while the session listing (or packet list)
is transiently empty and throw a descriptive TimeoutException only
when the 10 s budget is actually exhausted.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant