fix: make rollout /run failures recoverable

[yuchenwang3]

Two issues that together turn one resource-server error into a fatal crash:

1. raise_for_status re-raises aiohttp's ClientResponseError unchanged. It holds request_info/history/headers, which are multidict.CIMultiDictProxy objects that don't pickle. When rollout collection runs under Ray, the exception is pickled to cross actors and the run dies with can't pickle CIMultiDictProxy — so any resource-server 5xx takes down the whole job instead of failing a single rollout. Strip the unpicklable fields; keep status/message/response_content.

2. _post_subroutine never retries /run. A transient 5xx (e.g. a momentarily overloaded code-exec server) aborts the batch. Retry a few times on 5xx/connection errors; leave 4xx alone.

How we hit it: under GRPO, a vLLM worker 500s on a NaN logprob (fixed separately in NVIDIA-NeMo/RL#2962). That 500 reaches raise_for_status, and (1) then crashes the whole rollout collection on the unpicklable exception rather than surfacing the underlying error.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[yuchenwang3]

Ping — waiting on CI authorization since Jun 26. Could a maintainer /ok to test and review? This makes rollout /run failures recoverable instead of fatal, which matters for long RL runs where a single transient env error currently kills the job.

[yuchenwang3]

@marta-sd sorry for the direct ping — you've been the main recent committer on rollout_collection.py, so you seemed like the right reviewer. This makes rollout /run failures recoverable instead of fatal — right now a single transient env error kills a long RL run. Waiting on the copy-pr-bot gate since Jun 26 — could you /ok to test and take a look?

[yuchenwang3]

@kajalj22 pinging you as the most active recent merger in this repo — earlier pings didn't find an owner. This makes rollout /run failures recoverable instead of fatal: unpicklable ClientResponseError crossing Ray actors currently kills long RL runs on a single resource-server hiccup. It's been waiting on the copy-pr-bot gate since Jun 26. Could you /ok to test and review, or route it to the right owner?

[marta-sd]

cc @ritaneves for finding a reviewer

[bxyu-nvidia]

Hi @yuchenwang3! For these automatic retries, I think this would just end up with truly failing benchmarks retry longer and fail slower. Typically for these retries we will leave them for the individual servers to handle rather than have a global retry mechnism which may be suboptimal/unintuitive for some scenarios

[yuchenwang3]

Fair point on the retry — that half I'm happy to cut. I'd just want to keep the other half from getting thrown out with it, since the bug that actually kills runs has nothing to do with retry policy: raise_for_status re-raises aiohttp's ClientResponseError with request_info/history/headers still attached, and those are CIMultiDictProxy objects that don't pickle. So the first resource-server 5xx that crosses a Ray actor boundary takes down the entire rollout collection with can't pickle CIMultiDictProxy instead of failing that one rollout — you never even see the underlying error. That's how we hit it: a vLLM worker 500'd on a NaN logprob and the GRPO run died on the pickle error rather than surfacing the 500.

On retries themselves, agreed the individual server is the right owner — a global loop can't know which benchmarks are idempotent or how fast a truly-failing one should give up. So how about I drop the retry loop in _post_subroutine and this PR becomes just the exception-hygiene fix in server_utils.py (strip the unpicklable fields, keep status/message/response_content)? Failures still fail fast, they just propagate as the real error instead of a pickle crash. If that works I'll update the PR.

[ananthsub]

The exception handling change to avoid unpickleable exception content looks good to me. If we can make this PR just that addition without the extra retry loop changes, it's good to merge

[ananthsub]

@yuchenwang3 this change looks good to me. Let's start with getting this merged without the retry-loop addition

[yuchenwang3]

Thanks — #2726 takes the exception-serialization fix forward without the retry loop, with stronger spawn-process coverage, and keeps the co-author credit. Closing this one as superseded.