Skip to content

Commit 998a999

Browse files
committed
fix: stream errors to client before breaking generator loop (#397)
Since v1.7.7 (PR #384), when a generator handler yields an error the code breaks out of the streaming loop before calling stream_result(), so the error is never delivered to clients listening on /stream. The error is saved for /status but the stream goes silent. Additionally, the error extraction stripped all extra fields (context, refresh_worker, etc.) by replacing the dict with {"error": msg}. - Call stream_result() with the error BEFORE breaking out of the loop - Preserve extra fields from the error dict alongside the error message - Add tests for both streaming delivery and field preservation Closes #397
1 parent 5f5831b commit 998a999

2 files changed

Lines changed: 78 additions & 1 deletion

File tree

‎runpod/serverless/modules/rp_job.py‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,9 +204,18 @@ async def handle_job(session: ClientSession, config: Dict[str, Any], job) -> dic
204204

205205
if isinstance(stream_output.get("output"), dict):
206206
if stream_output["output"].get("error"):
207-
stream_output = {"error": str(stream_output["output"]["error"])}
207+
error_fields = {
208+
k: v
209+
for k, v in stream_output["output"].items()
210+
if k != "error"
211+
}
212+
stream_output = {
213+
"error": str(stream_output["output"]["error"]),
214+
**error_fields,
215+
}
208216

209217
if stream_output.get("error"):
218+
await stream_result(session, stream_output, job)
210219
job_result = stream_output
211220
break
212221

‎tests/test_serverless/test_modules/test_job.py‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -440,3 +440,71 @@ async def test_run_job_generator_exception(self):
440440
assert mock_log.info.call_count == 1
441441
mock_log.info.assert_called_with("Finished running generator.", "123")
442442

443+
444+
class TestHandleJobStreamError(IsolatedAsyncioTestCase):
445+
"""Tests that generator errors are streamed to the client. (Fixes #397)"""
446+
447+
@staticmethod
448+
async def _noop(*args, **kwargs):
449+
pass
450+
451+
async def test_error_is_streamed_before_break(self):
452+
"""When a generator yields an error, stream_result must be called with it."""
453+
454+
def error_gen(job):
455+
yield "chunk_1"
456+
yield {"error": "GPU OOM"}
457+
458+
config = {"handler": error_gen}
459+
job = {"id": "test-123"}
460+
session = Mock()
461+
462+
stream_calls = []
463+
464+
async def capture_stream(*args, **kwargs):
465+
stream_calls.append(args)
466+
467+
with patch(
468+
"runpod.serverless.modules.rp_job.stream_result", side_effect=capture_stream,
469+
), patch(
470+
"runpod.serverless.modules.rp_job.send_result", side_effect=self._noop,
471+
), patch(
472+
"runpod.serverless.modules.rp_job.log", new_callable=Mock
473+
):
474+
await rp_job.handle_job(session, config, job)
475+
476+
# stream_result should be called twice: once for "chunk_1", once for the error
477+
assert len(stream_calls) == 2
478+
streamed_payload = stream_calls[1][1] # second call, second positional arg
479+
assert "error" in streamed_payload
480+
481+
async def test_nested_error_preserves_extra_fields(self):
482+
"""Extra fields like refresh_worker should survive error extraction."""
483+
484+
def error_gen(job):
485+
yield {"error": "bad things", "context": "step 47", "refresh_worker": True}
486+
487+
config = {"handler": error_gen}
488+
job = {"id": "test-456"}
489+
session = Mock()
490+
491+
stream_calls = []
492+
493+
async def capture_stream(*args, **kwargs):
494+
stream_calls.append(args)
495+
496+
with patch(
497+
"runpod.serverless.modules.rp_job.stream_result", side_effect=capture_stream,
498+
), patch(
499+
"runpod.serverless.modules.rp_job.send_result", side_effect=self._noop,
500+
), patch(
501+
"runpod.serverless.modules.rp_job.log", new_callable=Mock
502+
):
503+
await rp_job.handle_job(session, config, job)
504+
505+
assert len(stream_calls) == 1
506+
streamed_payload = stream_calls[0][1]
507+
assert streamed_payload["error"] == "bad things"
508+
assert streamed_payload["context"] == "step 47"
509+
assert streamed_payload["refresh_worker"] is True
510+

0 commit comments

Comments
 (0)