diff --git a/haiku_rag_slim/haiku/rag/converters/pdf_split.py b/haiku_rag_slim/haiku/rag/converters/pdf_split.py index 83dfb147..f5a6a7f4 100644 --- a/haiku_rag_slim/haiku/rag/converters/pdf_split.py +++ b/haiku_rag_slim/haiku/rag/converters/pdf_split.py @@ -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). diff --git a/tests/test_pdf_split.py b/tests/test_pdf_split.py index 32d98567..accb1fd9 100644 --- a/tests/test_pdf_split.py +++ b/tests/test_pdf_split.py @@ -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