Skip to content

Commit 3080cf6

Browse files
badGarnetclaude
andcommitted
test: pin the orphan-cleanup behavior on NDJSON write failures
The unlink-on-failure paths added in the previous commit had no coverage -- the existing tests only walked the success roundtrip, so the cleanup could have been removed without anything going red. Three tests, each confirmed to fail with its corresponding unlink removed: - write_chunk_body_to_temp: os.fdopen is patched so the write raises ENOSPC. The wrapper still closes the real handle, so the fd is not leaked by the test itself. - _ndjson_elements_file and its async counterpart: a response whose byte iterator raises partway through. tempfile.tempdir is redirected at the test's tmp_path so the assertion can see whether anything was left behind. Each asserts both halves of the contract: no file survives, and the original exception still propagates rather than being swallowed by the cleanup. Coverage for the general.py pair was not requested in review, but those helpers grew the same delete=False cleanup in the same commit and had the same gap. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 5fc5a14 commit 3080cf6

1 file changed

Lines changed: 85 additions & 1 deletion

File tree

‎_test_unstructured_client/unit/test_ndjson_elements_file.py‎

Lines changed: 85 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,22 +9,29 @@
99
- leave no temp files behind other than the combined file the caller owns
1010
"""
1111

12+
import errno
1213
import json
1314
import os
15+
import tempfile
1416
from pathlib import Path
17+
from unittest import mock
1518

1619
import pytest
1720

1821
import httpx
1922

23+
from unstructured_client._hooks.custom import request_utils
2024
from unstructured_client._hooks.custom.request_utils import (
2125
ELEMENTS_FILE_EXTENSION_KEY,
2226
combine_chunk_files_to_ndjson,
2327
create_elements_file_response,
2428
write_chunk_body_to_temp,
2529
)
2630
from unstructured_client._hooks.custom.split_pdf_hook import SplitPdfHook
27-
from unstructured_client.general import _ndjson_elements_file
31+
from unstructured_client.general import (
32+
_ndjson_elements_file,
33+
_ndjson_elements_file_async,
34+
)
2835

2936

3037
def _elements(prefix, count):
@@ -171,6 +178,83 @@ def test_write_chunk_body_to_temp_roundtrips(tmp_path):
171178
assert _read_ndjson(path) == elements
172179

173180

181+
def test_spill_failure_leaves_no_orphan_file(tmp_path):
182+
"""A write that fails partway must take its own temp file with it.
183+
184+
The caller only registers the returned path for cleanup once this function returns,
185+
so an orphan here is permanent -- and a full disk, the likeliest cause, is exactly
186+
the failure that repeats on every retry.
187+
"""
188+
real_fdopen = os.fdopen
189+
190+
class _FailingWriter:
191+
"""Wraps the real handle so the fd is still closed, but the write blows up."""
192+
193+
def __init__(self, handle):
194+
self._handle = handle
195+
196+
def write(self, _data):
197+
raise OSError(errno.ENOSPC, "No space left on device")
198+
199+
def __enter__(self):
200+
return self
201+
202+
def __exit__(self, *_exc):
203+
self._handle.close()
204+
return False
205+
206+
def _failing_fdopen(fd, mode):
207+
return _FailingWriter(real_fdopen(fd, mode))
208+
209+
response = httpx.Response(status_code=200, content=b'{"type": "Table"}\n')
210+
211+
with mock.patch.object(request_utils.os, "fdopen", _failing_fdopen):
212+
with pytest.raises(OSError) as excinfo:
213+
write_chunk_body_to_temp(response, str(tmp_path))
214+
215+
assert excinfo.value.errno == errno.ENOSPC
216+
assert list(tmp_path.iterdir()) == []
217+
218+
219+
def test_elements_file_copy_failure_leaves_no_orphan(tmp_path, monkeypatch):
220+
"""A body that dies mid-copy must not leave the partial file behind.
221+
222+
The destination is created with delete=False so it can outlive the helper, which is
223+
exactly what makes an interrupted copy leak.
224+
"""
225+
monkeypatch.setattr(tempfile, "tempdir", str(tmp_path))
226+
227+
class _FailingResponse:
228+
extensions: dict = {}
229+
230+
def iter_bytes(self):
231+
yield b'{"type": "Table"}\n'
232+
raise httpx.ReadError("connection dropped")
233+
234+
with pytest.raises(httpx.ReadError):
235+
_ndjson_elements_file(_FailingResponse())
236+
237+
assert list(tmp_path.glob("unst_elements_*")) == []
238+
239+
240+
@pytest.mark.asyncio
241+
async def test_elements_file_copy_failure_leaves_no_orphan_async(tmp_path, monkeypatch):
242+
"""Async counterpart of `test_elements_file_copy_failure_leaves_no_orphan`."""
243+
monkeypatch.setattr(tempfile, "tempdir", str(tmp_path))
244+
245+
class _FailingResponse:
246+
extensions: dict = {}
247+
248+
async def aiter_bytes(self):
249+
yield b'{"type": "Table"}\n'
250+
raise httpx.ReadError("connection dropped")
251+
252+
with pytest.raises(httpx.ReadError):
253+
await _ndjson_elements_file_async(_FailingResponse())
254+
255+
assert list(tmp_path.glob("unst_elements_*")) == []
256+
257+
174258
def test_combine_accepts_bodies_spilled_without_caching(tmp_path):
175259
"""End-to-end of the uncached path: spill two bodies, then combine them."""
176260
a, b = _elements("a", 2), _elements("b", 3)

0 commit comments

Comments
 (0)