From 62dbffd0f71f8f9c54c503b0f5429a2e9c0c64a8 Mon Sep 17 00:00:00 2001 From: marcelo Date: Tue, 11 Aug 2026 19:46:50 +0000 Subject: [PATCH 1/3] [Fix] Parse flat PublicError responses in the SDK CLI login was falling back to a bare HTTP status when the API returned a flat {error, message, metadata} body. Teach to_api_error to read PublicError, OAuth error_description, and legacy top-level context so the CLI can print the server message without a local hint table. --- src/gumloop/errors.py | 67 +++++++++++++++++++---- tests/cli/test_login.py | 25 +++++++++ tests/sdk/test_errors.py | 115 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 195 insertions(+), 12 deletions(-) create mode 100644 tests/sdk/test_errors.py diff --git a/src/gumloop/errors.py b/src/gumloop/errors.py index 0c8665e..560335c 100644 --- a/src/gumloop/errors.py +++ b/src/gumloop/errors.py @@ -4,6 +4,8 @@ import httpx +_FLAT_RESERVED_KEYS = frozenset({"error", "message", "error_description", "trace_id", "metadata"}) + class GumloopError(Exception): """Base error for Gumloop SDK failures.""" @@ -21,23 +23,64 @@ def __init__(self, message: str, *, status_code: int, body: Any = None) -> None: self.status_code = status_code self.body = body self.error = body.get("error") if isinstance(body, dict) else None - self.code = self.error.get("code") if isinstance(self.error, dict) else None - self.type = self.error.get("type") if isinstance(self.error, dict) else None - self.param = self.error.get("param") if isinstance(self.error, dict) else None - self.details = self.error.get("details", {}) if isinstance(self.error, dict) else {} + if isinstance(self.error, dict): + self.code = self.error.get("code") + self.type = self.error.get("type") + self.param = self.error.get("param") + details = self.error.get("details") + else: + self.code = self.error if isinstance(self.error, str) else None + self.type = None + self.param = None + details = _flat_details(body) + self.details = details if isinstance(details, dict) else {} + + +def _flat_details(body: Any) -> dict[str, Any]: + """Extract machine-readable context from a flat error body. + + Prefer PublicError ``metadata``. Otherwise keep legacy top-level fields + such as ``minimum_tier`` / ``denied_keys`` (tier/policy denials). + """ + if not isinstance(body, dict): + return {} + metadata = body.get("metadata") + if isinstance(metadata, dict): + return metadata + return {key: value for key, value in body.items() if key not in _FLAT_RESERVED_KEYS} def to_api_error(response: httpx.Response) -> APIStatusError: - """Translate a non-success ``httpx.Response`` into :class:`APIStatusError`, - extracting the backend error envelope's ``message`` when present.""" + """Translate a non-success ``httpx.Response`` into :class:`APIStatusError`. + + Supports: + - Nested developer API: ``{"error": {"code", "message", "details", ...}}`` + - Flat PublicError: ``{"error": "code", "message": "...", "metadata": {...}}`` + - OAuth RFC: ``{"error": "code", "error_description": "..."}`` + - Legacy bare / enriched codes: ``{"error": "tier_required_pro", ...}`` + """ try: body: Any = response.json() except ValueError: body = response.text - error = body.get("error") if isinstance(body, dict) else None - message = ( - str(error.get("message") or f"Gumloop API returned HTTP {response.status_code}") - if isinstance(error, dict) - else f"Gumloop API returned HTTP {response.status_code}" - ) + + fallback = f"Gumloop API returned HTTP {response.status_code}" + if not isinstance(body, dict): + return APIStatusError(fallback, status_code=response.status_code, body=body) + + error = body.get("error") + top_level_message = body.get("message") + error_description = body.get("error_description") + + if isinstance(error, dict): + message = str(error.get("message") or fallback) + elif isinstance(top_level_message, str) and top_level_message: + message = top_level_message + elif isinstance(error_description, str) and error_description: + message = error_description + elif isinstance(error, str) and error: + message = f"{fallback}: {error}" + else: + message = fallback + return APIStatusError(message, status_code=response.status_code, body=body) diff --git a/tests/cli/test_login.py b/tests/cli/test_login.py index 645cabf..5f9e37d 100644 --- a/tests/cli/test_login.py +++ b/tests/cli/test_login.py @@ -81,6 +81,31 @@ def test_login_with_invalid_credentials_exits_nonzero_and_does_not_save(cli_runn assert load_credentials().api_key is None +@respx.mock +def test_login_403_public_error_explains_tier_and_does_not_save(cli_runner: CliRunner) -> None: + respx.get(f"{API_BASE}/models").mock( + return_value=httpx.Response( + 403, + json={ + "error": "subscription_tier_required", + "message": ( + "This feature isn't available on your current plan. Upgrade to continue." + ), + "metadata": { + "minimum_tier": "pro", + "denied_keys": ["gumloop_api"], + }, + }, + ) + ) + + result = cli_runner.invoke(app, ["login", "--access-token", "acct_free"]) + + assert result.exit_code != 0 + assert "This feature isn't available on your current plan" in result.output + assert load_credentials().access_token is None + + @respx.mock def test_login_against_custom_base_url_stores_it_in_keychain(cli_runner: CliRunner) -> None: custom = "https://example.com/api/v1" diff --git a/tests/sdk/test_errors.py b/tests/sdk/test_errors.py new file mode 100644 index 0000000..87f7dba --- /dev/null +++ b/tests/sdk/test_errors.py @@ -0,0 +1,115 @@ +from __future__ import annotations + +import httpx + +from gumloop.errors import to_api_error + + +def test_to_api_error_reads_flat_public_error_message_and_code() -> None: + response = httpx.Response( + 403, + json={ + "error": "subscription_tier_required", + "message": "This feature isn't available on your current plan. Upgrade to continue.", + "metadata": {"minimum_tier": "pro", "denied_keys": ["gumloop_api"]}, + }, + ) + + error = to_api_error(response) + + assert error.status_code == 403 + assert error.code == "subscription_tier_required" + assert str(error) == ( + "This feature isn't available on your current plan. Upgrade to continue." + ) + assert error.details == { + "minimum_tier": "pro", + "denied_keys": ["gumloop_api"], + } + + +def test_to_api_error_reads_nested_error_envelope() -> None: + response = httpx.Response( + 403, + json={ + "error": { + "code": "organization_sync_requires_pro", + "message": "Organization skill sync requires a Pro plan.", + "type": "permission_error", + "param": None, + "details": {}, + } + }, + ) + + error = to_api_error(response) + + assert error.code == "organization_sync_requires_pro" + assert str(error) == "Organization skill sync requires a Pro plan." + assert error.type == "permission_error" + + +def test_to_api_error_reads_nested_details() -> None: + response = httpx.Response( + 400, + json={ + "error": { + "code": "skill_not_found", + "message": "Skill not found.", + "type": "invalid_request_error", + "param": "skill_ids", + "details": {"skill_ids": ["sk_does_not_exist"]}, + } + }, + ) + + error = to_api_error(response) + + assert error.code == "skill_not_found" + assert error.param == "skill_ids" + assert error.details == {"skill_ids": ["sk_does_not_exist"]} + + +def test_to_api_error_exposes_legacy_bare_string_code() -> None: + response = httpx.Response(403, json={"error": "tier_required_pro"}) + + error = to_api_error(response) + + assert error.code == "tier_required_pro" + assert str(error) == "Gumloop API returned HTTP 403: tier_required_pro" + assert error.details == {} + + +def test_to_api_error_preserves_legacy_top_level_context() -> None: + response = httpx.Response( + 403, + json={ + "error": "tier_required_pro", + "minimum_tier": "pro", + "denied_keys": ["gumloop_api"], + }, + ) + + error = to_api_error(response) + + assert error.code == "tier_required_pro" + assert error.details == { + "minimum_tier": "pro", + "denied_keys": ["gumloop_api"], + } + + +def test_to_api_error_reads_oauth_error_description() -> None: + response = httpx.Response( + 401, + json={ + "error": "invalid_token", + "error_description": "Missing subject", + }, + ) + + error = to_api_error(response) + + assert error.code == "invalid_token" + assert str(error) == "Missing subject" + assert error.details == {} From f64fba5dcee985054d2bad90ff4ad8089cdef494 Mon Sep 17 00:00:00 2001 From: marcelo Date: Tue, 11 Aug 2026 19:47:37 +0000 Subject: [PATCH 2/3] [Fix] Align tier PublicError fixtures with auth metadata Auth-layer subscription_tier_required responses carry minimum_tier only, not a fake gumloop_api denied_key. --- tests/cli/test_login.py | 5 +---- tests/sdk/test_errors.py | 7 ++----- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/tests/cli/test_login.py b/tests/cli/test_login.py index 5f9e37d..06dcba5 100644 --- a/tests/cli/test_login.py +++ b/tests/cli/test_login.py @@ -91,10 +91,7 @@ def test_login_403_public_error_explains_tier_and_does_not_save(cli_runner: CliR "message": ( "This feature isn't available on your current plan. Upgrade to continue." ), - "metadata": { - "minimum_tier": "pro", - "denied_keys": ["gumloop_api"], - }, + "metadata": {"minimum_tier": "pro"}, }, ) ) diff --git a/tests/sdk/test_errors.py b/tests/sdk/test_errors.py index 87f7dba..dfdf1ac 100644 --- a/tests/sdk/test_errors.py +++ b/tests/sdk/test_errors.py @@ -11,7 +11,7 @@ def test_to_api_error_reads_flat_public_error_message_and_code() -> None: json={ "error": "subscription_tier_required", "message": "This feature isn't available on your current plan. Upgrade to continue.", - "metadata": {"minimum_tier": "pro", "denied_keys": ["gumloop_api"]}, + "metadata": {"minimum_tier": "pro"}, }, ) @@ -22,10 +22,7 @@ def test_to_api_error_reads_flat_public_error_message_and_code() -> None: assert str(error) == ( "This feature isn't available on your current plan. Upgrade to continue." ) - assert error.details == { - "minimum_tier": "pro", - "denied_keys": ["gumloop_api"], - } + assert error.details == {"minimum_tier": "pro"} def test_to_api_error_reads_nested_error_envelope() -> None: From 93916a0e7983f3d3e90741aff8c49ea07f3fb1bd Mon Sep 17 00:00:00 2001 From: marcelo Date: Tue, 11 Aug 2026 19:51:12 +0000 Subject: [PATCH 3/3] [Fix] Apply ruff format to PublicError parser tests --- tests/cli/test_login.py | 4 +--- tests/sdk/test_errors.py | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/tests/cli/test_login.py b/tests/cli/test_login.py index 06dcba5..1b88f7a 100644 --- a/tests/cli/test_login.py +++ b/tests/cli/test_login.py @@ -88,9 +88,7 @@ def test_login_403_public_error_explains_tier_and_does_not_save(cli_runner: CliR 403, json={ "error": "subscription_tier_required", - "message": ( - "This feature isn't available on your current plan. Upgrade to continue." - ), + "message": ("This feature isn't available on your current plan. Upgrade to continue."), "metadata": {"minimum_tier": "pro"}, }, ) diff --git a/tests/sdk/test_errors.py b/tests/sdk/test_errors.py index dfdf1ac..82a2fde 100644 --- a/tests/sdk/test_errors.py +++ b/tests/sdk/test_errors.py @@ -19,9 +19,7 @@ def test_to_api_error_reads_flat_public_error_message_and_code() -> None: assert error.status_code == 403 assert error.code == "subscription_tier_required" - assert str(error) == ( - "This feature isn't available on your current plan. Upgrade to continue." - ) + assert str(error) == ("This feature isn't available on your current plan. Upgrade to continue.") assert error.details == {"minimum_tier": "pro"}