feat: v0.6.0 — read build_stream log instead of dropping it (#2)
DSM emits a readable plaintext build log over the build_stream HTTP body (one short status line per step) and closes the connection when the build is done. The 0.2.5 implementation sent the request and dropped the body unread, leaving users with nothing more than a BUILD_FAILED polling status and no actionable diagnostic. DsmClient.trigger_build_stream now consumes the body line-by-line and returns the collected log as a string. Wall-clock budget of 210 s (under the Claude Desktop ~4 min ceiling); on timeout the partial log is returned with a "[build_stream: timeout — stream still open server-side]" marker so callers know the build continues server-side. Per-chunk ReadTimeout is treated the same way. JSON error envelope, transport-error mapping (M-4), and SID-scrubbed HTTP-error formatting are unchanged. redeploy_project and create_project now parse the returned log via _parse_build_stream_log (any line containing "Error response from daemon:" or ending in " Error" counts as a failure). On a failed log the tools abort immediately, surface the daemon line(s) in the result (e.g. "Error response from daemon: manifest for nginx:9.9.9 not found: manifest unknown"), and skip the polling step. The BUILD_FAILED polling guard (M-5) stays as a second safety net for late failures where the stream was clean but the container exited after start. No new MCP tool: the build log is a live stream and cannot be re-fetched after the build ends, so it is surfaced during redeploy_project / create_project rather than exposed as a standalone get_project_build_log call. Minor version bump because redeploy_project and create_project return materially different strings on a failed build and exit earlier in the failure path. Signatures unchanged. Tests: streamed-log collection, daemon-error log, header ReadTimeout marker, per-chunk ReadTimeout partial log, wall-clock budget truncation, _parse_build_stream_log unit tests, redeploy/create end- to-end behavior with a failing log. Closes #2 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
+110
-17
@@ -6,8 +6,8 @@ Covers the critical paths of DsmClient:
|
||||
sensitive-param log masking.
|
||||
- Session re-auth retry: single-retry semantics, thundering-herd,
|
||||
auth-manager-absent, re-auth failure.
|
||||
- trigger_build_stream: SSE fire-and-forget, JSON error detection,
|
||||
ReadTimeout swallowing, HTTP-error scrubbing.
|
||||
- trigger_build_stream: streamed log collection, JSON error detection,
|
||||
ReadTimeout marker, HTTP-error scrubbing.
|
||||
- upload_text / download_text happy-path + error-response.
|
||||
- _ensure_initialized double-checked-locking and M4 negative-cache cooldown.
|
||||
|
||||
@@ -490,27 +490,65 @@ async def test_request_reauth_thundering_herd_login_called_once() -> None:
|
||||
# ──────────────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
async def _aiter_from_lines(lines: list[str]):
|
||||
"""Async iterator that yields the given strings (httpx aiter_lines shape)."""
|
||||
for line in lines:
|
||||
yield line
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_build_stream_sse_fire_and_forget_does_not_read_body() -> None:
|
||||
async def test_build_stream_collects_streamed_log_lines() -> None:
|
||||
"""The streamed plaintext body is consumed line-by-line and returned."""
|
||||
async with DsmClient(base_url="https://nas.local:443") as client:
|
||||
mark_initialized(client)
|
||||
client._http = AsyncMock()
|
||||
|
||||
resp = make_response(
|
||||
{"success": True},
|
||||
content_type="text/event-stream",
|
||||
{"placeholder": True},
|
||||
content_type="text/html",
|
||||
)
|
||||
resp.aiter_lines = lambda: _aiter_from_lines(
|
||||
[
|
||||
"Container vault Running",
|
||||
"Container vault-db Running",
|
||||
"",
|
||||
]
|
||||
)
|
||||
|
||||
# If the code ever tries to read the SSE body, blow up the test.
|
||||
def _boom(*_a: Any, **_k: Any):
|
||||
raise AssertionError("SSE body must not be read")
|
||||
|
||||
resp.aiter_bytes = MagicMock(side_effect=_boom)
|
||||
client._http.stream = MagicMock(return_value=make_stream_ctx(resp))
|
||||
|
||||
result = await client.trigger_build_stream("proj-1")
|
||||
|
||||
assert result is None
|
||||
assert isinstance(result, str)
|
||||
assert "Container vault Running" in result
|
||||
assert "Container vault-db Running" in result
|
||||
# Empty lines are dropped.
|
||||
assert "\n\n" not in result
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_build_stream_collects_failure_log_with_daemon_error() -> None:
|
||||
"""Failure log (svc Error + daemon-error line) is returned verbatim."""
|
||||
async with DsmClient(base_url="https://nas.local:443") as client:
|
||||
mark_initialized(client)
|
||||
client._http = AsyncMock()
|
||||
|
||||
resp = make_response({"placeholder": True}, content_type="text/html")
|
||||
resp.aiter_lines = lambda: _aiter_from_lines(
|
||||
[
|
||||
"nginx Error",
|
||||
(
|
||||
"Error response from daemon: manifest for nginx:9.9.9-nonexistent "
|
||||
"not found: manifest unknown"
|
||||
),
|
||||
]
|
||||
)
|
||||
client._http.stream = MagicMock(return_value=make_stream_ctx(resp))
|
||||
|
||||
result = await client.trigger_build_stream("proj-1")
|
||||
|
||||
assert "nginx Error" in result
|
||||
assert "Error response from daemon:" in result
|
||||
assert "manifest unknown" in result
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -532,6 +570,7 @@ async def test_build_stream_json_error_raises_synology_error() -> None:
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_build_stream_json_success_accepted() -> None:
|
||||
"""JSON envelope with success=true → no streamed log, returns empty string."""
|
||||
async with DsmClient(base_url="https://nas.local:443") as client:
|
||||
mark_initialized(client)
|
||||
client._http = AsyncMock()
|
||||
@@ -543,11 +582,12 @@ async def test_build_stream_json_success_accepted() -> None:
|
||||
|
||||
result = await client.trigger_build_stream("proj-1")
|
||||
|
||||
assert result is None
|
||||
assert result == ""
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_build_stream_read_timeout_swallowed() -> None:
|
||||
async def test_build_stream_read_timeout_returns_marker() -> None:
|
||||
"""Header-arrival ReadTimeout returns the timeout marker (build still running)."""
|
||||
async with DsmClient(base_url="https://nas.local:443") as client:
|
||||
mark_initialized(client)
|
||||
client._http = AsyncMock()
|
||||
@@ -560,7 +600,60 @@ async def test_build_stream_read_timeout_swallowed() -> None:
|
||||
|
||||
result = await client.trigger_build_stream("proj-1")
|
||||
|
||||
assert result is None
|
||||
assert result == DsmClient.BUILD_STREAM_TIMEOUT_MARKER
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_build_stream_per_chunk_read_timeout_returns_partial_log() -> None:
|
||||
"""If a streamed chunk times out mid-build, return the partial log + marker."""
|
||||
|
||||
async def aiter_partial_then_timeout():
|
||||
yield "Container vault Pulling"
|
||||
raise httpx.ReadTimeout("chunk timed out")
|
||||
|
||||
async with DsmClient(base_url="https://nas.local:443") as client:
|
||||
mark_initialized(client)
|
||||
client._http = AsyncMock()
|
||||
|
||||
resp = make_response({"placeholder": True}, content_type="text/html")
|
||||
resp.aiter_lines = aiter_partial_then_timeout
|
||||
client._http.stream = MagicMock(return_value=make_stream_ctx(resp))
|
||||
|
||||
result = await client.trigger_build_stream("proj-1")
|
||||
|
||||
assert "Container vault Pulling" in result
|
||||
assert DsmClient.BUILD_STREAM_TIMEOUT_MARKER in result
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_build_stream_wallclock_budget_breaks_loop(monkeypatch) -> None:
|
||||
"""When the wall-clock budget is exhausted, the loop returns partial log + marker."""
|
||||
import mcp_synology_container.dsm_client as dsm_mod
|
||||
|
||||
# Force the budget to a value just below the initial loop.time() so the
|
||||
# check after the first emitted line trips immediately.
|
||||
monkeypatch.setattr(DsmClient, "BUILD_STREAM_BUDGET", -1.0)
|
||||
|
||||
async def aiter_many():
|
||||
yield "Container vault Pulling"
|
||||
yield "Container vault Running" # Should NOT appear in the result.
|
||||
|
||||
async with DsmClient(base_url="https://nas.local:443") as client:
|
||||
mark_initialized(client)
|
||||
client._http = AsyncMock()
|
||||
|
||||
resp = make_response({"placeholder": True}, content_type="text/html")
|
||||
resp.aiter_lines = aiter_many
|
||||
client._http.stream = MagicMock(return_value=make_stream_ctx(resp))
|
||||
|
||||
result = await client.trigger_build_stream("proj-1")
|
||||
|
||||
assert "Container vault Pulling" in result
|
||||
assert "Container vault Running" not in result
|
||||
assert DsmClient.BUILD_STREAM_TIMEOUT_MARKER in result
|
||||
|
||||
# Keep dsm_mod referenced so the test doesn't trip an unused-import warning.
|
||||
assert dsm_mod.DsmClient is DsmClient
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -628,7 +721,7 @@ async def test_build_stream_http_500_scrubs_sid() -> None:
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_build_stream_malformed_json_body_treated_as_accepted() -> None:
|
||||
"""Defensive branch: JSON content-type but body fails to parse → return None."""
|
||||
"""Defensive branch: JSON content-type but body fails to parse → empty string."""
|
||||
async with DsmClient(base_url="https://nas.local:443") as client:
|
||||
mark_initialized(client)
|
||||
client._http = AsyncMock()
|
||||
@@ -639,7 +732,7 @@ async def test_build_stream_malformed_json_body_treated_as_accepted() -> None:
|
||||
|
||||
result = await client.trigger_build_stream("proj-1")
|
||||
|
||||
assert result is None
|
||||
assert result == ""
|
||||
|
||||
|
||||
# ──────────────────────────────────────────────────────────────────────
|
||||
|
||||
@@ -210,6 +210,7 @@ def make_stateful_redeploy_mock(
|
||||
initial_status: str,
|
||||
stop_raises=None,
|
||||
build_stream_raises=None,
|
||||
build_stream_log: str = "",
|
||||
):
|
||||
"""Create a stateful client mock for redeploy tests.
|
||||
|
||||
@@ -235,6 +236,7 @@ def make_stateful_redeploy_mock(
|
||||
if build_stream_raises:
|
||||
raise build_stream_raises
|
||||
build_done = True # After build_stream, polling returns RUNNING
|
||||
return build_stream_log
|
||||
|
||||
client.request.side_effect = mock_request
|
||||
client.trigger_build_stream = AsyncMock(side_effect=mock_trigger_build_stream)
|
||||
@@ -346,6 +348,7 @@ async def test_redeploy_poll_timeout():
|
||||
async def mock_build_stream(project_id):
|
||||
nonlocal build_done
|
||||
build_done = True
|
||||
return ""
|
||||
|
||||
client.request.side_effect = mock_request
|
||||
client.trigger_build_stream = AsyncMock(side_effect=mock_build_stream)
|
||||
@@ -374,7 +377,7 @@ async def test_redeploy_unknown_status_returns_error():
|
||||
return {}
|
||||
|
||||
client.request.side_effect = mock_request
|
||||
client.trigger_build_stream = AsyncMock()
|
||||
client.trigger_build_stream = AsyncMock(return_value="")
|
||||
tools = make_projects_tools(client)
|
||||
|
||||
result = await tools["redeploy_project"]("myapp", confirmed=True)
|
||||
@@ -435,6 +438,111 @@ async def test_redeploy_build_stream_error_on_stopped_project_keeps_old_workarou
|
||||
assert "was stopped before this error" not in result
|
||||
|
||||
|
||||
# ──────────────────────────────────────────────────────────────────────
|
||||
# Issue #2: surface build_stream daemon errors directly
|
||||
# ──────────────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
def test_parse_build_stream_log_extracts_errors_and_info():
|
||||
"""The helper splits a streamed build log into error and info lines."""
|
||||
from mcp_synology_container.modules.projects import _parse_build_stream_log
|
||||
|
||||
log = (
|
||||
"Container vault Pulling\n"
|
||||
"nginx Error\n"
|
||||
"Error response from daemon: manifest for nginx:9.9.9 not found: "
|
||||
"manifest unknown\n"
|
||||
"\n"
|
||||
"Container vault Running\n"
|
||||
)
|
||||
|
||||
errors, info = _parse_build_stream_log(log)
|
||||
|
||||
assert errors == [
|
||||
"nginx Error",
|
||||
("Error response from daemon: manifest for nginx:9.9.9 not found: manifest unknown"),
|
||||
]
|
||||
assert info == ["Container vault Pulling", "Container vault Running"]
|
||||
|
||||
|
||||
def test_parse_build_stream_log_clean_log_no_errors():
|
||||
from mcp_synology_container.modules.projects import _parse_build_stream_log
|
||||
|
||||
log = "Container vault Running\nContainer vault-db Running\n"
|
||||
errors, info = _parse_build_stream_log(log)
|
||||
|
||||
assert errors == []
|
||||
assert info == ["Container vault Running", "Container vault-db Running"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_redeploy_surfaces_build_stream_daemon_error():
|
||||
"""build_stream log containing 'Error response from daemon:' aborts redeploy
|
||||
with that line visible — and skips the polling step entirely."""
|
||||
client, calls = make_stateful_redeploy_mock(
|
||||
"RUNNING",
|
||||
build_stream_log=(
|
||||
"nginx Error\n"
|
||||
"Error response from daemon: manifest for nginx:9.9.9-nonexistent "
|
||||
"not found: manifest unknown\n"
|
||||
),
|
||||
)
|
||||
tools = make_projects_tools(client)
|
||||
|
||||
with patch("mcp_synology_container.modules.projects.asyncio.sleep"):
|
||||
result = await tools["redeploy_project"]("myapp", confirmed=True)
|
||||
|
||||
assert "Build failed" in result
|
||||
assert "Error response from daemon: manifest for nginx:9.9.9-nonexistent" in result
|
||||
assert "redeploy aborted" in result
|
||||
assert "redeployed successfully" not in result
|
||||
|
||||
# Recovery hint must appear because the project was stopped first.
|
||||
assert "was stopped before this error" in result
|
||||
|
||||
# The polling step must NOT run — build_stream errors short-circuit.
|
||||
methods = [m for _, m in calls]
|
||||
list_calls = [m for m in methods if m == "list"]
|
||||
assert len(list_calls) == 1 # only the initial _find_project lookup
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_redeploy_clean_log_proceeds_to_polling():
|
||||
"""A clean build_stream log keeps the original happy-path behavior."""
|
||||
client, calls = make_stateful_redeploy_mock(
|
||||
"RUNNING",
|
||||
build_stream_log="Container myapp Pulling\nContainer myapp Running\n",
|
||||
)
|
||||
tools = make_projects_tools(client)
|
||||
|
||||
with patch("mcp_synology_container.modules.projects.asyncio.sleep"):
|
||||
result = await tools["redeploy_project"]("myapp", confirmed=True)
|
||||
|
||||
assert "redeployed successfully" in result
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_create_project_surfaces_build_stream_daemon_error():
|
||||
"""build_stream log with daemon error → registered-but-failed-to-build hint."""
|
||||
client, calls = make_create_project_client(
|
||||
build_stream_log=(
|
||||
"web Error\n"
|
||||
"Error response from daemon: manifest for nginx:0.0.0-bad not found: "
|
||||
"manifest unknown\n"
|
||||
),
|
||||
)
|
||||
tools = make_projects_tools(client)
|
||||
|
||||
with patch("mcp_synology_container.modules.projects.asyncio.sleep"):
|
||||
result = await tools["create_project"]("newapp", SIMPLE_COMPOSE, confirmed=True)
|
||||
|
||||
assert "Build failed" in result
|
||||
assert "Error response from daemon: manifest for nginx:0.0.0-bad" in result
|
||||
assert "is registered but failed to build" in result
|
||||
assert "redeploy_project" in result
|
||||
assert "created and started successfully" not in result
|
||||
|
||||
|
||||
# ──────────────────────────────────────────────────────────────────────
|
||||
# M-5: polling exits early on BUILD_FAILED / ERROR
|
||||
# ──────────────────────────────────────────────────────────────────────
|
||||
@@ -501,6 +609,7 @@ async def test_redeploy_surfaces_build_failed_with_hint():
|
||||
async def mock_build_stream(project_id):
|
||||
nonlocal build_done
|
||||
build_done = True
|
||||
return "" # Clean stream log — failure surfaces via polling.
|
||||
|
||||
client.request.side_effect = mock_request
|
||||
client.trigger_build_stream = AsyncMock(side_effect=mock_build_stream)
|
||||
@@ -539,6 +648,7 @@ def make_create_project_client(
|
||||
create_folder_raises: Exception | None = None,
|
||||
create_project_raises: Exception | None = None,
|
||||
build_stream_raises: Exception | None = None,
|
||||
build_stream_log: str = "",
|
||||
project_id: str = "uuid-new",
|
||||
final_status: str = "RUNNING",
|
||||
):
|
||||
@@ -588,6 +698,7 @@ def make_create_project_client(
|
||||
calls.append(("SYNO.Docker.Project", "build_stream", {"id": pid}))
|
||||
if build_stream_raises:
|
||||
raise build_stream_raises
|
||||
return build_stream_log
|
||||
|
||||
client.request.side_effect = mock_request
|
||||
client.post_request.side_effect = mock_post_request
|
||||
|
||||
Reference in New Issue
Block a user