mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-26 13:16:24 +00:00
fix(sandbox): surface missing-file AIO reads as actionable errors (#6834)
* fix(sandbox): surface missing-file AIO reads as actionable errors
When read_file targets a nonexistent file, the AIO server returns an error
envelope with data: {}, which the agent-sandbox SDK cannot parse into its
required FileReadResult fields. The resulting pydantic ValidationError text
became the model-visible tool result ("Error: 2 validation errors for
ResponseFileReadResult ..."), so the agent could not tell the file simply
does not exist and kept retrying or "fixing" nonexistent problems.
Catch ValidationError in AioSandbox.read_file and return an explicit
"Error: file not found or unreadable: <path>" instead. The branch is logged
at debug level: a missing file is an expected outcome, since
ReadBeforeWriteMiddleware probes every new-file write before allowing it, and
the previous error-level logging produced noisy gateway logs on that benign
path. Other exceptions keep the error log and the generic error string.
Fixes bytedance/deer-flow#6833
* docs(aio): align regression test reference style
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
This commit is contained in:
parent
a4be043aec
commit
6fd819f0a7
@ -93,3 +93,13 @@ cleanup. Failed discovery registration closes only its own HTTP client.
|
||||
|
||||
Regressions: `test_aio_sandbox_provider.py`, `test_sandbox_quarantine.py`,
|
||||
`test_remote_sandbox_backend.py`, `test_provisioner_request_threading.py`.
|
||||
|
||||
## Read failure contract
|
||||
|
||||
`read_file` returns content or an ``Error: ...`` string, never raises. The
|
||||
server reports missing files as an error envelope with ``data: {}``, which the
|
||||
SDK cannot parse into ``FileReadResult``; the resulting ``ValidationError``
|
||||
becomes ``Error: file not found or unreadable: <path>`` logged at debug
|
||||
(missing files are expected on ReadBeforeWrite probes of new files), never the
|
||||
raw pydantic text. Other exceptions keep the error log.
|
||||
Regressions: `test_aio_sandbox.py`.
|
||||
|
||||
@ -8,6 +8,7 @@ from dataclasses import dataclass, field
|
||||
import httpx
|
||||
from agent_sandbox import Sandbox as AioSandboxClient
|
||||
from agent_sandbox.core.api_error import ApiError
|
||||
from pydantic import ValidationError
|
||||
|
||||
from deerflow.config.paths import VIRTUAL_PATH_PREFIX
|
||||
from deerflow.sandbox.download import DEFAULT_DOWNLOAD_LIMIT as _MAX_DOWNLOAD_SIZE
|
||||
@ -1029,6 +1030,16 @@ class AioSandbox(Sandbox):
|
||||
kwargs["end_line"] = max(end_line, 0)
|
||||
result = self._client.file.read_file(file=path, **kwargs)
|
||||
return result.data.content if result.data else ""
|
||||
except ValidationError as e:
|
||||
# The server reports read failures (e.g. ENOENT) as an error
|
||||
# envelope with ``data: {}``, which cannot satisfy the required
|
||||
# ``content``/``file`` fields of the SDK's FileReadResult model,
|
||||
# so the SDK raises a ValidationError. A missing file is an
|
||||
# expected outcome (ReadBeforeWrite probes every new-file write),
|
||||
# so log quietly and return an explicit, model-actionable result
|
||||
# instead of leaking the pydantic error text.
|
||||
logger.debug("Sandbox read_file validation failure for %r: %s", path, e)
|
||||
return f"Error: file not found or unreadable: {path}"
|
||||
except Exception as e:
|
||||
logger.error(f"Failed to read file in sandbox: {e}")
|
||||
return f"Error: {e}"
|
||||
|
||||
@ -1,5 +1,6 @@
|
||||
"""Tests for AioSandbox concurrent command serialization (#1433)."""
|
||||
|
||||
import logging
|
||||
import os
|
||||
import shlex
|
||||
import subprocess
|
||||
@ -3080,3 +3081,67 @@ def test_list_dir_preserves_trailing_space_in_filename(sandbox):
|
||||
sandbox._client.shell.exec_command = MagicMock(return_value=SimpleNamespace(data=SimpleNamespace(output="/test/notes.txt \n/test/sub\n\n__DF_FIND_STATUS__:0\n", exit_code=0)))
|
||||
|
||||
assert sandbox.list_dir("/test") == ["/test/notes.txt ", "/test/sub"]
|
||||
|
||||
|
||||
def _sdk_file_read_validation_error() -> Exception:
|
||||
"""The exact exception the SDK raises for a server failure envelope.
|
||||
|
||||
The AIO server reports read failures (e.g. ENOENT) as
|
||||
``{"success": false, "message": ..., "data": {}}``; the empty ``data``
|
||||
cannot satisfy the required fields of the SDK's ``FileReadResult`` model,
|
||||
so parsing raises a ``ValidationError`` with two "Field required" errors.
|
||||
Building it from the real SDK models keeps the test faithful to what
|
||||
production raises.
|
||||
"""
|
||||
from agent_sandbox.file.client import ResponseFileReadResult
|
||||
from pydantic import ValidationError
|
||||
|
||||
try:
|
||||
ResponseFileReadResult.model_validate({"success": False, "message": "file not found", "data": {}})
|
||||
except ValidationError as e:
|
||||
return e
|
||||
raise AssertionError("model_validate should fail on data: {}")
|
||||
|
||||
|
||||
def test_read_file_missing_file_returns_actionable_error(sandbox, caplog):
|
||||
"""A server failure envelope must not leak the SDK's pydantic
|
||||
validation-error text to the model. The result must state the file is
|
||||
missing/unreadable so the agent can act on it (create it, pick another
|
||||
path) instead of retrying the same read or "fixing" a nonexistent file."""
|
||||
sandbox._client.file.read_file.side_effect = _sdk_file_read_validation_error()
|
||||
|
||||
with caplog.at_level(logging.DEBUG, logger="deerflow.community.aio_sandbox.aio_sandbox"):
|
||||
result = sandbox.read_file("/workspace/new_file.py")
|
||||
|
||||
assert result == "Error: file not found or unreadable: /workspace/new_file.py"
|
||||
assert "validation errors" not in result
|
||||
|
||||
|
||||
def test_read_file_missing_file_does_not_log_error_noise(sandbox, caplog):
|
||||
"""Missing files are an expected outcome — ReadBeforeWriteMiddleware
|
||||
probes every new-file write — so the validation failure must stay out of
|
||||
the error-level gateway log."""
|
||||
sandbox._client.file.read_file.side_effect = _sdk_file_read_validation_error()
|
||||
|
||||
with caplog.at_level(logging.DEBUG, logger="deerflow.community.aio_sandbox.aio_sandbox"):
|
||||
sandbox.read_file("/workspace/new_file.py")
|
||||
|
||||
assert not [record for record in caplog.records if record.levelno >= logging.ERROR]
|
||||
|
||||
|
||||
def test_read_file_success_returns_content(sandbox):
|
||||
sandbox._client.file.read_file.return_value = SimpleNamespace(data=SimpleNamespace(content="hello\n", file="/workspace/a.txt"))
|
||||
assert sandbox.read_file("/workspace/a.txt") == "hello\n"
|
||||
sandbox._client.file.read_file.assert_called_once_with(file="/workspace/a.txt")
|
||||
|
||||
|
||||
def test_read_file_unexpected_error_still_logged(sandbox, caplog):
|
||||
"""Non-validation failures keep the error log and the generic error
|
||||
string; only the server failure envelope is downgraded to debug."""
|
||||
sandbox._client.file.read_file.side_effect = RuntimeError("connection reset")
|
||||
|
||||
with caplog.at_level(logging.ERROR, logger="deerflow.community.aio_sandbox.aio_sandbox"):
|
||||
result = sandbox.read_file("/workspace/a.txt")
|
||||
|
||||
assert result == "Error: connection reset"
|
||||
assert any(record.levelno >= logging.ERROR for record in caplog.records)
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user