fix: retry _poll_task on transient 599 instead of aborting immediately
DirSize for large directories (e.g. /docker, 8441 folders, 46832 files) takes ~800ms to compute. While running, status returns intermediate progress (finished=false). But on the very first poll the task can return 599 transiently (task just started, not yet available). Previously _poll_task caught any SynologyError and returned immediately, making dir_size always fail on the first 599. Fix: treat 599 as a transient condition and continue polling. Give up only after 5 consecutive 599 responses. All other error codes remain immediately fatal. Investigation confirmed with test_dirsize_md5.py: - /test-mcp (2937 B): finished=true at 0ms - /docker (3.9 GB, 46832 files): finished=false at 35ms, finished=true at 789ms Tests: 2 new cases (retry-succeeds, 5x-599-gives-up) → 95 total Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
+1
-1
@@ -1,6 +1,6 @@
|
|||||||
[project]
|
[project]
|
||||||
name = "mcp-synology-filestation"
|
name = "mcp-synology-filestation"
|
||||||
version = "0.2.1"
|
version = "0.2.2"
|
||||||
description = "MCP server for Synology FileStation"
|
description = "MCP server for Synology FileStation"
|
||||||
requires-python = ">=3.12"
|
requires-python = ">=3.12"
|
||||||
dependencies = [
|
dependencies = [
|
||||||
|
|||||||
@@ -1,3 +1,3 @@
|
|||||||
"""MCP server for Synology FileStation."""
|
"""MCP server for Synology FileStation."""
|
||||||
|
|
||||||
__version__ = "0.2.1"
|
__version__ = "0.2.2"
|
||||||
|
|||||||
@@ -86,6 +86,7 @@ def register_filestation(
|
|||||||
delay = 0.2
|
delay = 0.2
|
||||||
elapsed = initial_delay
|
elapsed = initial_delay
|
||||||
timeout = 60.0
|
timeout = 60.0
|
||||||
|
consecutive_599 = 0
|
||||||
|
|
||||||
if initial_delay > 0:
|
if initial_delay > 0:
|
||||||
await asyncio.sleep(initial_delay)
|
await asyncio.sleep(initial_delay)
|
||||||
@@ -98,11 +99,19 @@ def register_filestation(
|
|||||||
version=version,
|
version=version,
|
||||||
params={"taskid": taskid},
|
params={"taskid": taskid},
|
||||||
)
|
)
|
||||||
|
consecutive_599 = 0
|
||||||
except _SynologyError as e:
|
except _SynologyError as e:
|
||||||
return False, f"Error: {e}"
|
if e.code == 599:
|
||||||
|
# 599 can be transient (task just started, not yet available).
|
||||||
if status_data.get("finished"):
|
# Retry up to 5 times before giving up.
|
||||||
return True, status_data
|
consecutive_599 += 1
|
||||||
|
if consecutive_599 >= 5:
|
||||||
|
return False, f"Error: {e}"
|
||||||
|
else:
|
||||||
|
return False, f"Error: {e}"
|
||||||
|
else:
|
||||||
|
if status_data.get("finished"):
|
||||||
|
return True, status_data
|
||||||
|
|
||||||
if elapsed >= timeout:
|
if elapsed >= timeout:
|
||||||
return (
|
return (
|
||||||
|
|||||||
+55
-98
@@ -1,19 +1,5 @@
|
|||||||
"""Wegwerfskript: DirSize + MD5 direkt gegen die NAS testen.
|
"""Wegwerfskript: DirSize + MD5 direkt gegen die NAS testen.
|
||||||
|
|
||||||
Finale Befunde:
|
|
||||||
DirSize: start v2 (path als plain string ODER JSON-Array), status v1 (0ms delay)
|
|
||||||
MD5: start v2, status v1 (0ms delay, ONE-SHOT)
|
|
||||||
|
|
||||||
DirSize status-Response Felder:
|
|
||||||
finished: bool
|
|
||||||
num_dir: int (Anzahl Unterordner)
|
|
||||||
num_file: int (Anzahl Dateien)
|
|
||||||
total_size: int (Gesamtgroesse in Bytes)
|
|
||||||
|
|
||||||
MD5 status-Response Felder:
|
|
||||||
finished: bool
|
|
||||||
md5: str (Hex-String, 32 Zeichen)
|
|
||||||
|
|
||||||
Ausfuehren: uv run python test_dirsize_md5.py
|
Ausfuehren: uv run python test_dirsize_md5.py
|
||||||
"""
|
"""
|
||||||
|
|
||||||
@@ -27,13 +13,13 @@ from mcp_synology_filestation.auth import AuthManager
|
|||||||
from mcp_synology_filestation.client import FileStationClient
|
from mcp_synology_filestation.client import FileStationClient
|
||||||
from mcp_synology_filestation.config import load_config
|
from mcp_synology_filestation.config import load_config
|
||||||
|
|
||||||
DIRSIZE_PATH = "/test-mcp"
|
DIRSIZE_PATHS = ["/test-mcp", "/docker"]
|
||||||
MD5_PATH = "/test-mcp/test.zip"
|
MD5_PATH = "/test-mcp/test.zip"
|
||||||
|
|
||||||
|
|
||||||
def pp(label: str, data: object, elapsed_ms: float | None = None) -> None:
|
def pp(label: str, data: object, elapsed_ms: float | None = None) -> None:
|
||||||
print(f"\n{'='*60}")
|
print(f"\n{'='*60}")
|
||||||
suffix = f" [{elapsed_ms:.1f} ms nach start]" if elapsed_ms is not None else ""
|
suffix = f" [{elapsed_ms:.1f} ms]" if elapsed_ms is not None else ""
|
||||||
print(f" {label}{suffix}")
|
print(f" {label}{suffix}")
|
||||||
print("=" * 60)
|
print("=" * 60)
|
||||||
print(json.dumps(data, indent=2, ensure_ascii=False))
|
print(json.dumps(data, indent=2, ensure_ascii=False))
|
||||||
@@ -48,87 +34,67 @@ async def raw(http: httpx.AsyncClient, url: str, sid: str, **params) -> dict:
|
|||||||
return {"_raw": r.text[:300], "_http_status": r.status_code}
|
return {"_raw": r.text[:300], "_http_status": r.status_code}
|
||||||
|
|
||||||
|
|
||||||
async def probe_dirsize_path_variants(
|
async def probe_dirsize_long(
|
||||||
http: httpx.AsyncClient, sid: str, api_url: str
|
http: httpx.AsyncClient, sid: str, api_url: str, path: str
|
||||||
) -> None:
|
) -> None:
|
||||||
"""Test all three path encoding variants for DirSize start."""
|
"""Start DirSize and poll v1 every 200ms for up to 15s.
|
||||||
|
|
||||||
|
Goal: find out if 599 means 'task still running' (keep polling)
|
||||||
|
or 'task gone' (give up). If the task eventually returns data,
|
||||||
|
599 = 'not ready yet'. If it never returns data, 599 = 'task gone'.
|
||||||
|
"""
|
||||||
print(f"\n{'#'*60}")
|
print(f"\n{'#'*60}")
|
||||||
print(" DIRSIZE path-Varianten (a=plain, b=json.dumps, c=manuell)")
|
print(f" DIRSIZE {path} — long poll (15s, every 200ms)")
|
||||||
print(f"{'#'*60}")
|
|
||||||
|
|
||||||
variants = [
|
|
||||||
("a) plain string", DIRSIZE_PATH),
|
|
||||||
("b) json.dumps([path])", json.dumps([DIRSIZE_PATH])),
|
|
||||||
("c) json.dumps(path)", json.dumps(DIRSIZE_PATH)),
|
|
||||||
]
|
|
||||||
|
|
||||||
for label, path_val in variants:
|
|
||||||
print(f"\n--- Variante {label} ---")
|
|
||||||
print(f" path={path_val!r}")
|
|
||||||
|
|
||||||
t0 = time.perf_counter()
|
|
||||||
start_body = await raw(
|
|
||||||
http, api_url, sid,
|
|
||||||
api="SYNO.FileStation.DirSize", version="2", method="start",
|
|
||||||
path=path_val,
|
|
||||||
)
|
|
||||||
pp(f"DirSize::start [{label}]", start_body, (time.perf_counter() - t0) * 1000)
|
|
||||||
|
|
||||||
taskid = (start_body.get("data") or {}).get("taskid")
|
|
||||||
if not taskid:
|
|
||||||
print(" => Kein taskid (start fehlgeschlagen)")
|
|
||||||
continue
|
|
||||||
|
|
||||||
# Poll immediately
|
|
||||||
t1 = time.perf_counter()
|
|
||||||
status_body = await raw(
|
|
||||||
http, api_url, sid,
|
|
||||||
api="SYNO.FileStation.DirSize", version="1", method="status",
|
|
||||||
taskid=taskid,
|
|
||||||
)
|
|
||||||
pp(f"DirSize::status [{label}]", status_body, (t1 - t0) * 1000)
|
|
||||||
|
|
||||||
data = (status_body.get("data") or {})
|
|
||||||
if data.get("finished"):
|
|
||||||
print(f" => OK: num_dir={data.get('num_dir')} "
|
|
||||||
f"num_file={data.get('num_file')} "
|
|
||||||
f"total_size={data.get('total_size')}")
|
|
||||||
else:
|
|
||||||
code = (status_body.get("error") or {}).get("code", "?")
|
|
||||||
print(f" => FEHLER code={code}")
|
|
||||||
|
|
||||||
|
|
||||||
async def probe_md5(http: httpx.AsyncClient, sid: str, api_url: str) -> None:
|
|
||||||
"""Test MD5 with status v1 at 0ms (correct settings)."""
|
|
||||||
print(f"\n{'#'*60}")
|
|
||||||
print(" MD5 -- start v2, status v1, 0ms delay")
|
|
||||||
print(f"{'#'*60}")
|
print(f"{'#'*60}")
|
||||||
|
|
||||||
t0 = time.perf_counter()
|
t0 = time.perf_counter()
|
||||||
start_body = await raw(
|
start_body = await raw(
|
||||||
http, api_url, sid,
|
http, api_url, sid,
|
||||||
api="SYNO.FileStation.MD5", version="2", method="start",
|
api="SYNO.FileStation.DirSize", version="2", method="start",
|
||||||
file_path=json.dumps(MD5_PATH),
|
path=json.dumps([path]),
|
||||||
)
|
)
|
||||||
pp("MD5::start", start_body, (time.perf_counter() - t0) * 1000)
|
elapsed_start = (time.perf_counter() - t0) * 1000
|
||||||
|
pp(f"DirSize::start ({path})", start_body, elapsed_start)
|
||||||
|
|
||||||
taskid = (start_body.get("data") or {}).get("taskid")
|
taskid = (start_body.get("data") or {}).get("taskid")
|
||||||
if not taskid:
|
if not taskid:
|
||||||
print("[!] No taskid")
|
print("[!] No taskid.")
|
||||||
return
|
return
|
||||||
|
|
||||||
t1 = time.perf_counter()
|
print(f"\n[*] Polling status v1 every 200ms for up to 15s (taskid={taskid[:12]}...)")
|
||||||
r = await raw(
|
for attempt in range(75): # 75 * 200ms = 15s
|
||||||
http, api_url, sid,
|
if attempt > 0:
|
||||||
api="SYNO.FileStation.MD5", version="1", method="status",
|
await asyncio.sleep(0.2)
|
||||||
taskid=taskid,
|
t = time.perf_counter()
|
||||||
)
|
r = await raw(
|
||||||
pp("MD5::status v1 [0ms]", r, (t1 - t0) * 1000)
|
http, api_url, sid,
|
||||||
data = (r.get("data") or {})
|
api="SYNO.FileStation.DirSize", version="1", method="status",
|
||||||
if data.get("finished"):
|
taskid=taskid,
|
||||||
print(f" => OK: md5={data.get('md5')}")
|
)
|
||||||
else:
|
elapsed = (t - t0) * 1000
|
||||||
print(f" => FEHLER: {r}")
|
success = r.get("success")
|
||||||
|
data = (r.get("data") or {})
|
||||||
|
finished = data.get("finished")
|
||||||
|
error_code = (r.get("error") or {}).get("code")
|
||||||
|
|
||||||
|
if finished:
|
||||||
|
print(f" [{elapsed:.0f}ms] attempt {attempt+1}: FERTIG! "
|
||||||
|
f"num_dir={data.get('num_dir')} "
|
||||||
|
f"num_file={data.get('num_file')} "
|
||||||
|
f"total_size={data.get('total_size')}")
|
||||||
|
pp(f"DirSize::status final ({path})", r, elapsed)
|
||||||
|
return
|
||||||
|
elif success and not finished:
|
||||||
|
# Still running — show current progress
|
||||||
|
print(f" [{elapsed:.0f}ms] attempt {attempt+1}: running... "
|
||||||
|
f"num_dir={data.get('num_dir', '?')} "
|
||||||
|
f"num_file={data.get('num_file', '?')} "
|
||||||
|
f"total_size={data.get('total_size', '?')}")
|
||||||
|
else:
|
||||||
|
print(f" [{elapsed:.0f}ms] attempt {attempt+1}: error code={error_code}")
|
||||||
|
# Continue polling — 599 might mean 'not ready yet'
|
||||||
|
|
||||||
|
print(f"\n[!] No result after 15s — task never returned data.")
|
||||||
|
|
||||||
|
|
||||||
async def main() -> None:
|
async def main() -> None:
|
||||||
@@ -141,22 +107,13 @@ async def main() -> None:
|
|||||||
sid = client.sid
|
sid = client.sid
|
||||||
base = config.base_url
|
base = config.base_url
|
||||||
|
|
||||||
for api_name in ["SYNO.FileStation.DirSize", "SYNO.FileStation.MD5"]:
|
info = client._api_cache.get("SYNO.FileStation.DirSize", {}) # noqa: SLF001
|
||||||
info = client._api_cache.get(api_name) # noqa: SLF001
|
api_url = f"{base}/webapi/{info.get('path', 'entry.cgi')}"
|
||||||
if info:
|
print(f"[*] DirSize API: {api_url} v{info.get('minVersion')}-v{info.get('maxVersion')}")
|
||||||
print(f"[*] {api_name}: path={info['path']} "
|
|
||||||
f"v{info['minVersion']}-v{info['maxVersion']}")
|
|
||||||
else:
|
|
||||||
print(f"[!] {api_name}: NOT in API cache!")
|
|
||||||
|
|
||||||
dirsize_info = client._api_cache.get("SYNO.FileStation.DirSize", {}) # noqa: SLF001
|
|
||||||
md5_info = client._api_cache.get("SYNO.FileStation.MD5", {}) # noqa: SLF001
|
|
||||||
dirsize_url = f"{base}/webapi/{dirsize_info.get('path', 'entry.cgi')}"
|
|
||||||
md5_url = f"{base}/webapi/{md5_info.get('path', 'entry.cgi')}"
|
|
||||||
|
|
||||||
async with httpx.AsyncClient(verify=config.connection.verify_ssl, timeout=30.0) as http:
|
async with httpx.AsyncClient(verify=config.connection.verify_ssl, timeout=30.0) as http:
|
||||||
await probe_dirsize_path_variants(http, sid, dirsize_url)
|
for path in DIRSIZE_PATHS:
|
||||||
await probe_md5(http, sid, md5_url)
|
await probe_dirsize_long(http, sid, api_url, path)
|
||||||
|
|
||||||
await auth.logout(client)
|
await auth.logout(client)
|
||||||
print("\n[*] Logout OK.")
|
print("\n[*] Logout OK.")
|
||||||
|
|||||||
@@ -1610,6 +1610,49 @@ async def test_dir_size_empty_path(config: AppConfig) -> None:
|
|||||||
client.request.assert_not_called()
|
client.request.assert_not_called()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_dir_size_retries_on_transient_599(config: AppConfig) -> None:
|
||||||
|
"""dir_size retries up to 4 times on code-599 then succeeds on 5th status call."""
|
||||||
|
client = MagicMock()
|
||||||
|
call_count = {"status": 0}
|
||||||
|
|
||||||
|
async def _request(api, method, version=None, params=None, **kwargs):
|
||||||
|
if method == "start":
|
||||||
|
return {"taskid": "FileStation_dirsize_599"}
|
||||||
|
call_count["status"] += 1
|
||||||
|
if call_count["status"] < 4:
|
||||||
|
raise SynologyError("DSM error code 599", code=599)
|
||||||
|
return {"finished": True, "num_dir": 2, "num_file": 10, "total_size": 1024}
|
||||||
|
|
||||||
|
client.request = AsyncMock(side_effect=_request)
|
||||||
|
tools = _make_mcp_and_tools(config, client)
|
||||||
|
|
||||||
|
with patch("asyncio.sleep", new_callable=AsyncMock):
|
||||||
|
result = await tools["dir_size"](path="/data")
|
||||||
|
|
||||||
|
assert "Total Size" in result
|
||||||
|
assert call_count["status"] == 4
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_dir_size_fails_after_5_consecutive_599(config: AppConfig) -> None:
|
||||||
|
"""dir_size gives up and returns Error: after 5 consecutive 599 responses."""
|
||||||
|
client = MagicMock()
|
||||||
|
|
||||||
|
async def _request(api, method, version=None, params=None, **kwargs):
|
||||||
|
if method == "start":
|
||||||
|
return {"taskid": "FileStation_dirsize_dead"}
|
||||||
|
raise SynologyError("DSM error code 599", code=599)
|
||||||
|
|
||||||
|
client.request = AsyncMock(side_effect=_request)
|
||||||
|
tools = _make_mcp_and_tools(config, client)
|
||||||
|
|
||||||
|
with patch("asyncio.sleep", new_callable=AsyncMock):
|
||||||
|
result = await tools["dir_size"](path="/dead")
|
||||||
|
|
||||||
|
assert result.startswith("Error:")
|
||||||
|
|
||||||
|
|
||||||
# ──────────────────────────────────────────────────────────────────────────
|
# ──────────────────────────────────────────────────────────────────────────
|
||||||
# get_md5
|
# get_md5
|
||||||
# ──────────────────────────────────────────────────────────────────────────
|
# ──────────────────────────────────────────────────────────────────────────
|
||||||
|
|||||||
Reference in New Issue
Block a user