credit pipeline: surface mid-run issuance-task exceptions instead of swallowing them (follow-up to #1039)

[ajcasagrande]

Summary

Follow-up to #1039. That PR closed the startup instance of a broader bug: exceptions raised inside the fire-and-forget execute_async(strategy.execute_phase()) task in the timing/credit pipeline are never retrieved, so a failure there silently stalls the run instead of failing it.

#1039 added a per-phase StickyCreditRouter.wait_for_workers() barrier so the first credit is only issued after a worker has registered — closing the "no worker registered yet" deadlock. But the general swallowed-exception class remains.

The general problem

send_credit() stays fail-fast for the real no-workers-mid-run case, and other exceptions can still arise inside the issuance task. Because the task is fire-and-forget and its result is never awaited/observed, any such exception is swallowed:

• all_credits_sent_event (and/or all_credits_returned_event) never gets set.
• _wait_for_sending_complete() awaits forever → the run hangs with no error surfaced.

Known residual triggers (called out in #1039's description and review):

1. The last worker unregisters after the wait_for_workers barrier returns but before the first send_credit() — send_credit() raises "No workers available", which is swallowed.
2. Any other exception inside strategy.execute_phase() after issuance begins.

Expected behavior

The issuance task's exception should be observed and routed to the phase-failure path (the same mechanism _on_phase_orchestrator_done / _publish_phase_failure uses for phase-setup failures), so the run exits non-zero with a clear error instead of hanging.

Options to consider:

• Attach a done-callback to the execute_async(strategy.execute_phase()) task that inspects task.exception() and drives the phase-failure path.
• Or set the completion events in a finally/error path so _wait_for_sending_complete() can't await forever, and record the error.

Acceptance

• An exception raised inside the issuance task fails the run (non-zero exit, BaseServiceErrorMessage recorded) rather than hanging.
• Regression test: inject a send_credit/execute_phase failure mid-run and assert the run fails fast with the error surfaced (no hang).

Context

• PR fix: gate phase credit issuance on first-worker readiness (startup race) #1039 (startup case) — merged/approved.
• Relevant code: src/aiperf/timing/manager.py (_on_phase_orchestrator_done), src/aiperf/timing/phase/runner.py (_run_strategy, execute_async(strategy.execute_phase())), src/aiperf/credit/sticky_router.py (send_credit).

[ajcasagrande]

Independently flagged on the PR by dynamo-ops — with a sharper fix direction

The review bot dynamo-ops raised this exact gap inline on #1039 (on the wait_for_workers await): #1039 (comment)

Because wait_for_workers() is explicitly best-effort, a worker that unregisters after this await can still make the first send_credit() raise inside _execution_task, and request-count phases then wait forever on all_credits_sent_event. Fix: make the sending-complete wait observe _execution_task completion and re-raise non-cancellation exceptions instead of waiting only on the progress event.

Two refinements to the issue based on verifying this against the code:

Symptom scope: request-count phases specifically

PhaseRunner._wait_for_sending_complete waits with timeout = self._lifecycle.time_left_in_seconds(). So duration-based phases are already bounded — on a swallowed issuance exception they time out and mark sending complete. The unbounded forever-hang is specific to request-count phases, where there is no meaningful time budget to fall back on.

Preferred fix location: race the wait against _execution_task

_wait_for_sending_complete already passes task_to_cancel=self._execution_task into _wait_for_event_with_timeout, but that only cancels the task on timeout — it never observes the task's exception. The cleaner fix (per dynamo-ops) is to have the sending-complete wait race all_credits_sent_event.wait() against self._execution_task completion: if the task finishes first with a non-CancelledError exception, re-raise it (routing through run()'s existing except → phase-failure path) instead of continuing to wait on the event.

This is more targeted than the done-callback approach sketched above and keeps the failure routing consistent with the startup-timeout path that #1039 added.

Acceptance (updated)

• A send_credit/execute_phase exception raised mid-run on a request-count phase fails the run fast (non-zero exit, error surfaced) rather than hanging.
• Regression test mirrors fix: gate phase credit issuance on first-worker readiness (startup race) #1039's real-router style: register a worker, let the barrier pass, then drop the worker / inject an exception before/at first send_credit, and assert the phase fails (no hang) for a request-count config.