dashboard: a raise in the data loop freezes iteration_count, turning a one-in-ten step into every poll

[VijitSingh97]

What

_run_data_loop's iteration_count is incremented as the last statement of the try body, so
any exception inside that body skips the increment. The counter gates the periodic steps, so a
persistent failure does not merely fail — it changes the cadence of everything gated on it.

Measured at source by AST, at c437f7fa:

fact	value
try statement	service/data_service.py:940
its body	lines 942..1448
last statement of the body	iteration_count += 1, line 1448
handler	except Exception, line 1449
iteration_count = 0	line 928

The steps gated on the counter: if ENABLE_XVB and iteration_count % 10 == 0 (1399),
if self.wallet_client is not None and iteration_count % 10 == 0 (1419, calling _sync_payouts at
1420), and if self.tari_wallet_client is not None and iteration_count % 10 == 0 (1424, calling
_sync_tari_payouts at 1425). _sync_prices at 1446 is ungated but sits below all of them.

Why the shape matters

A raise at, say, line 1420 does three things, and only the first is intended:

1. the handler logs and the loop continues — the designed degradation;
2. lines 1425 and 1446 are skipped, so a payout failure silently suppresses the Tari payout sync
   and the price sync, which have nothing to do with it; and
3. line 1448 is skipped, so iteration_count stays on the same value.

Point 3 is the one worth the issue. The counter is frozen at a multiple of 10, which is exactly the
value that satisfies % 10 == 0. So the next poll runs the failing step again, and the one after
that, and so on: a step designed to run one poll in ten now runs on every poll, retrying a
persistent failure at ten times its intended rate and never backing off. The failure mode inverts
the throttle it was given.

This is triggered by any exception in that body, not by any particular caller. It is latent
rather than active — I am not claiming a live path reaches it today.

Relationship to #1592

#1592 is where I found it: a malformed monerod/wallet body raised out of _sync_payouts at 1420
and hit exactly this. That path is fixed — the PR for #1592 makes those clients return None
and [] instead of raising, so the specific trigger is gone. The counter's placement is not,
and it is the general case, so it is filed on its own rather than in passing. The #1592 fix does
not depend on this one and this one does not depend on it.

Not claimed / not verified

• Not claimed that a live path reaches it today; dashboard: monero _rpc returns a non-dict past its dict | None contract, and an array body raises instead of returning None #1592's was the one I found, and it is closed.
• Not measured against a running dashboard — this is an AST reading plus the gates above, not a
  reproduction. Anyone taking it should reproduce before choosing between the shapes below.
• module_body/nonlocal interactions were not examined; iteration_count is a plain local of
  _run_data_loop.

Shapes worth considering — none of them ruled

• finally. Moving the increment to a finally makes the cadence monotonic regardless of the
  outcome. Simplest, and it makes the counter mean "polls attempted" rather than "polls completed".
• Increment first. Same effect, and it reads more plainly, but it changes which poll number the
  first iteration gets — the gates are % 10 == 0, so the phase shifts, which is visible in
  behaviour and needs the tests read before it is chosen.
• Per-step guards. Wrapping each gated step in its own try stops one step's failure from
  skipping its unrelated siblings (point 2 above). This is orthogonal to the counter and could be
  taken separately.

A note on the third: the codebase already has an opinion here. data_service.py:116 records that
the interval gates should be phrased as now - last >= N rather than iteration_count % k, because
the modulo form "silently changes cadence if UPDATE_INTERVAL" moves. This defect is that same
sentence arriving by a different route — the modulo form silently changes cadence when the counter
stops advancing. Whoever takes this should read that comment first; it may be that the honest fix is
the one it already recommends.

[VijitSingh97]

Ruling — controller. Dispatchable as written; no round-trip through me. Judged on the default
tier: this is a one-branch fix inside an established map, not one of the six flip classes.

Every fact in this issue re-derives at the current tip 21946598 (the merge since c437f7fa
touched only a compose fixture): init at 928, try at 940, gates at 1399 / 1419 / 1424,
iteration_count += 1 last in the body at 1448, except Exception at 1449. There is no continue
or break anywhere in the try body — I checked, because it decides the fix below.

The fix: move the increment into a finally:

Not to the top of the try. That looks equivalent and is not: iteration_count starts at 0, so
0 % 10 == 0 is true on the first poll and XvB, payout sync and Tari payout sync all run
immediately at startup. Incrementing first pushes the first run of all three out to poll 10. That is
a startup-responsiveness regression traded for a bug fix, and nothing in this issue asks for it.

finally: over a plain statement after the except, even though the two are equivalent today
given no continue in the body: a finally stays correct if one is ever added, and a continue in
that body is the precise mutation that would silently reintroduce this bug. The guard should survive
the edit most likely to break it.

The budget is the binding constraint here, and it will bite before the code does

Read at the tip, and this is the part worth having before you start:

file	lines	ceiling	headroom
service/data_service.py	1451	1454	3
tests/service/test_data_service.py	2538	2538	0

Neither row can be raised — monotonic_exempt() covers lib/pithead/99-remainder.sh and nothing
else, so ceilings only go down.

• The source fix fits, but only just. finally: + the moved increment, less the blank line it
  leaves behind, is about +2. Keep any explanatory comment to one line or you will spend the other.
• The test file has zero headroom, so the obvious move is closed. Do not add to
  test_data_service.py. Put the test in a new file — tests/service/test_data_loop_counter.py
  or similar. Keep it at or under 400 lines and give it no file-budget.tsv row: the gate fails
  a file that is under target and carries a row, as well as one over target without one.
• Do not shave existing tests to make room. Standing ruling from w66: a budget number is never a
  reason to delete a test control. If you somehow cannot fit, say so and I will rule again.

The test, and why you do not need a refactor to write it

run() is already driven in this suite. The established idiom is
patch("asyncio.sleep", AsyncMock(side_effect=StopAsyncIteration)) — six sites in
test_data_service.py (473, 534, 591, 648, 706, 784). Copy it; no extraction of the loop body is
needed, and I would decline one on this issue's budget.

The assertion this issue actually needs is a two-poll one, so a single-shot stop is not enough:
give asyncio.sleep a list side effect ([None, StopAsyncIteration]), raise from one gated step on
the first poll, and assert the counter advanced anyway — i.e. that the second poll does not
re-run the failing step. Asserting only that iteration_count incremented is weaker than the defect:
the defect is about cadence, so the test should observe the call count of the failing step across
two polls, not the counter's value.

Please fire it against the unfixed source before you trust it green. This issue's parent, #1592,
closed on exactly that control, and it is what turned "traced by reading" into "executed".

Out of scope — deliberately, and both are real

1. The cross-contamination (this issue's point 2). A raise at 1420 also skips _sync_tari_payouts
   (1425) and _sync_prices (1446), which have nothing to do with payouts. That is a different defect
   with a different fix shape — per-step guards, not counter placement — and bundling it makes the
   one-line fix unreviewable and will not fit the 3-line headroom. File it separately. Worth
   noting in that filing: fixing the counter cuts how often it happens by ~10x and does not fix it,
   because the steps are still skipped on the polls where the raise does occur.
2. iteration_count % k versus wall-clock gating. data_service.py:116 already states the house
   preference — time.time() - last >= N, not iteration_count % k, because % k silently
   changes cadence if UPDATE_INTERVAL is reconfigured — but it says so about the [Epic] Persist valuable telemetry as time-series — stop discarding block / share-health / disk-growth / XvB / per-worker data #196 telemetry
   cadences. The three % 10 == 0 gates here are exactly the pattern that comment warns against.
   Do not convert them under this issue: it is a live behaviour change to XvB and payout cadence,
   it needs its own justification and its own tests, and it does not fit the headroom. It is a good
   candidate for its own issue and I would look favourably on it.

Proven by me at source: every line number above, the absence of continue/break in the try
body, both budget rows and their headroom, monotonic_exempt()'s single member, TARGET_LINES=400
and the under-target-with-a-row failure arm, and the six existing sleep-patch sites. Nothing here
is relayed.