Skip to content

Retry completion responses missing 'choices' instead of crashing the actor - #148

Open
jlamypoirier wants to merge 1 commit into
mainfrom
jlp_actor_retry_missing_choices
Open

Retry completion responses missing 'choices' instead of crashing the actor#148
jlamypoirier wants to merge 1 commit into
mainfrom
jlp_actor_retry_missing_choices

Conversation

@jlamypoirier

@jlamypoirier jlamypoirier commented Jul 2, 2026

Copy link
Copy Markdown

Authored by Claude Opus 4.8 (1M context) via Claude Code.

Problem

The actor can die mid-run with:

File "pipelinerl/async_llm.py", line 173, in llm_async_generate
    content = response_data["choices"][0]["message"]["content"]
KeyError: 'choices'

A single response lacking choices crashes the whole rollout loop.

Root cause

llm_async_generate fetches the completion in a retry loop. _is_retryable_abort_response already inspects choices but returns False for a missing one (correctly — a missing choices is not an abort), the loop breaks, and the post-loop parse dereferences response_data["choices"][0]... unguarded. When the body lacks choices (e.g. an HTTP 200 carrying an error object, or an empty payload) this raises KeyError; the surrounding try/except only logs and re-raises, and the actor retries RetryableAbortedCompletionError but not a bare KeyError, so it propagates and kills the actor.

Fix

After the loop, if response_data has no choices, raise RetryableAbortedCompletionError so the response is retried like other transient vLLM failures.

Scope — what this does and does NOT cover

  • Covers: a transient bad response where the vLLM server stays alive (a one-off malformed / error payload). The actor retries and the run continues.
  • Does NOT cover: a permanent EngineCore death. If the engine dies, its run_vllm1 process exits non-zero, and the launcher's monitor is fail-fastlaunch.py tears down the entire run when any child exits non-zero (gently_stop_all_processes(); sys.exit(1); comment: "if just one dies non-zero, stop all"). So a dead engine ends the run regardless of this actor guard. Surviving that needs launcher-level resilience (restart the dead inference server + route the actor around it), which is out of scope here.
  • This change also does not diagnose why a server returns a body without choices (a server-side error / OOM / EngineCore death are all plausible; in the case that motivated this, the engine had died and was returning a 200 InternalServerError body).

🤖 Generated with Claude Code

…actor

A malformed or error response (an HTTP 200 carrying an error body, or an empty
payload) can lack 'choices'. llm_async_generate parsed it unguarded, raising
KeyError, which the actor does not catch — so a single bad vLLM response killed
the entire rollout loop mid-run. Raise RetryableAbortedCompletionError instead,
so it is retried like other transient vLLM failures.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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