Skip to content

Commit 86f06ef

Browse files
fix(serverless): keep log capture and shutdown behavior off the existing path
An audit of the most popular worker templates showed this feature reaching workers that never asked for it. Two of them install the SDK unpinned, so they take whatever we publish, and both fill the buffer with operational chatter while returning errors rather than raising, so the capture was pure exposure with no benefit. Meanwhile the workers the feature was built for run their engine as a subprocess, whose output arrives on inherited file descriptors and was never capturable in the first place. Make capture opt-in. RUNPOD_LOG_CAPTURE=auto, the default, installs the proxy only when prestart hooks are registered, so a worker that has not adopted prestart is untouched: streams unwrapped, payload unchanged. `all` and `off` force it either way. Stop truncating what this feature did not add. A handler's error_message and error_traceback are reported in full again; 16KB is well under the size of a real framework traceback, and the job-done body limit was not this change's problem to solve. Only the logs field stays bounded. Restore the in-flight job-take on ordinary shutdown. The platform assigns a job before writing the take response and does not release it when the client disconnects, so aborting a take that already carries one strands that request until the heartbeat sweeper reclaims it, spending its single retry. Prestart failure already had a grace period for this; every other shutdown now gets a short one. Also make the proxy indistinguishable from the stream it wraps: register it on io.TextIOBase so isinstance checks still pass (subclassing would shadow encoding with None), and capture writelines, which bypassed write(). Document what capture cannot see: child processes, log handlers constructed before startup, and plain threads.
1 parent 00b11fd commit 86f06ef

6 files changed

Lines changed: 231 additions & 49 deletions

File tree

‎docs/serverless/worker.md‎

Lines changed: 31 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -94,9 +94,9 @@ when hooks are registered. It never silently skips registered startup work.
9494

9595
## Failure logs
9696

97-
When a handler or a prestart hook fails, the SDK attaches the last 16 KB of
98-
whatever the worker wrote to `stdout`/`stderr` to the failure it returns, under
99-
a `logs` key:
97+
When a prestart hook or a handler fails, the SDK can attach the last 16 KB of
98+
whatever the worker wrote to `stdout`/`stderr` to the failure it reports, under a
99+
`logs` key:
100100

101101
```json
102102
{
@@ -107,20 +107,36 @@ a `logs` key:
107107
}
108108
```
109109

110-
That payload is returned to whoever called the request, so anything the worker
111-
prints can reach the caller. On a private endpoint the caller is you, and the
112-
same output is already in your worker logs. On a public or shared endpoint the
113-
caller is a third party. If your worker prints credentials, connection strings,
114-
or other data you do not want returned, either stop printing them or turn
115-
capture off:
110+
That payload is returned to whoever called the request, so capture is **not on by
111+
default**. `RUNPOD_LOG_CAPTURE` controls it:
116112

117-
```bash
118-
RUNPOD_SKIP_LOG_CAPTURE=true
119-
```
113+
| Value | Behavior |
114+
|-------|----------|
115+
| `auto` (default) | Capture only when prestart hooks are registered. A worker that has not adopted prestart hooks is untouched: streams are not wrapped and no `logs` key is added. |
116+
| `all` | Always capture, including handler failures on workers with no prestart hooks. |
117+
| `off` | Never capture, even with prestart hooks registered. |
118+
119+
Only the `logs` field is bounded. `error_message` and `error_traceback` are
120+
reported in full, exactly as they were before this feature existed.
121+
122+
### What capture can and cannot see
123+
124+
Capture replaces `sys.stdout`/`sys.stderr` at worker startup, so it sees `print`
125+
and direct stream writes made while a hook or handler runs. It does **not** see:
126+
127+
- **Child processes.** A subprocess inherits file descriptors 1 and 2 directly,
128+
below Python. If your worker runs an engine as a subprocess and wants its output
129+
captured, pipe it (`subprocess.Popen(..., stdout=PIPE)`) and re-emit the lines.
130+
- **Log handlers created before startup.** `logging.StreamHandler` resolves
131+
`sys.stderr` once, when it is constructed. A `logging.basicConfig()` at module
132+
import binds the real stream before the SDK starts, so those records bypass
133+
capture. Call `logging.basicConfig()` from inside a prestart hook if you want
134+
its output attached.
135+
- **Plain threads.** Output from a thread your code starts directly is not
136+
captured; `asyncio.to_thread` is.
120137

121-
With capture off, failures still report the exception type, message, and
122-
traceback; only the `logs` key is dropped. Output always continues to reach your
123-
worker logs either way.
138+
In all of these cases output still reaches your worker logs as usual — only the
139+
`logs` field is affected.
124140

125141
## Worker Refresh
126142

‎runpod/serverless/modules/rp_capture.py‎

Lines changed: 44 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,30 @@
66
real stream and a buffer in a contextvar.
77
88
Captured output is attached to failure payloads, which are returned to whoever
9-
called the request. Set `RUNPOD_SKIP_LOG_CAPTURE=true` to keep worker output
10-
inside the worker.
9+
called the request, so capture is not on by default. `RUNPOD_LOG_CAPTURE`
10+
selects when it runs:
11+
12+
auto (default) capture only when prestart hooks are registered, so a worker
13+
that has not adopted prestart behaves exactly as before
14+
all always capture, including handler failures on workers with
15+
no prestart hooks
16+
off never capture
1117
"""
1218

1319
import contextlib
1420
import contextvars
21+
import io
1522
import os
1623
import sys
1724
from collections.abc import Generator
1825

1926
MAX_CAPTURED_CHARS = 16 * 1024
2027

28+
CAPTURE_AUTO = "auto"
29+
CAPTURE_ALL = "all"
30+
CAPTURE_OFF = "off"
31+
_CAPTURE_MODES = (CAPTURE_AUTO, CAPTURE_ALL, CAPTURE_OFF)
32+
2133
# Capture buffer for the current context
2234
_current: "contextvars.ContextVar[_RingBuffer | None]" = contextvars.ContextVar(
2335
"rp_stdio_capture", default=None
@@ -54,6 +66,11 @@ def write(self, text) -> int:
5466
buffer.write(text)
5567
return n
5668

69+
def writelines(self, lines) -> None:
70+
# Not covered by write(); callers that use it would otherwise bypass capture.
71+
for line in lines:
72+
self.write(line)
73+
5774
def flush(self) -> None:
5875
self._real.flush()
5976

@@ -62,15 +79,36 @@ def __getattr__(self, name):
6279
return getattr(self._real, name)
6380

6481

82+
# Registered rather than subclassed: inheriting io.TextIOBase would shadow
83+
# encoding/errors/newlines with None instead of delegating to the real stream.
84+
# Registration is what keeps `isinstance(sys.stdout, io.TextIOBase)` true for
85+
# libraries that type-check the stream.
86+
io.TextIOBase.register(_TeeProxy)
87+
88+
89+
def capture_mode() -> str:
90+
"""Resolve `RUNPOD_LOG_CAPTURE`, falling back to `auto` on anything unknown."""
91+
mode = os.environ.get("RUNPOD_LOG_CAPTURE", CAPTURE_AUTO).strip().lower()
92+
return mode if mode in _CAPTURE_MODES else CAPTURE_AUTO
93+
94+
6595
def install() -> None:
66-
"""Install the tee proxy on stdout/stderr. Idempotent.
96+
"""Install the tee proxy on stdout/stderr, if this worker wants capture.
6797
68-
Skipped when `RUNPOD_SKIP_LOG_CAPTURE=true`. Without the proxy nothing ever
69-
reaches a capture buffer, so no failure payload carries worker output.
98+
Idempotent. Without the proxy nothing ever reaches a capture buffer, so no
99+
failure payload carries worker output and the streams are left untouched.
70100
"""
71-
if os.environ.get("RUNPOD_SKIP_LOG_CAPTURE", "").lower() == "true":
101+
mode = capture_mode()
102+
if mode == CAPTURE_OFF:
72103
return
73104

105+
if mode == CAPTURE_AUTO:
106+
# Local import: rp_prestart imports this module.
107+
from .rp_prestart import has_prestart_hooks
108+
109+
if not has_prestart_hooks():
110+
return
111+
74112
if not isinstance(sys.stdout, _TeeProxy):
75113
sys.stdout = _TeeProxy(sys.stdout)
76114
if not isinstance(sys.stderr, _TeeProxy):

‎runpod/serverless/modules/rp_job.py‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515

1616
from ...version import __version__ as runpod_version
1717
from ..utils import rp_debugger
18-
from .rp_capture import capture, clip
18+
from .rp_capture import capture
1919
from .rp_handler import is_generator
2020
from .rp_http import send_result, stream_result
2121
from .rp_tips import check_return_size
@@ -290,8 +290,8 @@ async def run_job(handler: Callable, job: Dict[str, Any]) -> Dict[str, Any]:
290290
captured_logs = cap.getvalue()
291291
error_info = {
292292
"error_type": str(type(err)),
293-
"error_message": clip(str(err)),
294-
"error_traceback": clip(traceback.format_exc()),
293+
"error_message": str(err),
294+
"error_traceback": traceback.format_exc(),
295295
"hostname": os.environ.get("RUNPOD_POD_HOSTNAME", "unknown"),
296296
"worker_id": os.environ.get("RUNPOD_POD_ID", "unknown"),
297297
"runpod_version": runpod_version,
@@ -339,6 +339,6 @@ async def run_job_generator(
339339
error = f"handler: {str(err)} \ntraceback: {traceback.format_exc()}"
340340
if captured_logs:
341341
error += f"\nlogs:\n{captured_logs}"
342-
yield {"error": clip(error)}
342+
yield {"error": error}
343343
finally:
344344
log.info("Finished running generator.", job["id"])

‎runpod/serverless/modules/rp_scale.py‎

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,9 @@ def __init__(self, config: dict[str, Any]):
7070
self.jobs_fetcher = get_job
7171
self.jobs_fetcher_timeout = 90
7272
self.prestart_claim_timeout = 10
73+
# Only needs to outlast a response already in transit, so it is far
74+
# shorter than prestart_claim_timeout, which waits for a job to appear.
75+
self.shutdown_take_grace = 2
7376
self.jobs_handler = handle_job
7477

7578
if concurrency_modifier := config.get("concurrency_modifier"):
@@ -218,8 +221,15 @@ def current_occupancy(self) -> int:
218221
return current_progress_count + current_queue_count
219222

220223
async def _fetch_jobs_until_stopped(self, session: ClientSession, jobs_needed: int):
221-
"""After prestart failure, briefly keep an in-flight job-take alive so
222-
its request can receive the error."""
224+
"""Run one blocking job-take without letting it delay shutdown.
225+
226+
Shutdown cannot simply abandon the take. The platform assigns a job
227+
before it writes the response and does not release it when the client
228+
disconnects, so aborting a take that already carries a job strands that
229+
request until the worker-heartbeat sweeper reclaims it, which also
230+
spends the job's one retry. Give the response a bounded window to land:
231+
long enough to report a prestart failure against, short otherwise.
232+
"""
223233
fetch_task = asyncio.create_task(self.jobs_fetcher(session, jobs_needed))
224234
shutdown_task = asyncio.create_task(self._shutdown_event.wait())
225235
try:
@@ -232,14 +242,13 @@ async def _fetch_jobs_until_stopped(self, session: ClientSession, jobs_needed: i
232242
return await fetch_task
233243

234244
if shutdown_task in done:
235-
if self._prestart_error is None:
236-
return None
237-
238-
self._failure_take_done = True
245+
if self._prestart_error is not None:
246+
self._failure_take_done = True
247+
grace = self.prestart_claim_timeout
248+
else:
249+
grace = self.shutdown_take_grace
239250
try:
240-
return await asyncio.wait_for(
241-
fetch_task, timeout=self.prestart_claim_timeout
242-
)
251+
return await asyncio.wait_for(fetch_task, timeout=grace)
243252
except asyncio.TimeoutError:
244253
return None
245254

‎tests/test_serverless/test_capture.py‎

Lines changed: 82 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,10 @@
1313

1414
from runpod.serverless.modules import rp_capture
1515
from runpod.serverless.modules.rp_job import run_job, run_job_generator
16+
from runpod.serverless.modules.rp_prestart import (
17+
clear_prestart_hooks,
18+
register_prestart_hook,
19+
)
1620

1721

1822
def _run(coro):
@@ -65,27 +69,57 @@ def test_install_is_idempotent(self):
6569
assert sys.stderr is installed_stderr
6670

6771

68-
class TestCaptureOptOut(unittest.TestCase):
69-
"""`RUNPOD_SKIP_LOG_CAPTURE` keeps worker output out of failure payloads."""
72+
class TestCaptureIsOptIn(unittest.TestCase):
73+
"""A worker that has not adopted prestart hooks must see no change at all:
74+
streams untouched and no logs field in the failure it reports."""
7075

71-
def test_install_is_skipped(self):
72-
real_stdout, real_stderr = sys.stdout, sys.stderr
76+
def setUp(self):
77+
clear_prestart_hooks()
78+
self.addCleanup(clear_prestart_hooks)
79+
80+
def _install_with(self, env) -> bool:
81+
"""Install under `env` and report whether the proxy was applied."""
82+
real_stdout, real_stderr = sys.__stdout__, sys.__stderr__
7383
with (
74-
patch.dict(os.environ, {"RUNPOD_SKIP_LOG_CAPTURE": "true"}),
84+
patch.dict(os.environ, env, clear=False),
7585
patch.object(sys, "stdout", real_stdout),
7686
patch.object(sys, "stderr", real_stderr),
7787
):
7888
rp_capture.install()
89+
return isinstance(sys.stdout, rp_capture._TeeProxy) and isinstance(
90+
sys.stderr, rp_capture._TeeProxy
91+
)
92+
93+
def test_auto_skips_install_without_hooks(self):
94+
assert self._install_with({"RUNPOD_LOG_CAPTURE": "auto"}) is False
95+
96+
def test_auto_is_the_default(self):
97+
env = {k: v for k, v in os.environ.items() if k != "RUNPOD_LOG_CAPTURE"}
98+
with patch.dict(os.environ, env, clear=True):
99+
assert rp_capture.capture_mode() == rp_capture.CAPTURE_AUTO
100+
assert self._install_with({}) is False
101+
102+
def test_auto_installs_once_a_hook_is_registered(self):
103+
register_prestart_hook(lambda: None)
104+
assert self._install_with({"RUNPOD_LOG_CAPTURE": "auto"}) is True
105+
106+
def test_all_installs_without_hooks(self):
107+
assert self._install_with({"RUNPOD_LOG_CAPTURE": "all"}) is True
108+
109+
def test_off_skips_install_even_with_hooks(self):
110+
register_prestart_hook(lambda: None)
111+
assert self._install_with({"RUNPOD_LOG_CAPTURE": "off"}) is False
79112

80-
assert not isinstance(sys.stdout, rp_capture._TeeProxy)
81-
assert not isinstance(sys.stderr, rp_capture._TeeProxy)
113+
def test_unknown_mode_falls_back_to_auto(self):
114+
with patch.dict(os.environ, {"RUNPOD_LOG_CAPTURE": "yes-please"}):
115+
assert rp_capture.capture_mode() == rp_capture.CAPTURE_AUTO
82116

83-
def test_error_omits_logs_without_the_proxy(self):
117+
def test_error_payload_is_unchanged_without_capture(self):
84118
def handler(_job):
85119
print("HF_TOKEN=hf_secret")
86120
raise RuntimeError("kernel panic")
87121

88-
with patch.dict(os.environ, {"RUNPOD_SKIP_LOG_CAPTURE": "true"}):
122+
with patch.dict(os.environ, {"RUNPOD_LOG_CAPTURE": "off"}):
89123
rp_capture.install()
90124
result = _run(run_job(handler, {"id": "j3"}))
91125

@@ -141,8 +175,8 @@ def test_handler_success_has_no_error(self):
141175

142176

143177
class TestErrorFieldBounding(unittest.TestCase):
144-
"""Error strings shipped back to the platform are bounded so a huge message/log can't
145-
blow past the job-done body limit."""
178+
"""Only the log tail this SDK adds is bounded. A handler's own error message and
179+
traceback are reported exactly as before, so existing workers see no truncation."""
146180

147181
def test_clip_keeps_head_and_tail(self):
148182
text = "A" * 100 + "B" * 100
@@ -155,17 +189,34 @@ def test_clip_keeps_head_and_tail(self):
155189
def test_clip_passthrough_when_small(self):
156190
assert rp_capture.clip("short", limit=100) == "short"
157191

158-
def test_run_job_bounds_error_message(self):
192+
def test_run_job_reports_a_huge_handler_error_untruncated(self):
159193
huge = "x" * (rp_capture.MAX_CAPTURED_CHARS * 3)
160194

161195
def handler(_job):
162196
raise ValueError(huge)
163197

164198
result = _run(run_job(handler, {"id": "big"}))
165199
error = json.loads(result["error"])
166-
assert len(error["error_message"]) <= rp_capture.MAX_CAPTURED_CHARS + 100
200+
assert error["error_message"] == huge
201+
assert "truncated" not in error["error_traceback"]
167202

168-
def test_run_job_generator_bounds_combined_error(self):
203+
def test_run_job_bounds_only_the_captured_logs(self):
204+
def handler(_job):
205+
print("y" * (rp_capture.MAX_CAPTURED_CHARS * 3))
206+
raise ValueError("boom")
207+
208+
real = io.StringIO()
209+
with (
210+
patch.object(sys, "stdout", rp_capture._TeeProxy(real)),
211+
patch.object(sys, "stderr", rp_capture._TeeProxy(real)),
212+
):
213+
result = _run(run_job(handler, {"id": "big-logs"}))
214+
215+
error = json.loads(result["error"])
216+
assert error["error_message"] == "boom"
217+
assert len(error["logs"]) <= rp_capture.MAX_CAPTURED_CHARS
218+
219+
def test_run_job_generator_bounds_only_the_captured_logs(self):
169220
huge = "x" * (rp_capture.MAX_CAPTURED_CHARS * 3)
170221

171222
def handler(_job):
@@ -180,7 +231,23 @@ def handler(_job):
180231
):
181232
result = _run_gen(run_job_generator(handler, {"id": "big-generator"}))
182233

183-
assert len(result[0]["error"]) <= rp_capture.MAX_CAPTURED_CHARS + 100
234+
# The handler's own error survives whole; only the appended log tail is capped.
235+
assert huge in result[0]["error"]
236+
logs = result[0]["error"].split("\nlogs:\n", 1)[1]
237+
assert len(logs) <= rp_capture.MAX_CAPTURED_CHARS
238+
239+
def test_proxy_is_indistinguishable_from_a_text_stream(self):
240+
proxy = rp_capture._TeeProxy(io.StringIO())
241+
assert isinstance(proxy, io.TextIOBase)
242+
assert isinstance(proxy, io.IOBase)
243+
244+
def test_writelines_is_captured(self):
245+
real = io.StringIO()
246+
proxy = rp_capture._TeeProxy(real)
247+
with patch.object(sys, "stdout", proxy), rp_capture.capture() as buf:
248+
sys.stdout.writelines(["a\n", "b\n"])
249+
assert buf.getvalue() == "a\nb\n"
250+
assert real.getvalue() == "a\nb\n"
184251

185252

186253
if __name__ == "__main__":

0 commit comments

Comments
 (0)