fix(read_file): surface document extraction failures instead of the generic binary-file error
When extraction of a binary document format (.pdf, .docx, .xlsx, Office, EPUB…) fails for a specific reason — the anydoc size cap, an encrypted or malformed file — read_file previously swallowed the ExtractionError at debug level and fell through to the generic 'Cannot read binary file' guard, so the agent never saw the actionable reason (e.g. 'Document too large to convert (N bytes, limit is 52,428,800)'). read_file now returns the specific extraction failure for binary document formats. Fallthrough behavior is preserved where a raw read is still useful: .ipynb (plain JSON) and converter-unavailable PDFs keep their historical raw-read path, and the 'Unsupported document type' shape (no extra information) keeps the generic guard. Follow-up to #80004, where the size-cap message was being generated but never reached the agent.
This commit is contained in:
parent
cd9fbf9f19
commit
cbb8cee47d
|
|
@ -477,14 +477,64 @@ class TestReadFileToolIntegration(unittest.TestCase):
|
|||
self.assertIn("print(1)", res["content"])
|
||||
|
||||
|
||||
def test_corrupt_docx_falls_through_to_binary_guard(self):
|
||||
def test_corrupt_docx_surfaces_extraction_error(self):
|
||||
p = os.path.join(self.tmp, "bad.docx")
|
||||
with open(p, "wb") as fh:
|
||||
fh.write(b"not a zip")
|
||||
res = json.loads(read_file_tool(p))
|
||||
# Should NOT crash; falls through to the binary-extension guard.
|
||||
# Should NOT crash; the binary guard fires but surfaces the
|
||||
# specific extraction failure instead of the generic message.
|
||||
self.assertIn("error", res)
|
||||
self.assertIn("binary", res["error"].lower())
|
||||
self.assertIn("extraction failed", res["error"].lower())
|
||||
self.assertIn("docx", res["error"].lower())
|
||||
|
||||
def test_oversized_anydoc_read_surfaces_size_error(self):
|
||||
import tools.read_extract as rex
|
||||
|
||||
saved_cap = rex.MAX_ANYDOC_BYTES
|
||||
saved_module = rex._anydoc_module
|
||||
|
||||
class _FakeAnydoc:
|
||||
def to_markdown(self, path): # pragma: no cover - must not be called
|
||||
raise AssertionError("conversion should be rejected before call")
|
||||
|
||||
rex._anydoc_module = _FakeAnydoc()
|
||||
rex.MAX_ANYDOC_BYTES = 10
|
||||
try:
|
||||
p = os.path.join(self.tmp, "big.pdf")
|
||||
with open(p, "wb") as fh:
|
||||
fh.write(b"x" * 11)
|
||||
res = json.loads(read_file_tool(p))
|
||||
self.assertIn("error", res)
|
||||
self.assertIn("too large", res["error"].lower())
|
||||
# The size hint reaches the agent instead of a generic binary error.
|
||||
self.assertNotIn("cannot read binary file", res["error"].lower())
|
||||
finally:
|
||||
rex.MAX_ANYDOC_BYTES = saved_cap
|
||||
rex._anydoc_module = saved_module
|
||||
|
||||
def test_unavailable_converter_falls_back_to_raw_read(self):
|
||||
import time
|
||||
|
||||
import tools.read_extract as rex
|
||||
|
||||
saved_module = rex._anydoc_module
|
||||
saved_failed_at = rex._anydoc_failed_at
|
||||
# Simulate "converter unavailable and in cooldown": _anydoc() returns
|
||||
# None, the .pdf is not treated as extractable, and read_file keeps
|
||||
# its historical raw-read fallthrough (no extraction error surfaced).
|
||||
rex._anydoc_module = None
|
||||
rex._anydoc_failed_at = time.monotonic()
|
||||
try:
|
||||
p = os.path.join(self.tmp, "doc.pdf")
|
||||
with open(p, "wb") as fh:
|
||||
fh.write(b"%PDF-1.4 fake")
|
||||
res = json.loads(read_file_tool(p))
|
||||
self.assertNotIn("error", res)
|
||||
self.assertIn("%PDF-1.4 fake", res.get("content", ""))
|
||||
finally:
|
||||
rex._anydoc_module = saved_module
|
||||
rex._anydoc_failed_at = saved_failed_at
|
||||
|
||||
def test_docx_read_extracts(self):
|
||||
p = os.path.join(self.tmp, "d.docx")
|
||||
|
|
|
|||
|
|
@ -1535,6 +1535,8 @@ def read_file_tool(path: str, offset: int = 1, limit: int = 2000, task_id: str =
|
|||
# Try before the binary-extension guard so .docx/.xlsx can render as text.
|
||||
# Malformed documents fall through to the normal path/binary guard.
|
||||
from tools.read_extract import (
|
||||
ANYDOC_EXTENSIONS,
|
||||
EXTRACTABLE_EXTENSIONS,
|
||||
MAX_DOCUMENT_BYTES,
|
||||
ExtractionError,
|
||||
extract_document_bytes,
|
||||
|
|
@ -1555,8 +1557,31 @@ def read_file_tool(path: str, offset: int = 1, limit: int = 2000, task_id: str =
|
|||
extracted_text = extract_document_bytes(
|
||||
document_bytes, str(_resolved)
|
||||
)
|
||||
except (ExtractionError, ValueError, base64.binascii.Error):
|
||||
except (ExtractionError, ValueError, base64.binascii.Error) as exc:
|
||||
logger.debug("document extraction failed for %s", path, exc_info=True)
|
||||
# For binary document formats, surface the specific failure
|
||||
# (size cap, encrypted, malformed…) instead of falling through
|
||||
# — the fallthrough path can only produce a generic
|
||||
# binary-file error or garbage raw bytes, hiding the
|
||||
# actionable reason (e.g. "Document too large to convert").
|
||||
# .ipynb stays on the fallthrough path: it is plain JSON text
|
||||
# and a raw read is genuinely useful. Byte-transport issues
|
||||
# (ValueError / binascii) keep the fallthrough too — only a
|
||||
# specific ExtractionError carries an actionable reason.
|
||||
_doc_ext = _resolved.suffix.lower()
|
||||
_binary_doc = _doc_ext in ANYDOC_EXTENSIONS or (
|
||||
_doc_ext in EXTRACTABLE_EXTENSIONS and _doc_ext != ".ipynb"
|
||||
)
|
||||
if (
|
||||
_binary_doc
|
||||
and isinstance(exc, ExtractionError)
|
||||
and not str(exc).startswith("Unsupported document type")
|
||||
):
|
||||
return tool_error(
|
||||
f"Cannot read '{path}' ({_doc_ext}): document "
|
||||
f"extraction failed — {exc}. Use terminal utilities "
|
||||
"to inspect or convert the file."
|
||||
)
|
||||
else:
|
||||
lines = extracted_text.splitlines()
|
||||
total_lines = len(lines)
|
||||
|
|
|
|||
Loading…
Reference in New Issue