From 37553df007be8ec964d745e0b325c5df37347d52 Mon Sep 17 00:00:00 2001 From: Josh Mabry Date: Fri, 3 Jul 2026 01:43:38 -0700 Subject: [PATCH] feat: readable Google errors, 401 retry-once, locked token cache, label quoting - auth.request/get_access_token now raise GoogleError carrying Google's own JSON error message ('insufficient authentication scopes' beats a bare 403); the token endpoint's invalid_grant hints at reconnecting from the Google panel - a 401 on a data call invalidates the cached access token and retries once with a fresh one (revoked/early-expired token), then gives up loudly - token cache is lock-guarded (tools run on worker threads); network refresh stays outside the lock - gmail_list_unread quotes the label so multi-word labels don't split into label:Priority + free-text Inbox Co-Authored-By: Claude Fable 5 --- __init__.py | 5 +++- auth.py | 65 +++++++++++++++++++++++++++++++++++------- tests/test_plugin.py | 9 ++++++ tests/test_services.py | 65 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 132 insertions(+), 12 deletions(-) diff --git a/__init__.py b/__init__.py index d758de1..f2303fe 100644 --- a/__init__.py +++ b/__init__.py @@ -70,7 +70,10 @@ def gmail_list_unread(label: str = "INBOX", max: int = 20) -> str: """ from . import gmail - out = _run(gmail.list_messages, _creds(), f"label:{label} is:unread", max) + # Quote the label: an unquoted multi-word label ("Priority Inbox") would split + # into label:Priority + free-text "Inbox" in Gmail's query syntax. + quoted = '"%s"' % label.replace('"', "") + out = _run(gmail.list_messages, _creds(), f"label:{quoted} is:unread", max) return out if isinstance(out, str) else json.dumps({"label": label, "count": len(out), "messages": out}, indent=2) diff --git a/auth.py b/auth.py index 132a912..542b3a0 100644 --- a/auth.py +++ b/auth.py @@ -8,10 +8,15 @@ granted. Mint the refresh token with the FULL workspace scope set you intend to grow into (e.g. gmail.modify, calendar, drive, documents, spreadsheets) so adding a service later needs no re-consent. + +Errors surface as GoogleError carrying Google's own message (the JSON error body), +not a bare HTTP status — "insufficient authentication scopes" beats "403". A 401 on +a data call retries once with a fresh token (the cached one may have been revoked). """ from __future__ import annotations +import threading import time from dataclasses import dataclass @@ -35,19 +40,42 @@ def configured(self) -> bool: return bool(self.client_id and self.client_secret and self.refresh_token) -# {refresh_token: (access_token, expires_at)} +# {refresh_token: (access_token, expires_at)} — tools run on worker threads, so the +# cache is lock-guarded; the network refresh happens outside the lock (a rare +# duplicate refresh is harmless, a held lock across I/O is not). _TOKEN_CACHE: dict[str, tuple[str, float]] = {} +_CACHE_LOCK = threading.Lock() def _now() -> float: return time.time() +def _error_detail(resp: httpx.Response) -> str: + """Google's own error message out of a failed response's JSON body.""" + try: + payload = resp.json() + except Exception: # noqa: BLE001 — non-JSON error body + return (resp.text or "")[:200] + err = payload.get("error") + if isinstance(err, dict): # API style: {"error": {"code": ..., "message": ..., "status": ...}} + return str(err.get("message") or err.get("status") or "")[:300] + # OAuth style: {"error": "invalid_grant", "error_description": "..."} + return str(payload.get("error_description") or err or "")[:300] + + +def invalidate_token(creds: Creds) -> None: + """Drop the cached access token (e.g. after a 401 — it may have been revoked).""" + with _CACHE_LOCK: + _TOKEN_CACHE.pop(creds.refresh_token, None) + + def get_access_token(creds: Creds, *, client: httpx.Client | None = None) -> str: """Exchange the refresh token for a cached short-lived access token.""" if not creds.configured(): raise GoogleError("Google is not configured — set client_id, client_secret, refresh_token.") - cached = _TOKEN_CACHE.get(creds.refresh_token) + with _CACHE_LOCK: + cached = _TOKEN_CACHE.get(creds.refresh_token) if cached and _now() < cached[1] - _EARLY_REFRESH_S: return cached[0] owns = client is None @@ -59,13 +87,19 @@ def get_access_token(creds: Creds, *, client: httpx.Client | None = None) -> str "client_secret": creds.client_secret, "refresh_token": creds.refresh_token, }) - resp.raise_for_status() + if resp.status_code != 200: + detail = _error_detail(resp) or f"HTTP {resp.status_code}" + raise GoogleError( + f"token refresh failed: {detail} — if the grant was revoked or expired, " + "reconnect from the Google panel (or re-mint the refresh token)." + ) payload = resp.json() finally: if owns: client.close() token = payload["access_token"] - _TOKEN_CACHE[creds.refresh_token] = (token, _now() + int(payload.get("expires_in", 3600))) + with _CACHE_LOCK: + _TOKEN_CACHE[creds.refresh_token] = (token, _now() + int(payload.get("expires_in", 3600))) return token @@ -75,17 +109,26 @@ def request(creds: Creds, method: str, url: str, *, params: dict | None = None, Any service module builds on this — pass a full endpoint URL + params/json. Pass ``client`` (e.g. an httpx.MockTransport client) to unit-test offline. + A 401 retries once with a freshly minted token; other errors raise GoogleError + with Google's message from the response body. """ - token = get_access_token(creds, client=client) owns = client is None c = client or httpx.Client(timeout=30) try: - resp = c.request(method, url, params=params, json=json, - headers={"Authorization": f"Bearer {token}"}) - resp.raise_for_status() - if resp.status_code == 204 or not resp.content: - return {} - return resp.json() + for attempt in (1, 2): + token = get_access_token(creds, client=c) + resp = c.request(method, url, params=params, json=json, + headers={"Authorization": f"Bearer {token}"}) + if resp.status_code == 401 and attempt == 1: + invalidate_token(creds) # cached token revoked/expired early — retry fresh + continue + break finally: if owns: c.close() + if resp.status_code >= 400: + detail = _error_detail(resp) or "no detail" + raise GoogleError(f"{method} {resp.url.path} -> {resp.status_code}: {detail}") + if resp.status_code == 204 or not resp.content: + return {} + return resp.json() diff --git a/tests/test_plugin.py b/tests/test_plugin.py index f6919ff..b519b86 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -86,6 +86,15 @@ def test_tools_hint_when_unconfigured(monkeypatch): assert "isn't configured" in out and "GOOGLE_CLIENT_ID" in out +def test_list_unread_quotes_the_label(monkeypatch): + # An unquoted multi-word label would split into label:Priority + free-text "Inbox". + monkeypatch.setattr(plugin, "_CREDS", Creds("c", "s", "r")) + seen = {} + monkeypatch.setattr(gmail, "list_messages", lambda creds, q, mx, **kw: seen.update(q=q) or []) + plugin.gmail_list_unread.invoke({"label": "Priority Inbox"}) + assert seen["q"] == 'label:"Priority Inbox" is:unread' + + def test_draft_tool_requires_target(monkeypatch): monkeypatch.setattr(plugin, "_CREDS", Creds("c", "s", "r")) out = plugin.gmail_create_draft.invoke({"body": "hi"}) diff --git a/tests/test_services.py b/tests/test_services.py index 67ad9cb..16c03d9 100644 --- a/tests/test_services.py +++ b/tests/test_services.py @@ -154,3 +154,68 @@ def test_calendar_list_and_detail(client): assert detail["description"] == "daily" assert detail["attendees"][0] == {"email": "a@x.com", "name": "Al", "status": "accepted"} assert detail["organizer"]["email"] == "o@x.com" + + +# ── Error surfacing + 401 retry ─────────────────────────────────────────────── + + +def test_api_error_surfaces_googles_message(): + def handler(request: httpx.Request) -> httpx.Response: + if request.url.host == "oauth2.googleapis.com": + return httpx.Response(200, json={"access_token": "at", "expires_in": 3600}) + return httpx.Response( + 403, + json={ + "error": { + "code": 403, + "message": "Request had insufficient authentication scopes.", + "status": "PERMISSION_DENIED", + } + }, + ) + + auth._TOKEN_CACHE.clear() + with httpx.Client(transport=httpx.MockTransport(handler)) as c: + with pytest.raises(GoogleError, match="403.*insufficient authentication scopes"): + auth.request(CREDS, "GET", "https://gmail.googleapis.com/gmail/v1/users/me/messages", client=c) + + +def test_token_refresh_error_is_readable_and_hints_reconnect(): + resp = httpx.Response( + 400, json={"error": "invalid_grant", "error_description": "Token has been expired or revoked."} + ) + auth._TOKEN_CACHE.clear() + with httpx.Client(transport=httpx.MockTransport(lambda r: resp)) as c: + with pytest.raises(GoogleError, match="expired or revoked.*[Rr]econnect"): + auth.get_access_token(CREDS, client=c) + + +def test_401_retries_once_with_a_fresh_token(): + counts = {"token": 0, "data": 0} + + def handler(request: httpx.Request) -> httpx.Response: + if request.url.host == "oauth2.googleapis.com": + counts["token"] += 1 + return httpx.Response(200, json={"access_token": f"at-{counts['token']}", "expires_in": 3600}) + counts["data"] += 1 + if request.headers["Authorization"] == "Bearer at-1": # first (revoked) token + return httpx.Response(401, json={"error": {"code": 401, "message": "Invalid Credentials"}}) + return httpx.Response(200, json={"ok": True}) + + auth._TOKEN_CACHE.clear() + with httpx.Client(transport=httpx.MockTransport(handler)) as c: + out = auth.request(CREDS, "GET", "https://gmail.googleapis.com/gmail/v1/users/me/profile", client=c) + assert out == {"ok": True} + assert counts == {"token": 2, "data": 2} # one invalidation, one retry — no loop + + +def test_401_gives_up_after_one_retry(): + def handler(request: httpx.Request) -> httpx.Response: + if request.url.host == "oauth2.googleapis.com": + return httpx.Response(200, json={"access_token": "at", "expires_in": 3600}) + return httpx.Response(401, json={"error": {"code": 401, "message": "Invalid Credentials"}}) + + auth._TOKEN_CACHE.clear() + with httpx.Client(transport=httpx.MockTransport(handler)) as c: + with pytest.raises(GoogleError, match="401.*Invalid Credentials"): + auth.request(CREDS, "GET", "https://gmail.googleapis.com/gmail/v1/users/me/profile", client=c)