mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-26 13:16:24 +00:00
4a92cb579a
* fix(uploads): keep document-conversion stat and write-back off the event loop convert_file_to_markdown offloads the conversion itself for files above 1 MB (documented in its docstring), but the two bookend filesystem calls still ran on the event loop: the size stat before the threshold check and the write of the converted markdown afterwards — the latter a potentially multi-megabyte write in the per-upload ingestion path. Route both through run_file_io (the dedicated file IO pool that also keeps the caller's context), matching the off-the-loop fixes already applied to the sibling upload/document paths. Add a tests/blocking_io anchor: with the converter patched to a 2 MB payload it fails on the previous head (Blockbuster catches the blocking os.stat first; the broad except swallows it into a None return) and passes with this change. * fix(uploads): address review — drain conversion write-back, make run_file_io re-entrant Review follow-up on the off-loop conversion stat/write PR: 1. The write-back now goes through await_drained(): a plain run_file_io await cancels immediately while the worker keeps writing, so the ingestion scope could remove its private directory over an in-flight write and leave a large hidden partial file. The drain delivers the cancellation only after the worker settled, and the handler removes the partial output before re-raising. 2. run_file_io is now re-entrant: a nested offload issued from code already running inside a file-io worker (project-document conversion runs asyncio.run(convert_file_to_markdown(...)) inside a run_file_io worker) executes inline instead of submitting to the same bounded executor, which deadlocked with a single configured worker. Both fixes carry deterministic regressions: the cancellation test pins the drain ordering (cancel issued before the worker is released) and the partial-file cleanup; the re-entrancy test pins inline execution by thread identity. Both fail on the previous head. Full blocking_io + project-document + conversion suites show an identical failure set to the previous head (pre-existing Windows platform failures only). * fix(uploads): keep cancellation authoritative when conversion cleanup fails If the post-cancellation unlink of the staging output raised, the new exception escaped the CancelledError handler, was swallowed by the broad except as an ordinary conversion failure (None return), and left both the caller's cancellation and the partial file behind. Catch and log cleanup failures inside the handler and always re-raise the captured cancellation; a second cancellation still passes through untouched (CancelledError is not an Exception). Red/green: tests/blocking_io/test_file_conversion_offloop.py gains a regression where unlink fails while the write-back is in flight — on the reviewed head the task settled on None (DID NOT RAISE); with the change the CancelledError propagates and the failure is logged. Signed-off-by: yetuge <2219677952@qq.com> --------- Signed-off-by: yetuge <2219677952@qq.com>
96 lines
3.8 KiB
Python
96 lines
3.8 KiB
Python
"""Regression anchor: ``convert_file_to_markdown`` must not block the event loop.
|
|
|
|
The converter itself is offloaded to a thread for files above 1 MB
|
|
(``_ASYNC_THRESHOLD_BYTES``), but the converted markdown was written back with
|
|
a synchronous ``Path.write_text`` on the event loop — a multi-megabyte blocking
|
|
write in the upload ingestion path (``app/gateway/upload_ingestion.py`` calls
|
|
this per uploaded document). This anchor drives the real
|
|
``convert_file_to_markdown`` under the strict Blockbuster gate with the
|
|
converter patched to return a large payload, so only the write-back is
|
|
exercised.
|
|
|
|
If the write regresses back onto the event loop, Blockbuster raises
|
|
``BlockingError`` — which the function's broad ``except`` turns into a ``None``
|
|
return, so this test fails on the missing output file.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import asyncio
|
|
import threading
|
|
from pathlib import Path
|
|
|
|
import pytest
|
|
|
|
pytestmark = pytest.mark.asyncio
|
|
|
|
|
|
async def test_convert_file_to_markdown_write_does_not_block_event_loop(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
|
from deerflow.utils import file_conversion
|
|
|
|
large_text = "x" * (2 * 1024 * 1024) # 2 MB conversion result
|
|
monkeypatch.setattr(file_conversion, "_do_convert", lambda *_args, **_kwargs: large_text)
|
|
|
|
source = tmp_path / "doc.txt"
|
|
source.write_text("source", encoding="utf-8") # test-side seeding (not in scanned_modules)
|
|
|
|
md_path = await file_conversion.convert_file_to_markdown(source)
|
|
|
|
assert md_path is not None, "conversion failed — see captured logs for the swallowed error"
|
|
assert md_path.read_text(encoding="utf-8") == large_text
|
|
|
|
|
|
async def test_cancelled_conversion_cleanup_failure_preserves_cancellation(
|
|
tmp_path: Path,
|
|
monkeypatch: pytest.MonkeyPatch,
|
|
caplog: pytest.LogCaptureFixture,
|
|
) -> None:
|
|
"""A failing cleanup ``unlink`` must not swallow the cancellation.
|
|
|
|
The cleanup ran bare inside the ``CancelledError`` handler: an unlink
|
|
OSError escaped the handler, was converted into an ordinary conversion
|
|
failure (``None`` return) by the broad ``except``, and left both the
|
|
caller's cancellation and the staging output behind. Cleanup failures
|
|
are now caught and logged inside the handler, and the captured
|
|
cancellation is always re-raised.
|
|
"""
|
|
from deerflow.utils import file_conversion
|
|
|
|
large_text = "x" * (2 * 1024 * 1024)
|
|
monkeypatch.setattr(file_conversion, "_do_convert", lambda *_args, **_kwargs: large_text)
|
|
|
|
source = tmp_path / "doc.txt"
|
|
source.write_text("source", encoding="utf-8")
|
|
|
|
def failing_unlink(self: Path, missing_ok: bool = False) -> None:
|
|
raise OSError("simulated unlink failure")
|
|
|
|
monkeypatch.setattr(Path, "unlink", failing_unlink)
|
|
|
|
real_run_file_io = file_conversion.run_file_io
|
|
started = threading.Event()
|
|
finish = threading.Event()
|
|
|
|
async def gated_run_file_io(fn, *args, **kwargs):
|
|
if getattr(fn, "__name__", "") == "write_text":
|
|
# Hold the write-back worker start until the test has cancelled
|
|
# the task, so the cancellation arrives while the write is in
|
|
# flight (deterministic ordering, no sleeps).
|
|
started.set()
|
|
await asyncio.to_thread(finish.wait, 5)
|
|
return await real_run_file_io(fn, *args, **kwargs)
|
|
|
|
monkeypatch.setattr(file_conversion, "run_file_io", gated_run_file_io)
|
|
|
|
task = asyncio.create_task(file_conversion.convert_file_to_markdown(source))
|
|
try:
|
|
await asyncio.to_thread(started.wait, 5)
|
|
task.cancel()
|
|
finish.set()
|
|
with pytest.raises(asyncio.CancelledError):
|
|
await task
|
|
finally:
|
|
finish.set()
|
|
|
|
assert any("partial conversion output" in record.getMessage() for record in caplog.records), "cleanup failure should be logged inside the handler, not escaped"
|