Unlink PDF slice tempfiles when slice write fails
This commit is contained in:
parent
98efd73177
commit
e5e0df6ade
2 changed files with 60 additions and 7 deletions
|
|
@ -99,13 +99,14 @@ async def convert_pdf_with_splitting(
|
|||
if slice_item is _SENTINEL:
|
||||
break
|
||||
start, end, pdf_bytes = slice_item
|
||||
with tempfile.NamedTemporaryFile(
|
||||
mode="wb", suffix=".pdf", delete=False
|
||||
) as tmp:
|
||||
tmp.write(pdf_bytes)
|
||||
tmp.flush()
|
||||
tmp_path = Path(tmp.name)
|
||||
tmp_path: Path | None = None
|
||||
try:
|
||||
with tempfile.NamedTemporaryFile(
|
||||
mode="wb", suffix=".pdf", delete=False
|
||||
) as tmp:
|
||||
tmp_path = Path(tmp.name)
|
||||
tmp.write(pdf_bytes)
|
||||
tmp.flush()
|
||||
with logfire.span(
|
||||
"document.convert_slice",
|
||||
uri=source_uri,
|
||||
|
|
@ -122,7 +123,12 @@ async def convert_pdf_with_splitting(
|
|||
) from exc
|
||||
converted.append(slice_doc)
|
||||
finally:
|
||||
tmp_path.unlink(missing_ok=True)
|
||||
# `delete=False` is required so the converter (which opens
|
||||
# tmp_path itself) sees a fully written, closed file. Unlink
|
||||
# in finally so a mid-write disk-full / mid-convert failure
|
||||
# doesn't leak the slice on disk.
|
||||
if tmp_path is not None:
|
||||
tmp_path.unlink(missing_ok=True)
|
||||
finally:
|
||||
# Close the generator under the pdfium lock so src.close() runs
|
||||
# even when we abort mid-stream (slice failure, cancellation).
|
||||
|
|
|
|||
|
|
@ -69,6 +69,53 @@ def test_iter_pdf_slices_rejects_zero_slice_size(tmp_path):
|
|||
list(iter_pdf_slices(src, slice_size=0))
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_convert_unlinks_slice_tempfile_on_write_failure(tmp_path, monkeypatch):
|
||||
"""If writing the slice bytes to the tempfile raises (e.g. ENOSPC), the
|
||||
tempfile is created on disk but never reaches the converter. The original
|
||||
error must surface and the orphaned file must be removed."""
|
||||
src = _make_pdf(2, tmp_path)
|
||||
|
||||
monkeypatch.setattr("tempfile.tempdir", str(tmp_path))
|
||||
|
||||
import tempfile as _tempfile
|
||||
|
||||
real_factory = _tempfile.NamedTemporaryFile
|
||||
created: list[Path] = []
|
||||
|
||||
def _make_failing_tempfile(*args, **kwargs):
|
||||
handle = real_factory(*args, **kwargs)
|
||||
created.append(Path(handle.name))
|
||||
original_write = handle.write
|
||||
|
||||
def _raising_write(_data):
|
||||
# Touch the underlying file once so we know the path exists on
|
||||
# disk and the cleanup actually has something to remove.
|
||||
original_write(b"\0")
|
||||
raise OSError("No space left on device")
|
||||
|
||||
handle.write = _raising_write
|
||||
return handle
|
||||
|
||||
monkeypatch.setattr(_tempfile, "NamedTemporaryFile", _make_failing_tempfile)
|
||||
|
||||
class _UnusedConverter:
|
||||
async def convert_file(self, path: Path, *, source_uri):
|
||||
raise AssertionError("converter must not be reached on write failure")
|
||||
|
||||
with pytest.raises(OSError, match="No space left on device"):
|
||||
await convert_pdf_with_splitting(
|
||||
_UnusedConverter(), # ty: ignore[invalid-argument-type]
|
||||
src,
|
||||
source_uri=None,
|
||||
slice_size=1,
|
||||
)
|
||||
|
||||
assert created, "expected NamedTemporaryFile to be called at least once"
|
||||
leftover = [p for p in created if p.exists()]
|
||||
assert leftover == [], f"tempfiles leaked after write failure: {leftover}"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_convert_aborts_and_cleans_up_on_mid_stream_slice_failure(
|
||||
tmp_path, monkeypatch
|
||||
|
|
|
|||
Loading…
Reference in a new issue