Skip to content

Commit 36be018

Browse files
badGarnetclaude
andcommitted
fix: judge an empty chunk body against its own media type
The previous commit raised on every empty HTTP-200 chunk body, which is wrong for NDJSON: zero records is zero lines, so an empty body is a well-formed result and a split whose pages are blank would have failed. Recombination now takes each chunk's Content-Type. An empty body raises only when the chunk is not application/x-ndjson -- JSON has no empty document, so an empty JSON 200 stays a defect, and an unknown media type is read as JSON, which is what the deployed API returns. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 0539741 commit 36be018

4 files changed

Lines changed: 206 additions & 37 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
## 0.46.1
22

33
### Fixes
4-
* Fail loudly when a split-PDF chunk returns HTTP 200 with an empty body in NDJSON elements-file mode. The chunk used to be logged and skipped, so the combined `elements_file` was silently short by those pages while the call still returned 200 — and `split_pdf_allow_failed=False` did not catch it, because an empty 200 counts as a successful chunk. Recombination now raises `EmptyChunkResponseError` (a `ValueError`), matching the buffered path, which raises `JSONDecodeError` on the same response. A chunk that legitimately produced no elements returns an empty JSON array and is still accepted.
4+
* Fail loudly when a split-PDF chunk returns HTTP 200 with an empty JSON body in NDJSON elements-file mode. The chunk used to be logged and skipped, so the combined `elements_file` was silently short by those pages while the call still returned 200 — and `split_pdf_allow_failed=False` did not catch it, because an empty 200 counts as a successful chunk. Recombination now raises `EmptyChunkResponseError` (a `ValueError`), matching the buffered path, which raises `JSONDecodeError` on the same response. Emptiness is judged against the chunk's own `Content-Type`: JSON has no empty document (a chunk with no elements is `[]`), while `application/x-ndjson` encodes zero records as zero lines, so an empty NDJSON chunk is well formed and still contributes nothing.
55

66
## 0.46.0
77

_test_unstructured_client/unit/test_ndjson_elements_file.py

Lines changed: 139 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -118,12 +118,16 @@ def test_mixed_chunk_formats(tmp_path):
118118

119119

120120
@pytest.mark.parametrize("body", ["", " ", "\n\n"])
121-
def test_empty_chunk_files_raise(tmp_path, body):
122-
"""An empty 200 body must fail, not be skipped.
123-
124-
Skipping it made the document silently short: this function returns only a path and an
125-
element count, so nothing downstream could tell a truncated document from a complete
126-
one. `split_pdf_allow_failed` does not cover it either -- the chunk reported success.
121+
@pytest.mark.parametrize("media_type", [None, "application/json", "application/json; charset=utf-8"])
122+
def test_empty_json_chunk_files_raise(tmp_path, body, media_type):
123+
"""An empty 200 body from a JSON chunk must fail, not be skipped.
124+
125+
JSON has no empty document -- a chunk with no elements is `[]` -- so this is a
126+
malformed response. Skipping it made the document silently short: this function returns
127+
only a path and an element count, so nothing downstream could tell a truncated document
128+
from a complete one. `split_pdf_allow_failed` does not cover it either -- the chunk
129+
reported success. An unset `Content-Type` is treated as JSON, which is what the
130+
deployed API returns.
127131
"""
128132
chunk_a = tmp_path / "a.json"
129133
chunk_empty = tmp_path / "empty.json"
@@ -132,7 +136,18 @@ def test_empty_chunk_files_raise(tmp_path, body):
132136

133137
out = tmp_path / "combined.ndjson"
134138
with pytest.raises(request_utils.EmptyChunkResponseError, match="empty.json"):
135-
combine_chunk_files_to_ndjson([str(chunk_a), str(chunk_empty)], str(out))
139+
combine_chunk_files_to_ndjson(
140+
[str(chunk_a), str(chunk_empty)], str(out), ["application/json", media_type]
141+
)
142+
143+
144+
def test_empty_chunk_media_types_default_to_json_when_not_passed(tmp_path):
145+
"""Callers that cannot supply media types must still get the strict reading."""
146+
chunk_empty = tmp_path / "empty.json"
147+
chunk_empty.write_text("", encoding="utf-8")
148+
149+
with pytest.raises(request_utils.EmptyChunkResponseError):
150+
combine_chunk_files_to_ndjson([str(chunk_empty)], str(tmp_path / "combined.ndjson"))
136151

137152

138153
def test_empty_chunk_error_is_a_value_error(tmp_path):
@@ -141,14 +156,59 @@ def test_empty_chunk_error_is_a_value_error(tmp_path):
141156
chunk_empty.write_text("", encoding="utf-8")
142157

143158
with pytest.raises(ValueError):
144-
combine_chunk_files_to_ndjson([str(chunk_empty)], str(tmp_path / "combined.ndjson"))
159+
combine_chunk_files_to_ndjson(
160+
[str(chunk_empty)], str(tmp_path / "combined.ndjson"), ["application/json"]
161+
)
162+
163+
164+
@pytest.mark.parametrize("body", ["", " ", "\n\n"])
165+
@pytest.mark.parametrize(
166+
"media_type", ["application/x-ndjson", "application/x-ndjson; charset=utf-8", "APPLICATION/X-NDJSON"]
167+
)
168+
def test_empty_ndjson_chunk_is_zero_records(tmp_path, body, media_type):
169+
"""NDJSON spells zero records as zero lines, so an empty body is well formed.
170+
171+
The counterpart to `test_empty_json_chunk_files_raise`: the same empty file on disk is
172+
a defect from a JSON chunk and a legitimate result from an NDJSON one, and the declared
173+
media type is the only thing that separates them. Failing here would break a split
174+
whose pages are blank.
175+
"""
176+
chunk_a = tmp_path / "a.ndjson"
177+
chunk_empty = tmp_path / "empty.ndjson"
178+
elements_a = _elements("a", 2)
179+
_write_ndjson(chunk_a, elements_a)
180+
chunk_empty.write_text(body, encoding="utf-8")
181+
182+
out = tmp_path / "combined.ndjson"
183+
written = combine_chunk_files_to_ndjson(
184+
[str(chunk_a), str(chunk_empty)], str(out), ["application/x-ndjson", media_type]
185+
)
186+
187+
assert written == 2
188+
assert _read_ndjson(out) == elements_a
189+
190+
191+
def test_every_ndjson_chunk_empty_yields_an_empty_output(tmp_path):
192+
"""A document whose every chunk produced no elements is empty, not an error."""
193+
chunk_a = tmp_path / "a.ndjson"
194+
chunk_b = tmp_path / "b.ndjson"
195+
chunk_a.write_text("", encoding="utf-8")
196+
chunk_b.write_text("", encoding="utf-8")
197+
198+
out = tmp_path / "combined.ndjson"
199+
written = combine_chunk_files_to_ndjson(
200+
[str(chunk_a), str(chunk_b)], str(out), ["application/x-ndjson"] * 2
201+
)
202+
203+
assert written == 0
204+
assert out.read_text(encoding="utf-8") == ""
145205

146206

147207
def test_empty_array_chunk_contributes_nothing(tmp_path):
148208
"""A chunk that legitimately produced no elements (e.g. blank pages).
149209
150-
The counterpart to `test_empty_chunk_files_raise`: `[]` and an empty body are
151-
different responses on the wire, and only the latter is a defect.
210+
The JSON counterpart to `test_empty_ndjson_chunk_is_zero_records`: `[]` and an empty
211+
body are different responses on the wire, and only the latter is a defect.
152212
"""
153213
chunk_a = tmp_path / "a.json"
154214
chunk_b = tmp_path / "b.json"
@@ -396,16 +456,31 @@ def test_server_cannot_name_a_local_file_via_a_response_header(tmp_path):
396456
# --- hook-level temp-file lifecycle ------------------------------------------------
397457

398458

399-
def _ndjson_response(elements):
459+
def _ndjson_response(elements, media_type="application/x-ndjson"):
400460
body = "".join(json.dumps(e) + "\n" for e in elements).encode()
401-
return httpx.Response(status_code=200, content=body)
461+
headers = {"content-type": media_type} if media_type else {}
462+
return httpx.Response(status_code=200, headers=headers, content=body)
463+
464+
465+
def _cached_chunk_response(path, media_type="application/x-ndjson"):
466+
"""What the cached path leaves behind: the body replaced by its temp-file path.
467+
468+
Mirrors `_await_elements`' cached branch, which streams the body to disk and rebuilds
469+
the response with `content=temp_file_name` and the server's original headers -- so the
470+
declared media type still describes the file's contents, not the path in the body.
471+
"""
472+
return httpx.Response(
473+
status_code=200,
474+
headers={"content-type": media_type} if media_type else {},
475+
content=str(path).encode(),
476+
)
402477

403478

404-
def _hook_in_ndjson_mode(operation_id, tmp_path):
479+
def _hook_in_ndjson_mode(operation_id, tmp_path, cache_tmp_data=False):
405480
"""A hook set up as `before_request` would leave it for an uncached NDJSON run."""
406481
hook = SplitPdfHook()
407482
hook.ndjson_mode[operation_id] = True
408-
hook.cache_tmp_data_feature[operation_id] = False
483+
hook.cache_tmp_data_feature[operation_id] = cache_tmp_data
409484
hook.cache_tmp_data_dir[operation_id] = str(tmp_path)
410485
hook.allow_failed[operation_id] = False
411486
# Marks the operation live; `_clear_operation` removing it is what signals teardown.
@@ -562,8 +637,9 @@ def test_malformed_chunk_leaves_no_partial_output_behind(tmp_path):
562637
assert list(Path(tmp_path).glob("*.partial")) == []
563638

564639

565-
def test_empty_chunk_response_fails_the_operation(tmp_path):
566-
"""An empty 200 chunk must fail the whole partition, as the buffered path does.
640+
@pytest.mark.parametrize("media_type", ["application/json", None])
641+
def test_empty_json_chunk_response_fails_the_operation(tmp_path, media_type):
642+
"""An empty 200 JSON chunk must fail the whole partition, as the buffered path does.
567643
568644
Driven through the hook because that is where the consequence lives: previously the
569645
chunk was skipped and `_build_after_success_response` handed back a combined file that
@@ -572,9 +648,10 @@ def test_empty_chunk_response_fails_the_operation(tmp_path):
572648
"""
573649
operation_id = "op-empty-chunk"
574650
hook = _hook_in_ndjson_mode(operation_id, tmp_path)
651+
headers = {"content-type": media_type} if media_type else {}
575652
responses = [
576653
(0, _ndjson_response(_elements("a", 2))),
577-
(1, httpx.Response(status_code=200, content=b"")),
654+
(1, httpx.Response(status_code=200, headers=headers, content=b"")),
578655
]
579656

580657
with pytest.raises(request_utils.EmptyChunkResponseError):
@@ -585,6 +662,51 @@ def test_empty_chunk_response_fails_the_operation(tmp_path):
585662
assert list(Path(tmp_path).glob("*.partial")) == []
586663

587664

665+
def test_empty_ndjson_chunk_response_is_accepted_in_memory_mode(tmp_path):
666+
"""A zero-record NDJSON chunk is a valid result and must not fail the partition.
667+
668+
The default path (`cache_tmp_data` off): the empty body is spilled to disk, so the
669+
media type carried by the response is the only thing that distinguishes it from the
670+
malformed JSON case above.
671+
"""
672+
operation_id = "op-empty-ndjson-memory"
673+
hook = _hook_in_ndjson_mode(operation_id, tmp_path)
674+
elements_a, elements_c = _elements("a", 2), _elements("c", 3)
675+
responses = [
676+
(0, _ndjson_response(elements_a)),
677+
(1, _ndjson_response([])), # a chunk of blank pages
678+
(2, _ndjson_response(elements_c)),
679+
]
680+
681+
hook._elements_from_task_responses(operation_id, responses, started_at=0.0)
682+
683+
combined = hook.ndjson_output_path[operation_id]
684+
assert _read_ndjson(combined) == elements_a + elements_c
685+
# The spilled chunk files are gone; only the combined output survives.
686+
assert _ndjson_files_in(tmp_path) == [Path(combined).name]
687+
688+
689+
def test_empty_ndjson_chunk_response_is_accepted_in_cached_mode(tmp_path):
690+
"""Cached counterpart: the chunk file on disk is empty and the body is its path."""
691+
operation_id = "op-empty-ndjson-cached"
692+
hook = _hook_in_ndjson_mode(operation_id, tmp_path, cache_tmp_data=True)
693+
# Cached chunk files live in the operation's tempdir, kept out of the directory the
694+
# combined file lands in so the assertion below stays unambiguous.
695+
cache_dir = tmp_path / "cache"
696+
cache_dir.mkdir()
697+
elements_a = _elements("a", 2)
698+
chunk_a, chunk_empty = cache_dir / "a.json", cache_dir / "empty.json"
699+
_write_ndjson(chunk_a, elements_a)
700+
chunk_empty.write_text("", encoding="utf-8")
701+
responses = [(0, _cached_chunk_response(chunk_a)), (1, _cached_chunk_response(chunk_empty))]
702+
703+
hook._elements_from_task_responses(operation_id, responses, started_at=0.0)
704+
705+
combined = hook.ndjson_output_path[operation_id]
706+
assert _read_ndjson(combined) == elements_a
707+
assert _ndjson_files_in(tmp_path) == [Path(combined).name]
708+
709+
588710
def test_no_combined_file_is_created_when_every_chunk_failed(tmp_path):
589711
operation_id = "op-all-failed"
590712
hook = _hook_in_ndjson_mode(operation_id, tmp_path)

src/unstructured_client/_hooks/custom/request_utils.py

Lines changed: 49 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -291,18 +291,27 @@ def create_response(elements: list) -> httpx.Response:
291291

292292

293293
class EmptyChunkResponseError(ValueError):
294-
"""A split-PDF chunk returned HTTP 200 with a body that holds no elements at all.
294+
"""A split-PDF chunk returned HTTP 200 with a body its own media type cannot produce.
295295
296-
An empty body is not the same wire response as an empty JSON array: `[]` is a chunk
297-
that legitimately produced no elements (blank pages) and is accepted, while nothing
298-
at all is a defect. Skipping it would make the document silently short, and
299-
`split_pdf_allow_failed` cannot catch it because the chunk reports success.
296+
Whether an empty body is a defect depends on the format the chunk claims. JSON has no
297+
empty document -- a chunk with no elements is `[]` -- so nothing at all is malformed,
298+
and skipping it would make the document silently short with no way for the caller to
299+
notice (`split_pdf_allow_failed` does not cover it, because the chunk reports
300+
success). NDJSON is a record per line, so an empty body is a well-formed zero-record
301+
response and is accepted.
300302
301303
Subclasses `ValueError` so that it lands where the buffered path's `JSONDecodeError`
302304
(also a `ValueError`) already does for callers that guard the parse.
303305
"""
304306

305307

308+
def _is_ndjson_media_type(media_type: Optional[str]) -> bool:
309+
"""Whether a chunk's `Content-Type` declares NDJSON, ignoring any parameters."""
310+
if not media_type:
311+
return False
312+
return media_type.split(";", 1)[0].strip().lower() == NDJSON_MEDIA_TYPE
313+
314+
306315
def _first_non_space_char(stream: TextIO) -> str:
307316
"""Return the first non-whitespace character in `stream`, or "" if there is none."""
308317
while True:
@@ -314,7 +323,11 @@ def _first_non_space_char(stream: TextIO) -> str:
314323
return stripped[0]
315324

316325

317-
def combine_chunk_files_to_ndjson(chunk_paths: list[str], out_path: str) -> int:
326+
def combine_chunk_files_to_ndjson(
327+
chunk_paths: list[str],
328+
out_path: str,
329+
media_types: Optional[list[Optional[str]]] = None,
330+
) -> int:
318331
"""Combine per-chunk split-PDF response files into one NDJSON file on disk.
319332
320333
Recombining chunks by parsing them builds four full copies of the document (a list
@@ -329,35 +342,55 @@ def combine_chunk_files_to_ndjson(chunk_paths: list[str], out_path: str) -> int:
329342
330343
NDJSON chunks are copied through without parsing.
331344
345+
An empty body is read against the chunk's declared media type, which is the only
346+
thing that distinguishes the two cases: NDJSON encodes zero records as zero lines, so
347+
an empty NDJSON chunk contributes nothing and is fine; JSON has no empty document, so
348+
an empty one is malformed and raises rather than silently shortening the result.
349+
A chunk whose media type is unknown is treated as JSON -- that is what the deployed
350+
API returns, and guessing NDJSON would reinstate the silent truncation.
351+
332352
Args:
333353
chunk_paths: Per-chunk response files, in element order.
334354
out_path: File to write the combined NDJSON to.
355+
media_types: The `Content-Type` each chunk was served with, positionally matching
356+
`chunk_paths`. Omit when the media types are not known.
335357
336358
Returns:
337359
The number of elements written.
338360
339361
Raises:
340-
EmptyChunkResponseError: A chunk returned 200 with an empty body.
362+
EmptyChunkResponseError: A non-NDJSON chunk returned 200 with an empty body.
341363
"""
342364
total = 0
343365
with open(out_path, "w", encoding="utf-8") as out:
344-
for chunk_path in chunk_paths:
366+
for index, chunk_path in enumerate(chunk_paths):
367+
media_type = media_types[index] if media_types is not None else None
345368
with open(chunk_path, "r", encoding="utf-8") as chunk:
346369
first_char = _first_non_space_char(chunk)
347370
if not first_char:
348-
# A 200 with an empty body. Fail loudly: skipping it makes the document
349-
# silently short with no way for the caller to notice, since this
350-
# function's only output is the combined path. The buffered path raises
351-
# `JSONDecodeError` on the same response, so NDJSON mode must not turn
352-
# a hard failure into truncation.
371+
if _is_ndjson_media_type(media_type):
372+
# Zero records, spelled the only way NDJSON can spell it.
373+
logger.debug(
374+
"split_pdf event=ndjson_empty_chunk file=%s media_type=%s",
375+
os.path.basename(chunk_path),
376+
media_type,
377+
)
378+
continue
379+
# A JSON 200 with an empty body. Fail loudly: skipping it makes the
380+
# document silently short with no way for the caller to notice, since
381+
# this function's only output is the combined path. The buffered path
382+
# raises `JSONDecodeError` on the same response, so NDJSON mode must
383+
# not turn a hard failure into truncation.
353384
logger.error(
354-
"split_pdf event=ndjson_empty_chunk file=%s",
385+
"split_pdf event=empty_chunk_body file=%s media_type=%s",
355386
os.path.basename(chunk_path),
387+
media_type,
356388
)
357389
raise EmptyChunkResponseError(
358390
"A split-PDF chunk returned HTTP 200 with an empty body "
359-
f"({os.path.basename(chunk_path)}); its elements would be missing "
360-
"from the combined output. An empty result must be an empty JSON "
391+
f"({os.path.basename(chunk_path)}, Content-Type: "
392+
f"{media_type or 'unset'}); its elements would be missing from the "
393+
"combined output. A JSON chunk with no elements must be an empty "
361394
"array."
362395
)
363396
chunk.seek(0)

0 commit comments

Comments
 (0)