fix(llm): surface llm-connect's error body instead of a bare 502
llm-connect maps every provider API error onto HTTP 502 and puts the real cause in the body (llm_connect/server.py::_error_response: error, provider_status). LLMConnectClient.complete called raise_for_status() and threw that body away, so a revoked OpenRouter key was indistinguishable from a downed gateway — four days of production evidence read as "llm-connect is down". Live check confirms one fault, not two: the llm-connect pod is Running 1/1 with healthy endpoints, and today's FI/Binky/triage runs still 502 after yesterday's rollout, matching the sanitized upstream 401 railiance-platform reported. The client now raises with error, provider_status, provider, model and a bounded copy of llm-connect's already-sanitized message, under a field allowlist so no provider blob or key material reaches the run artefact. Refs ACTIVITY-WP-0031-T01, T03. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
0d9ddbaa08
commit
459a272974
4 changed files with 169 additions and 5 deletions
|
|
@ -184,7 +184,7 @@
|
||||||
| task | ACTIVITY-WP-0029-T02 | done | — | workplans/ACTIVITY-WP-0029-hub-port-alignment.md |
|
| task | ACTIVITY-WP-0029-T02 | done | — | workplans/ACTIVITY-WP-0029-hub-port-alignment.md |
|
||||||
| task | ACTIVITY-WP-0029-T03 | wait | — | workplans/ACTIVITY-WP-0029-hub-port-alignment.md |
|
| task | ACTIVITY-WP-0029-T03 | wait | — | workplans/ACTIVITY-WP-0029-hub-port-alignment.md |
|
||||||
| task | ACTIVITY-WP-0029-T04 | done | — | workplans/ACTIVITY-WP-0029-hub-port-alignment.md |
|
| task | ACTIVITY-WP-0029-T04 | done | — | workplans/ACTIVITY-WP-0029-hub-port-alignment.md |
|
||||||
| task | ACTIVITY-WP-0030-T01 | wait | — | workplans/ACTIVITY-WP-0030-daily-sbom-catchup.md |
|
| task | ACTIVITY-WP-0030-T01 | done | — | workplans/ACTIVITY-WP-0030-daily-sbom-catchup.md |
|
||||||
| task | ACTIVITY-WP-0030-T02 | wait | — | workplans/ACTIVITY-WP-0030-daily-sbom-catchup.md |
|
| task | ACTIVITY-WP-0030-T02 | wait | — | workplans/ACTIVITY-WP-0030-daily-sbom-catchup.md |
|
||||||
| task | ACTIVITY-WP-0030-T03 | progress | — | workplans/ACTIVITY-WP-0030-daily-sbom-catchup.md |
|
| task | ACTIVITY-WP-0030-T03 | progress | — | workplans/ACTIVITY-WP-0030-daily-sbom-catchup.md |
|
||||||
| task | ACTIVITY-WP-0030-T04 | wait | — | workplans/ACTIVITY-WP-0030-daily-sbom-catchup.md |
|
| task | ACTIVITY-WP-0030-T04 | wait | — | workplans/ACTIVITY-WP-0030-daily-sbom-catchup.md |
|
||||||
|
|
|
||||||
|
|
@ -55,7 +55,8 @@ class LLMConnectClient:
|
||||||
json=payload,
|
json=payload,
|
||||||
timeout=self.timeout_seconds,
|
timeout=self.timeout_seconds,
|
||||||
)
|
)
|
||||||
resp.raise_for_status()
|
if resp.status_code >= 400:
|
||||||
|
raise RuntimeError(_llm_connect_error_text(resp))
|
||||||
data = resp.json()
|
data = resp.json()
|
||||||
self.last_response_metadata = _extract_response_metadata(data)
|
self.last_response_metadata = _extract_response_metadata(data)
|
||||||
content = data.get("content")
|
content = data.get("content")
|
||||||
|
|
@ -64,6 +65,43 @@ class LLMConnectClient:
|
||||||
return content
|
return content
|
||||||
|
|
||||||
|
|
||||||
|
_SAFE_ERROR_KEYS = ("error", "provider_status", "provider", "model")
|
||||||
|
_MAX_ERROR_MESSAGE_CHARS = 400
|
||||||
|
|
||||||
|
|
||||||
|
def _llm_connect_error_text(resp: httpx.Response) -> str:
|
||||||
|
"""Describe an llm-connect error using its own diagnostic body.
|
||||||
|
|
||||||
|
llm-connect maps every provider API error onto HTTP 502 and puts the real
|
||||||
|
cause in the body (``error``, ``provider_status``) — see
|
||||||
|
``llm_connect/server.py::_error_response``. Reporting only the transport
|
||||||
|
status makes a rejected provider key look identical to a dead gateway,
|
||||||
|
which is exactly the ambiguity ACTIVITY-WP-0031-T03 evidence must not have.
|
||||||
|
|
||||||
|
llm-connect sanitizes its own ``message`` before returning it; this keeps a
|
||||||
|
bounded copy of that text plus an allowlist of non-secret fields.
|
||||||
|
"""
|
||||||
|
base = f"llm-connect returned HTTP {resp.status_code}"
|
||||||
|
try:
|
||||||
|
body = resp.json()
|
||||||
|
except ValueError:
|
||||||
|
return base
|
||||||
|
if not isinstance(body, dict):
|
||||||
|
return base
|
||||||
|
|
||||||
|
parts = [
|
||||||
|
f"{key}={body[key]}"
|
||||||
|
for key in _SAFE_ERROR_KEYS
|
||||||
|
if body.get(key) not in (None, "")
|
||||||
|
]
|
||||||
|
message = body.get("message")
|
||||||
|
if isinstance(message, str) and message.strip():
|
||||||
|
parts.append(f"message={message.strip()[:_MAX_ERROR_MESSAGE_CHARS]}")
|
||||||
|
if not parts:
|
||||||
|
return base
|
||||||
|
return f"{base}: " + "; ".join(parts)
|
||||||
|
|
||||||
|
|
||||||
_SAFE_RESPONSE_METADATA_KEYS = {
|
_SAFE_RESPONSE_METADATA_KEYS = {
|
||||||
"finish_reason",
|
"finish_reason",
|
||||||
"usage",
|
"usage",
|
||||||
|
|
|
||||||
|
|
@ -1,6 +1,7 @@
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import httpx
|
import httpx
|
||||||
|
import pytest
|
||||||
|
|
||||||
from activity_core.llm_client import LLMConnectClient
|
from activity_core.llm_client import LLMConnectClient
|
||||||
|
|
||||||
|
|
@ -9,8 +10,7 @@ def test_llm_connect_client_forwards_run_config(monkeypatch) -> None:
|
||||||
captured: dict = {}
|
captured: dict = {}
|
||||||
|
|
||||||
class Response:
|
class Response:
|
||||||
def raise_for_status(self) -> None:
|
status_code = 200
|
||||||
pass
|
|
||||||
|
|
||||||
def json(self) -> dict:
|
def json(self) -> dict:
|
||||||
return {
|
return {
|
||||||
|
|
@ -59,3 +59,103 @@ def test_llm_connect_client_forwards_run_config(monkeypatch) -> None:
|
||||||
"finish_reason": "stop",
|
"finish_reason": "stop",
|
||||||
"usage": {"input_tokens": 10, "output_tokens": 20},
|
"usage": {"input_tokens": 10, "output_tokens": 20},
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
class ErrorResponse:
|
||||||
|
"""Models an llm-connect error reply, which carries the real cause."""
|
||||||
|
|
||||||
|
def __init__(self, status_code: int, body) -> None:
|
||||||
|
self.status_code = status_code
|
||||||
|
self._body = body
|
||||||
|
|
||||||
|
def json(self):
|
||||||
|
if self._body is _NON_JSON:
|
||||||
|
raise ValueError("not json")
|
||||||
|
return self._body
|
||||||
|
|
||||||
|
|
||||||
|
_NON_JSON = object()
|
||||||
|
|
||||||
|
|
||||||
|
def _client(monkeypatch, response) -> LLMConnectClient:
|
||||||
|
monkeypatch.setattr(httpx, "post", lambda url, json, timeout: response)
|
||||||
|
return LLMConnectClient("http://llm-connect.local")
|
||||||
|
|
||||||
|
|
||||||
|
def test_rejected_provider_key_is_distinguishable_from_a_dead_gateway(monkeypatch) -> None:
|
||||||
|
"""llm-connect maps every provider API error onto 502 (server.py:230).
|
||||||
|
|
||||||
|
ACTIVITY-WP-0031: reporting only "502 Bad Gateway" made a revoked
|
||||||
|
OpenRouter key look like a downed llm-connect for four days.
|
||||||
|
"""
|
||||||
|
client = _client(
|
||||||
|
monkeypatch,
|
||||||
|
ErrorResponse(
|
||||||
|
502,
|
||||||
|
{
|
||||||
|
"error": "provider_api_error",
|
||||||
|
"provider_status": 401,
|
||||||
|
"provider": "openrouter",
|
||||||
|
"message": "No auth credentials found",
|
||||||
|
},
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
|
with pytest.raises(RuntimeError) as excinfo:
|
||||||
|
client.complete("Prompt")
|
||||||
|
|
||||||
|
text = str(excinfo.value)
|
||||||
|
assert "HTTP 502" in text
|
||||||
|
assert "error=provider_api_error" in text
|
||||||
|
assert "provider_status=401" in text
|
||||||
|
assert "provider=openrouter" in text
|
||||||
|
assert "message=No auth credentials found" in text
|
||||||
|
|
||||||
|
|
||||||
|
def test_error_text_keeps_only_allowlisted_fields(monkeypatch) -> None:
|
||||||
|
client = _client(
|
||||||
|
monkeypatch,
|
||||||
|
ErrorResponse(
|
||||||
|
502,
|
||||||
|
{
|
||||||
|
"error": "provider_api_error",
|
||||||
|
"provider_status": 401,
|
||||||
|
"api_key": "sk-should-never-appear",
|
||||||
|
"raw_response": {"authorization": "Bearer secret"},
|
||||||
|
},
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
|
with pytest.raises(RuntimeError) as excinfo:
|
||||||
|
client.complete("Prompt")
|
||||||
|
|
||||||
|
text = str(excinfo.value)
|
||||||
|
assert "sk-should-never-appear" not in text
|
||||||
|
assert "authorization" not in text
|
||||||
|
assert "provider_status=401" in text
|
||||||
|
|
||||||
|
|
||||||
|
def test_error_message_is_bounded(monkeypatch) -> None:
|
||||||
|
client = _client(
|
||||||
|
monkeypatch,
|
||||||
|
ErrorResponse(502, {"error": "provider_api_error", "message": "x" * 5000}),
|
||||||
|
)
|
||||||
|
|
||||||
|
with pytest.raises(RuntimeError) as excinfo:
|
||||||
|
client.complete("Prompt")
|
||||||
|
|
||||||
|
assert len(str(excinfo.value)) < 600
|
||||||
|
|
||||||
|
|
||||||
|
def test_error_without_a_usable_body_still_reports_the_status(monkeypatch) -> None:
|
||||||
|
client = _client(monkeypatch, ErrorResponse(504, _NON_JSON))
|
||||||
|
|
||||||
|
with pytest.raises(RuntimeError, match="llm-connect returned HTTP 504"):
|
||||||
|
client.complete("Prompt")
|
||||||
|
|
||||||
|
|
||||||
|
def test_error_with_a_non_dict_body_still_reports_the_status(monkeypatch) -> None:
|
||||||
|
client = _client(monkeypatch, ErrorResponse(500, ["unexpected"]))
|
||||||
|
|
||||||
|
with pytest.raises(RuntimeError, match="llm-connect returned HTTP 500"):
|
||||||
|
client.complete("Prompt")
|
||||||
|
|
|
||||||
|
|
@ -8,7 +8,7 @@ status: active
|
||||||
owner: codex
|
owner: codex
|
||||||
topic_slug: activity-core
|
topic_slug: activity-core
|
||||||
created: "2026-08-20"
|
created: "2026-08-20"
|
||||||
updated: "2026-08-20"
|
updated: "2026-08-21"
|
||||||
related:
|
related:
|
||||||
- ACTIVITY-WP-0021
|
- ACTIVITY-WP-0021
|
||||||
- ACTIVITY-WP-0022
|
- ACTIVITY-WP-0022
|
||||||
|
|
@ -49,6 +49,32 @@ still returns sanitized OpenRouter HTTP 401, proving the canonical key itself
|
||||||
is rejected. T01 remains `wait` on an attended OpenRouter account owner to mint
|
is rejected. T01 remains `wait` on an attended OpenRouter account owner to mint
|
||||||
and safely provision a replacement key; no key value was read or printed.
|
and safely provision a replacement key; no key value was read or printed.
|
||||||
|
|
||||||
|
Progress 2026-08-21 (activity-core half, no custody boundary crossed): live
|
||||||
|
evidence confirms a single fault, not two. FI, Binky, and daily triage still
|
||||||
|
fail today (`05:20` / `05:30` / `06:23` UTC) with `502 Bad Gateway` from
|
||||||
|
llm-connect — **after** yesterday's rollout (`llm-connect` pod is `Running`
|
||||||
|
1/1, endpoints healthy). That 502 is not a dead gateway:
|
||||||
|
`llm_connect/server.py::_error_response` maps **every** `LLMAPIError` onto 502
|
||||||
|
and carries the real cause in the body (`error`, `provider_status`). So the
|
||||||
|
502s and the reported upstream 401 are the same rejected key.
|
||||||
|
|
||||||
|
`LLMConnectClient.complete` called `raise_for_status()` and discarded that
|
||||||
|
body, so four days of production evidence read as "llm-connect is down" when it
|
||||||
|
meant "the provider rejected the key". Fixed: the client now raises with
|
||||||
|
`error`, `provider_status`, `provider`, `model`, and a bounded copy of
|
||||||
|
llm-connect's already-sanitized `message`, under a field allowlist so no
|
||||||
|
provider blob or key material can reach the run artefact. Five tests cover the
|
||||||
|
401-behind-502 case, the allowlist, bounding, and unusable bodies.
|
||||||
|
|
||||||
|
The identical body-discarding call exists in
|
||||||
|
`rein-aharness/rein_aharness/llm_connect_client.py:65`, which is what produces
|
||||||
|
the opaque ops_run failure text in the status table. Not ours to edit —
|
||||||
|
handed to rein-aharness.
|
||||||
|
|
||||||
|
T01 still `wait`: the key rotation itself remains attended and
|
||||||
|
railiance-platform-owned. What changed is that the next failure will name its
|
||||||
|
own cause.
|
||||||
|
|
||||||
## Emergency-pause weekly SBOM fan-out
|
## Emergency-pause weekly SBOM fan-out
|
||||||
|
|
||||||
```task
|
```task
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue