mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-07-28 17:06:05 +00:00
75fa028e89
* fix(artifacts): serve inline binary artifacts via FileResponse for Range support Audio/video/image artifacts that are previewed inline (not downloaded, not active content) were served by reading the whole file into memory and returning it through a plain Response. That response never sets Accept-Ranges and ignores any Range header the browser sends, so seeking an <audio>/<video> element backed by this endpoint always gets the full 200 response back instead of a 206 partial response for the requested byte range -- which is why dragging the seek bar on an audio artifact restarts playback from the beginning instead of jumping to the new position. Route this case through FileResponse instead (as the active-content/ download branch already does), which handles Range/If-Range natively. Verified with a real Range request against the endpoint: initial load now reports Accept-Ranges: bytes, and a ranged GET returns 206 with the correct Content-Range and only the requested slice of bytes. Fixes #3240 * fix(artifacts): address review nits on the inline FileResponse branch Two non-blocking nits from review: - Drop the redundant filename= kwarg on the inline_file FileResponse call. Content-Disposition is already set explicitly on this branch, and FileResponse only uses filename to setdefault that same header -- which is a no-op once it's already present. Harmless today, but removes the latent risk that a future Starlette version turning that setdefault into a hard set would silently flip inline preview to attachment. - Reword the blocking-IO regression-anchor docstring: it claimed awaiting get_artifact for a binary artifact does "zero filesystem IO", but _read_artifact_payload still runs exists/is_file/ mimetypes.guess_type/is_text_file_by_content (an 8 KB read) for binary files too, just offloaded via asyncio.to_thread like the text branch. The gate has nothing to catch because that IO is off the event loop, not because there's none -- reworded to say no full-file read happens, matching the accurate framing already used one paragraph up. tests/test_artifacts_router.py, tests/blocking_io/test_artifacts_router.py (19 tests) are green, and ruff check/format are clean on both changed files.
99 lines
4.3 KiB
Python
99 lines
4.3 KiB
Python
"""Regression anchor: serving artifacts must not block the event loop.
|
|
|
|
``get_artifact`` probes the artifact path (``exists`` / ``is_file``), reads
|
|
text content (``read_text``), sniffs text-ness (``is_text_file_by_content``),
|
|
and extracts ``.skill`` archive members — all blocking filesystem IO. The
|
|
handler offloads each branch's IO via ``asyncio.to_thread``; if any regresses
|
|
back onto the event loop, the strict Blockbuster gate raises ``BlockingError``
|
|
and these tests fail.
|
|
|
|
Binary (non-text, non-active-content) artifacts still run the same
|
|
``exists`` / ``is_file`` / ``mimetypes.guess_type`` / ``is_text_file_by_content``
|
|
probes as the text branch, offloaded the same way via ``asyncio.to_thread``.
|
|
What differs is the payload: instead of a read, the handler returns a
|
|
``FileResponse`` that defers its own file IO to ASGI response time (streamed
|
|
via ``anyio.open_file`` when the response is actually sent, which this test
|
|
never triggers) — so awaiting ``get_artifact`` itself for a binary artifact
|
|
does no full-file read; ``FileResponse`` streams the bytes at ASGI send
|
|
time, off the loop, which is why the gate has nothing to catch there
|
|
either way.
|
|
|
|
The ``@require_permission`` decorator is bypassed via ``__wrapped__`` so the
|
|
anchor exercises the handler's own filesystem IO, not the authz layer. Imports
|
|
sit at module top so any import-time IO runs at collection, outside the gate.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import asyncio
|
|
import zipfile
|
|
from pathlib import Path
|
|
|
|
import pytest
|
|
from starlette.responses import FileResponse
|
|
|
|
from app.gateway.path_utils import resolve_thread_virtual_path
|
|
from app.gateway.routers.artifacts import get_artifact
|
|
|
|
pytestmark = pytest.mark.asyncio
|
|
|
|
# The undecorated coroutine (``require_permission`` uses ``functools.wraps``).
|
|
_get_artifact = get_artifact.__wrapped__
|
|
|
|
|
|
async def _seed(tmp_path: Path, monkeypatch, thread_id: str, virtual_path: str) -> Path:
|
|
monkeypatch.setenv("DEER_FLOW_HOME", str(tmp_path))
|
|
# Rebuild cached Paths against the tmp home so the artifact resolves under it.
|
|
import deerflow.config.paths as paths_mod
|
|
|
|
monkeypatch.setattr(paths_mod, "_paths", None)
|
|
# Test-side path resolution also touches the filesystem (`.resolve()`); offload
|
|
# it so this seeding helper doesn't itself trip the gate.
|
|
target = await asyncio.to_thread(resolve_thread_virtual_path, thread_id, virtual_path)
|
|
await asyncio.to_thread(target.parent.mkdir, parents=True, exist_ok=True)
|
|
return target
|
|
|
|
|
|
async def test_get_artifact_text_does_not_block_event_loop(tmp_path: Path, monkeypatch) -> None:
|
|
vpath = "mnt/user-data/outputs/notes.txt"
|
|
target = await _seed(tmp_path, monkeypatch, "t1", vpath)
|
|
await asyncio.to_thread(target.write_text, "hello world", encoding="utf-8")
|
|
|
|
resp = await _get_artifact("t1", vpath, request=None, download=False)
|
|
|
|
assert resp.status_code == 200
|
|
assert resp.body == b"hello world"
|
|
|
|
|
|
async def test_get_artifact_binary_does_not_block_event_loop(tmp_path: Path, monkeypatch) -> None:
|
|
vpath = "mnt/user-data/outputs/blob.bin"
|
|
target = await _seed(tmp_path, monkeypatch, "t1", vpath)
|
|
payload = b"\x00\x01\x02PNGDATA" # null byte -> binary branch (inline FileResponse)
|
|
await asyncio.to_thread(target.write_bytes, payload)
|
|
|
|
resp = await _get_artifact("t1", vpath, request=None, download=False)
|
|
|
|
# Binary artifacts are streamed via FileResponse (so browsers can issue
|
|
# byte-Range requests) instead of being read into memory up front, so the
|
|
# file bytes are only touched later during ASGI send, never here.
|
|
assert isinstance(resp, FileResponse)
|
|
assert resp.status_code == 200
|
|
assert Path(resp.path) == target
|
|
assert resp.headers.get("content-disposition", "").startswith("inline;")
|
|
|
|
|
|
async def test_get_artifact_skill_archive_member_does_not_block_event_loop(tmp_path: Path, monkeypatch) -> None:
|
|
skill_vpath = "mnt/user-data/outputs/demo.skill"
|
|
target = await _seed(tmp_path, monkeypatch, "t1", skill_vpath)
|
|
|
|
def _build_skill_zip() -> None:
|
|
with zipfile.ZipFile(target, "w") as zf:
|
|
zf.writestr("SKILL.md", "# demo skill\n")
|
|
|
|
await asyncio.to_thread(_build_skill_zip)
|
|
|
|
resp = await _get_artifact("t1", f"{skill_vpath}/SKILL.md", request=None, download=False)
|
|
|
|
assert resp.status_code == 200
|
|
assert b"# demo skill" in resp.body
|