From 2f78707a796d3533c6a0d5463ee582d41d991605 Mon Sep 17 00:00:00 2001 From: worlldz Date: Fri, 21 Aug 2026 18:24:02 +0300 Subject: [PATCH] fix(web): fail over keyless auth rejections --- plugins/web/keyless_mcp.py | 34 +++++++++--- tests/tools/test_web_keyless_fallback.py | 68 +++++++++++++++++++++++- 2 files changed, 93 insertions(+), 9 deletions(-) diff --git a/plugins/web/keyless_mcp.py b/plugins/web/keyless_mcp.py index e59d33b71b..7c2f721e34 100644 --- a/plugins/web/keyless_mcp.py +++ b/plugins/web/keyless_mcp.py @@ -47,6 +47,23 @@ def _is_rate_limitish(message: str) -> bool: return any(marker in (message or "").lower() for marker in _RATE_LIMIT_MARKERS) +_AUTH_STATUS_RE = re.compile( + r"\b(?:http(?:\s+status)?|status(?:\s+code)?|client\s+error|error(?:\s+code)?)" + r"\s*[:=']*\s*(?:401|403)\b", + re.IGNORECASE, +) + + +def _is_search_failover_eligible(message: str) -> bool: + """Return whether another anonymous vendor may serve the search. + + Rate limits and structured HTTP 401/403 provider rejections are local to + one free search endpoint. Free-text markers are deliberately ignored: + vendors may echo the query in an otherwise terminal error. + """ + return _is_rate_limitish(message) or bool(_AUTH_STATUS_RE.search(message or "")) + + def _fail_msg(vendor: str, kind: str, exc: Any, *, other_backends: bool = True) -> str: label, env_key, site = _VENDOR_HINTS[vendor] alt = " or another web backend via `hermes tools`" if other_backends else "" @@ -369,31 +386,32 @@ def _walk_ring(name: str, kind: str, call, throttled) -> tuple: if not throttled(result): return order, vendor, result, False if i + 1 < len(order): - logger.info("keyless %s %s throttled; failing over to %s", vendor, kind, order[i + 1]) + logger.info("keyless %s %s unavailable; failing over to %s", vendor, kind, order[i + 1]) return order, vendor, result, True def search_with_failover(name: str, query: str, limit: int = 5) -> Dict[str, Any]: - """Rate-limit-shaped errors advance to the next vendor, other errors stop the walk - (a malformed query fails everywhere). ``data.served_by`` is set when the serving - vendor differs from *name*.""" + """Rate limits and anonymous-endpoint auth/policy rejections advance to the next + vendor, other errors stop the walk (a malformed query fails everywhere). + ``data.served_by`` is set when the serving vendor differs from *name*.""" def _throttled(result: Dict[str, Any]) -> bool: - return not result.get("success") and _is_rate_limitish(result.get("error", "")) + return not result.get("success") and _is_search_failover_eligible(result.get("error", "")) order, vendor, result, exhausted = _walk_ring(name, "search", lambda v: _KEYLESS_SEARCHERS[v](query, limit), _throttled) if not order: return search_fail(_ALL_PAID_MSG) if exhausted: - result["error"] = f"{result.get('error', '')} (all keyless vendors throttled: {', '.join(order)})" + result["error"] = f"{result.get('error', '')} (all keyless vendors unavailable: {', '.join(order)})" elif result.get("success") and vendor != name: result.setdefault("data", {})["served_by"] = vendor return result def extract_with_failover(name: str, urls: List[str]) -> List[Dict[str, Any]]: - """Fails over only when EVERY url in a batch is rate-limit-shaped (partial failures - are page problems, returned as-is).""" + """Advances to the next ring vendor only when EVERY url in a batch comes + back with a rate-limit-shaped error — partial failures and HTTP 403s can + be page problems, not provider throttling, and return as-is.""" def _all_throttled(results: List[Dict[str, Any]]) -> bool: return bool(results) and all(r.get("error", "") and _is_rate_limitish(r.get("error", "")) for r in results) diff --git a/tests/tools/test_web_keyless_fallback.py b/tests/tools/test_web_keyless_fallback.py index fe0b2400f3..f20191ff5e 100644 --- a/tests/tools/test_web_keyless_fallback.py +++ b/tests/tools/test_web_keyless_fallback.py @@ -509,6 +509,26 @@ class TestKeylessFailover: """Pin *name* so the ring starts there deterministically.""" monkeypatch.setattr(keyless_mcp, "_vendor_pinned", lambda n: n == name) + @pytest.mark.parametrize( + ("message", "expected"), + [ + ("free MCP rate limit", True), + ("Client error '403 Forbidden'", True), + ("HTTP status 401: unauthorized", True), + ("status code=403", True), + ("HTTP 2403", False), + ("error 4031", False), + ("FORBIDDEN", False), + ("extract failed for 'forbidden kingdom trailer': connection timeout", False), + ("HTTP 400: malformed request", False), + ("", False), + ], + ) + def test_search_failover_eligibility_requires_structured_status( + self, message, expected + ): + assert keyless_mcp._is_search_failover_eligible(message) is expected + def test_search_fails_over_on_rate_limit(self, monkeypatch): self._pin(monkeypatch, "exa") monkeypatch.setitem(keyless_mcp._KEYLESS_SEARCHERS, "exa", lambda q, l: self._throttled("Exa")) @@ -517,6 +537,27 @@ class TestKeylessFailover: assert out["success"] is True assert out["data"]["served_by"] == "parallel" + def test_search_fails_over_on_anonymous_provider_forbidden(self, monkeypatch): + self._pin(monkeypatch, "firecrawl") + monkeypatch.setitem( + keyless_mcp._KEYLESS_SEARCHERS, + "firecrawl", + lambda q, l: { + "success": False, + "error": "Keyless Firecrawl search failed: Client error '403 Forbidden'", + }, + ) + monkeypatch.setitem( + keyless_mcp._KEYLESS_SEARCHERS, + "keenable", + lambda q, l: self._ok("keenable"), + ) + + out = keyless_mcp.search_with_failover("firecrawl", "q") + + assert out["success"] is True + assert out["data"]["served_by"] == "keenable" + def test_search_no_failover_on_non_throttle_error(self, monkeypatch): self._pin(monkeypatch, "exa") monkeypatch.setitem( @@ -541,7 +582,7 @@ class TestKeylessFailover: ) out = keyless_mcp.search_with_failover("exa", "q") assert out["success"] is False - assert "all keyless vendors throttled" in out["error"] + assert "all keyless vendors unavailable" in out["error"] def test_search_walks_ring_past_multiple_throttles(self, monkeypatch): # exa -> parallel all throttled; firecrawl serves. @@ -596,6 +637,31 @@ class TestKeylessFailover: out = keyless_mcp.extract_with_failover("exa", ["https://a", "https://b"]) assert out == good + def test_extract_all_forbidden_stays_on_primary(self, monkeypatch): + self._pin(monkeypatch, "firecrawl") + forbidden = [ + {"url": url, "title": "", "content": "", "error": "HTTP 403"} + for url in ("https://a", "https://b") + ] + called = [] + monkeypatch.setitem( + keyless_mcp._KEYLESS_EXTRACTORS, + "firecrawl", + lambda urls: forbidden, + ) + monkeypatch.setitem( + keyless_mcp._KEYLESS_EXTRACTORS, + "keenable", + lambda urls: called.append(1) or [], + ) + + out = keyless_mcp.extract_with_failover( + "firecrawl", ["https://a", "https://b"] + ) + + assert out == forbidden + assert not called + def test_extract_partial_failure_stays_on_primary(self, monkeypatch): self._pin(monkeypatch, "exa") partial = [