mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-29 18:46:59 +00:00
Preserve Photon sidecar retry semantics
Photon's Node sidecar intentionally hides raw handler exceptions, but the Python adapter still needs a safe failure class and retryability bit so delivery retries do not collapse into an opaque generic 500. Constraint: Sidecar responses must not leak raw stack traces or private exception text Rejected: Retry every internal sidecar error | masks permanent auth/config failures Confidence: high Scope-risk: narrow Directive: Keep sidecar error text generic; extend safe error classes instead of exposing raw SDK failures Tested: uv run --with pytest-timeout pytest tests/plugins/platforms/photon/test_overflow_recovery.py -q Tested: uv run --with pytest-timeout pytest tests/plugins/platforms/photon -q Tested: uv run ruff check plugins/platforms/photon/adapter.py tests/plugins/platforms/photon/test_overflow_recovery.py Tested: python3 -m py_compile plugins/platforms/photon/adapter.py tests/plugins/platforms/photon/test_overflow_recovery.py Tested: node --check plugins/platforms/photon/sidecar/index.mjs Tested: git diff --check Tested: python3 scripts/check-windows-footguns.py --diff origin/main Not-tested: Live Photon/Spectrum delivery against a real iMessage account Related: #50971
This commit is contained in:
parent
c0ff5e9169
commit
91c4c6f9d2
3 changed files with 259 additions and 11 deletions
|
|
@ -223,6 +223,53 @@ _DEFAULT_MENTION_PATTERNS = [
|
|||
# ---------------------------------------------------------------------------
|
||||
# Module-level helpers — also used by check_fn / standalone send
|
||||
|
||||
class PhotonSidecarError(RuntimeError):
|
||||
"""Structured failure returned by the supervised Photon sidecar."""
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
*,
|
||||
path: str,
|
||||
status_code: int,
|
||||
error: str,
|
||||
error_class: str = "sidecar_error",
|
||||
retryable: bool = False,
|
||||
) -> None:
|
||||
self.path = path
|
||||
self.status_code = status_code
|
||||
self.error = error
|
||||
self.error_class = error_class
|
||||
self.retryable = retryable
|
||||
super().__init__(
|
||||
f"Photon sidecar {path} returned {status_code} "
|
||||
f"({error_class}, retryable={retryable}): {error}"
|
||||
)
|
||||
|
||||
|
||||
def _sidecar_error_from_response(
|
||||
path: str,
|
||||
status_code: int,
|
||||
text: str,
|
||||
data: Optional[Dict[str, Any]] = None,
|
||||
) -> PhotonSidecarError:
|
||||
if data is None:
|
||||
try:
|
||||
parsed = json.loads(text)
|
||||
except Exception:
|
||||
parsed = {}
|
||||
data = parsed if isinstance(parsed, dict) else {}
|
||||
|
||||
error = str(data.get("error") or text[:200] or "sidecar error")
|
||||
error_class = str(data.get("error_class") or "sidecar_error")
|
||||
retryable = bool(data.get("retryable"))
|
||||
return PhotonSidecarError(
|
||||
path=path,
|
||||
status_code=status_code,
|
||||
error=error,
|
||||
error_class=error_class,
|
||||
retryable=retryable,
|
||||
)
|
||||
|
||||
def _coerce_port(value: Any, default: int) -> int:
|
||||
try:
|
||||
return int(value)
|
||||
|
|
@ -1701,6 +1748,8 @@ class PhotonAdapter(BasePlatformAdapter):
|
|||
if not error:
|
||||
return False
|
||||
lowered = error.lower()
|
||||
if "retryable=false" in lowered or "auth_or_config" in lowered:
|
||||
return False
|
||||
return any(pat in lowered for pat in _PHOTON_RETRYABLE_PATTERNS)
|
||||
|
||||
async def _send_with_retry(
|
||||
|
|
@ -1728,6 +1777,14 @@ class PhotonAdapter(BasePlatformAdapter):
|
|||
if result.success:
|
||||
return result
|
||||
|
||||
sidecar_failure = result.raw_response
|
||||
if (
|
||||
isinstance(sidecar_failure, dict)
|
||||
and sidecar_failure.get("error_class") == "auth_or_config"
|
||||
and sidecar_failure.get("retryable") is False
|
||||
):
|
||||
return result
|
||||
|
||||
error_str = result.error or ""
|
||||
is_network = result.retryable or self._is_retryable_error(error_str)
|
||||
if not is_network and self._is_timeout_error(error_str):
|
||||
|
|
@ -1787,6 +1844,16 @@ class PhotonAdapter(BasePlatformAdapter):
|
|||
body["format"] = "markdown"
|
||||
try:
|
||||
data = await self._sidecar_call("/send", body)
|
||||
except PhotonSidecarError as e:
|
||||
return SendResult(
|
||||
success=False,
|
||||
error=str(e),
|
||||
raw_response={
|
||||
"error_class": e.error_class,
|
||||
"retryable": e.retryable,
|
||||
},
|
||||
retryable=e.retryable,
|
||||
)
|
||||
except Exception as e:
|
||||
return SendResult(success=False, error=str(e))
|
||||
self._record_sent_message(data.get("messageId"))
|
||||
|
|
@ -1836,6 +1903,16 @@ class PhotonAdapter(BasePlatformAdapter):
|
|||
body["caption"] = caption
|
||||
try:
|
||||
data = await self._sidecar_call("/send-attachment", body)
|
||||
except PhotonSidecarError as e:
|
||||
return SendResult(
|
||||
success=False,
|
||||
error=str(e),
|
||||
raw_response={
|
||||
"error_class": e.error_class,
|
||||
"retryable": e.retryable,
|
||||
},
|
||||
retryable=e.retryable,
|
||||
)
|
||||
except Exception as e:
|
||||
return SendResult(success=False, error=str(e))
|
||||
self._record_sent_message(data.get("messageId"))
|
||||
|
|
@ -1855,14 +1932,10 @@ class PhotonAdapter(BasePlatformAdapter):
|
|||
async with httpx.AsyncClient(timeout=30.0, trust_env=False) as client:
|
||||
resp = await client.post(url, json=body, headers=headers)
|
||||
if resp.status_code != 200:
|
||||
raise RuntimeError(
|
||||
f"Photon sidecar {path} returned {resp.status_code}: {resp.text[:200]}"
|
||||
)
|
||||
raise _sidecar_error_from_response(path, resp.status_code, resp.text)
|
||||
data = resp.json() or {}
|
||||
if not data.get("ok"):
|
||||
raise RuntimeError(
|
||||
f"Photon sidecar {path} reported error: {data.get('error')}"
|
||||
)
|
||||
raise _sidecar_error_from_response(path, resp.status_code, resp.text, data)
|
||||
return data
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -596,13 +596,56 @@ function badRequest(res, msg) {
|
|||
res.end(JSON.stringify({ ok: false, error: msg }));
|
||||
}
|
||||
|
||||
function serverError(res) {
|
||||
function classifySidecarError(err) {
|
||||
const message = String(err && err.message ? err.message : err || "");
|
||||
const lowered = message.toLowerCase();
|
||||
|
||||
if (
|
||||
lowered.includes("unauthorized") ||
|
||||
lowered.includes("forbidden") ||
|
||||
lowered.includes("permission") ||
|
||||
lowered.includes("401") ||
|
||||
lowered.includes("403") ||
|
||||
lowered.includes("invalid token") ||
|
||||
lowered.includes("project secret") ||
|
||||
lowered.includes("unable to resolve space id")
|
||||
) {
|
||||
return { errorClass: "auth_or_config", retryable: false };
|
||||
}
|
||||
|
||||
if (
|
||||
lowered.includes("timeout") ||
|
||||
lowered.includes("timed out") ||
|
||||
lowered.includes("econnreset") ||
|
||||
lowered.includes("econnrefused") ||
|
||||
lowered.includes("socket hang up") ||
|
||||
lowered.includes("fetch failed") ||
|
||||
lowered.includes("unavailable") ||
|
||||
lowered.includes("upstream connect") ||
|
||||
lowered.includes("reset reason") ||
|
||||
lowered.includes("overflow") ||
|
||||
lowered.includes("temporarily") ||
|
||||
lowered.includes("429")
|
||||
) {
|
||||
return { errorClass: "upstream_transient", retryable: true };
|
||||
}
|
||||
|
||||
return { errorClass: "sidecar_internal", retryable: false };
|
||||
}
|
||||
|
||||
function serverError(res, err) {
|
||||
res.statusCode = 500;
|
||||
res.setHeader("Content-Type", "application/json");
|
||||
const classified = classifySidecarError(err);
|
||||
// Don't leak stack traces or raw exception text to the caller — even
|
||||
// though we listen on loopback, the supervisor logs the real error
|
||||
// and the client only needs a generic failure signal.
|
||||
res.end(JSON.stringify({ ok: false, error: "internal sidecar error" }));
|
||||
// though we listen on loopback, the supervisor logs the real error. The
|
||||
// Python adapter only needs a safe class plus retryability.
|
||||
res.end(JSON.stringify({
|
||||
ok: false,
|
||||
error: "internal sidecar error",
|
||||
error_class: classified.errorClass,
|
||||
retryable: classified.retryable,
|
||||
}));
|
||||
}
|
||||
|
||||
function ok(res, data) {
|
||||
|
|
@ -847,7 +890,7 @@ const server = http.createServer(async (req, res) => {
|
|||
);
|
||||
// serverError() intentionally returns a generic message — see its
|
||||
// body for the rationale.
|
||||
return serverError(res);
|
||||
return serverError(res, e);
|
||||
}
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -25,6 +25,8 @@ from typing import Any, Dict
|
|||
import pytest
|
||||
|
||||
from gateway.config import PlatformConfig
|
||||
from gateway.platforms.base import SendResult
|
||||
from plugins.platforms.photon import adapter as photon_adapter
|
||||
from plugins.platforms.photon.adapter import PhotonAdapter
|
||||
|
||||
|
||||
|
|
@ -63,6 +65,136 @@ def test_base_network_patterns_still_match() -> None:
|
|||
assert PhotonAdapter._is_retryable_error("ConnectError: connection refused") is True
|
||||
|
||||
|
||||
def test_structured_non_retryable_sidecar_error_not_legacy_retried() -> None:
|
||||
error = str(
|
||||
photon_adapter.PhotonSidecarError(
|
||||
path="/send",
|
||||
status_code=500,
|
||||
error="internal sidecar error",
|
||||
error_class="auth_or_config",
|
||||
retryable=False,
|
||||
)
|
||||
)
|
||||
|
||||
assert PhotonAdapter._is_retryable_error(error) is False
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_structured_sidecar_retryable_error_preserved(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
adapter = _make_adapter(monkeypatch)
|
||||
adapter._http_client = object()
|
||||
adapter._sidecar_bind = "127.0.0.1"
|
||||
adapter._sidecar_port = 43210
|
||||
adapter._sidecar_token = "token"
|
||||
|
||||
class _Resp:
|
||||
status_code = 500
|
||||
text = (
|
||||
'{"ok":false,"error":"temporary upstream failure",'
|
||||
'"error_class":"upstream_transient","retryable":true}'
|
||||
)
|
||||
|
||||
@staticmethod
|
||||
def json() -> Dict[str, Any]:
|
||||
return {
|
||||
"ok": False,
|
||||
"error": "temporary upstream failure",
|
||||
"error_class": "upstream_transient",
|
||||
"retryable": True,
|
||||
}
|
||||
|
||||
class _FakeClient:
|
||||
def __init__(self, *a: Any, **k: Any) -> None:
|
||||
pass
|
||||
|
||||
async def __aenter__(self) -> "_FakeClient":
|
||||
return self
|
||||
|
||||
async def __aexit__(self, *a: Any) -> bool:
|
||||
return False
|
||||
|
||||
async def post(self, *a: Any, **k: Any) -> _Resp:
|
||||
return _Resp()
|
||||
|
||||
monkeypatch.setattr(photon_adapter.httpx, "AsyncClient", _FakeClient)
|
||||
|
||||
result = await adapter._sidecar_send("space-1", "hello")
|
||||
|
||||
assert result.success is False
|
||||
assert result.retryable is True
|
||||
assert "upstream_transient" in (result.error or "")
|
||||
assert "temporary upstream failure" in (result.error or "")
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_with_retry_uses_structured_retryable_flag(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
adapter = _make_adapter(monkeypatch)
|
||||
calls = 0
|
||||
sleeps: list[float] = []
|
||||
|
||||
async def _fake_sleep(delay: float) -> None:
|
||||
sleeps.append(delay)
|
||||
|
||||
async def _fake_sidecar_call(path: str, body: Dict[str, Any]) -> Dict[str, Any]:
|
||||
nonlocal calls
|
||||
calls += 1
|
||||
if calls == 1:
|
||||
raise photon_adapter.PhotonSidecarError(
|
||||
path=path,
|
||||
status_code=500,
|
||||
error="temporary upstream failure",
|
||||
error_class="upstream_transient",
|
||||
retryable=True,
|
||||
)
|
||||
return {"ok": True, "messageId": "m-2"}
|
||||
|
||||
monkeypatch.setattr(photon_adapter.asyncio, "sleep", _fake_sleep)
|
||||
monkeypatch.setattr(adapter, "_sidecar_call", _fake_sidecar_call)
|
||||
|
||||
result = await adapter._send_with_retry(
|
||||
"space-1", "hello", max_retries=1, base_delay=0.25
|
||||
)
|
||||
|
||||
assert result.success is True
|
||||
assert result.message_id == "m-2"
|
||||
assert calls == 2
|
||||
assert sleeps == [0.25]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_with_retry_does_not_fallback_after_auth_or_config_error(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
adapter = _make_adapter(monkeypatch)
|
||||
calls = 0
|
||||
|
||||
async def _permanent_failure(**kwargs: Any) -> SendResult:
|
||||
nonlocal calls
|
||||
calls += 1
|
||||
return SendResult(
|
||||
success=False,
|
||||
error="Photon sidecar /send returned 500 (auth_or_config)",
|
||||
raw_response={
|
||||
"error_class": "auth_or_config",
|
||||
"retryable": False,
|
||||
},
|
||||
retryable=False,
|
||||
)
|
||||
|
||||
monkeypatch.setattr(adapter, "send", _permanent_failure)
|
||||
|
||||
result = await adapter._send_with_retry(
|
||||
"space-1", "hello", max_retries=2, base_delay=0
|
||||
)
|
||||
|
||||
assert result.success is False
|
||||
assert calls == 1
|
||||
|
||||
|
||||
# -- Gap 2: typing-indicator cooldown ---------------------------------------
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue