Skip to content

Commit 8b959b3

Browse files
committed
fix(sandbox): use sandlock's exit_code == -1 sentinel for timeout
Sandbox.run() does not populate result.error on timeout — only the Pipeline path does. String-matching on result.error was therefore unreliable for the common single-sandbox case: a real timeout from Sandbox.run() returns success=False, exit_code=-1, empty stderr, and error=None, which my previous logic mis-classified as FAILED. Switch to the structural signal: sandlock's ExitStatus::Timeout is exposed as exit_code == -1 (see sandlock's _sdk.py around line 1475). This matches how sandlock itself detects pipeline timeouts and works uniformly across Sandbox.run() and any future execution paths. Verified end-to-end with a real forced timeout against real sandlock: status: SandboxStatus.TIMEOUT exit: -1 error: Execution timed out after 1s Test updated to match: mock_timeout_result.exit_code = -1 and error = None (reflecting actual Sandbox.run() behavior).
1 parent 4847b13 commit 8b959b3

2 files changed

Lines changed: 11 additions & 12 deletions

File tree

src/praisonai/praisonai/sandbox/sandlock.py

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -290,21 +290,19 @@ def _run() -> Any:
290290
stdout = self._decode(result.stdout)
291291
stderr = self._decode(result.stderr)
292292

293-
# sandlock surfaces timeouts via result.error containing "timed out".
294-
# This is authoritative — wall-clock guesses are unreliable because a
295-
# process can legitimately finish just under the limit.
296-
err_text = (result.error or "") if not result.success else ""
297-
is_timeout = "timed out" in err_text.lower() or "timeout" in err_text.lower()
298-
293+
# sandlock uses exit_code == -1 as the ExitStatus::Timeout sentinel
294+
# (see sandlock's python/src/sandlock/_sdk.py). This is a
295+
# structural signal — Sandbox.run() doesn't populate result.error
296+
# for timeouts, so string-matching on it is unreliable.
299297
if result.success:
300298
status = SandboxStatus.COMPLETED
301299
error = None
302-
elif is_timeout:
300+
elif result.exit_code == -1:
303301
status = SandboxStatus.TIMEOUT
304302
error = f"Execution timed out after {limits.timeout_seconds}s"
305303
else:
306304
status = SandboxStatus.FAILED
307-
error = f"Execution failed with exit code {result.exit_code}: {stderr or err_text}"
305+
error = f"Execution failed with exit code {result.exit_code}: {stderr}"
308306

309307
return SandboxResult(
310308
execution_id=execution_id,

src/praisonai/tests/unit/sandbox/test_sandlock_sandbox.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ async def test_sandlock_execution_success(self):
125125

126126
@pytest.mark.asyncio
127127
async def test_sandlock_execution_timeout(self):
128-
"""Timeout is detected via result.error, not wall-clock heuristics."""
128+
"""Timeout is detected via exit_code == -1 (ExitStatus::Timeout)."""
129129
mock_sandlock = Mock()
130130
mock_sandlock.Policy.return_value = Mock()
131131
mock_sandlock.Sandbox.return_value = Mock()
@@ -135,11 +135,12 @@ async def test_sandlock_execution_timeout(self):
135135

136136
mock_timeout_result = Mock()
137137
mock_timeout_result.success = False
138-
mock_timeout_result.exit_code = 124
138+
# sandlock's timeout sentinel — Sandbox.run() does not populate
139+
# result.error on timeout, so we rely on the exit_code instead.
140+
mock_timeout_result.exit_code = -1
139141
mock_timeout_result.stdout = b""
140142
mock_timeout_result.stderr = b""
141-
# sandlock surfaces timeout via the error field.
142-
mock_timeout_result.error = "process timed out after 10s"
143+
mock_timeout_result.error = None
143144

144145
with patch("asyncio.get_running_loop") as mock_loop:
145146
mock_loop.return_value.run_in_executor = AsyncMock(

0 commit comments

Comments
 (0)