From 2b3136b0b2264d4e3fa85d195ad6cc7f2f0bcf52 Mon Sep 17 00:00:00 2001 From: Petr Date: Tue, 18 Aug 2026 09:06:02 -0400 Subject: [PATCH] fix(http): never replay a credential-minting call on an ambiguous failure (#599) POST /v2/storage/tokens is a create, not a read, yet the shared retry loop replayed it up to 3 times on 500/502/503/504 and on read timeouts. A server that mints the token and then fails to answer hands back a second live credential the caller never sees a value for -- and therefore can never revoke. _do_request now takes `idempotent` (default True, so every existing call site is byte-identical); the two token-create paths and the data-app managed-repo git-credentials-create pass False. Failures that provably never reached the handler stay retryable even then -- 429 (rejected by the rate limiter before the handler runs), connect error, connect timeout -- so no resilience is lost where replay is safe. token refresh is deliberately left retryable: a replayed rotation overwrites the value the lost response carried instead of leaving a second credential behind. Second half of the issue: a 5xx now names itself as upstream and carries the support id. `API error 500 ... Application error.` gave the caller no next step, so a persistent upstream outage read like a broken request. Server errors now append the status-page/support hint plus `(exceptionId: ...)` when the body carries one; 4xx messages are untouched. A non-idempotent 5xx also reports retryable: false in the --json envelope. --- docs/sdk.md | 2 +- src/keboola_agent_cli/auth/auth_client.py | 12 +- src/keboola_agent_cli/changelog.py | 26 +++ src/keboola_agent_cli/client/tokens.py | 8 +- src/keboola_agent_cli/constants.py | 30 +++ src/keboola_agent_cli/data_science_client.py | 3 + src/keboola_agent_cli/http_base.py | 72 +++++- tests/test_client_device_enrollment.py | 24 ++ tests/test_http_base.py | 234 ++++++++++++++++++- 9 files changed, 399 insertions(+), 12 deletions(-) diff --git a/docs/sdk.md b/docs/sdk.md index e921ddb0..3edf292a 100644 --- a/docs/sdk.md +++ b/docs/sdk.md @@ -29,7 +29,7 @@ with Client(url=os.environ["KBC_URL"], token=os.environ["KBC_TOKEN"]) as kbc: **It is:** -- **Stateless.** A `Client` holds the stack URL, the token, one pooled HTTP client (with the shared retry/backoff), and an optional idempotency store. Nothing else. +- **Stateless.** A `Client` holds the stack URL, the token, one pooled HTTP client (with the shared retry/backoff), and an optional idempotency store. Nothing else. The retry loop deliberately skips the credential-minting calls (`create_scoped_token` and friends) on an ambiguous failure -- a replayed 5xx or read timeout could mint a second live token you never see a value for (#599). A 429, connect error or connect timeout never reached the handler, so those stay retryable. - **Config-dir-free.** No `~/.config/keboola-agent-cli/`, no `kbagent project add`, no `config.json`. Auth is the token you pass in (12-factor: you read `KBC_TOKEN` yourself). - **In-process.** No CLI subprocess, no `kbagent serve` daemon. Just function calls. - **Typed.** `py.typed` ships in the wheel; the high-traffic operations return pydantic models (`JobResult`, `QueryResult`, ...). Your `mypy`/`ty`/IDE sees the shapes. diff --git a/src/keboola_agent_cli/auth/auth_client.py b/src/keboola_agent_cli/auth/auth_client.py index 8b38e75f..5c882521 100644 --- a/src/keboola_agent_cli/auth/auth_client.py +++ b/src/keboola_agent_cli/auth/auth_client.py @@ -770,7 +770,13 @@ def _extract_error_message(response: httpx.Response) -> str: # Shared error mapping # ------------------------------------------------------------------ - def _raise_api_error(self, response: httpx.Response, base_url: str | None = None) -> None: + def _raise_api_error( + self, + response: httpx.Response, + base_url: str | None = None, + *, + idempotent: bool = True, + ) -> None: """Escalate a 404 before falling back to the shared error mapping. `BaseHttpClient._do_request` calls this method for every @@ -778,6 +784,10 @@ def _raise_api_error(self, response: httpx.Response, base_url: str | None = None (rather than adding a check in each method) covers all of them at once. `poll_device_token` bypasses `_do_request` entirely and calls `_map_auth_error` directly for the same 404 case. + + `idempotent` is accepted to keep the override signature-compatible + with the base class; no auth call passes it, and `_map_auth_error` + raises before the base mapping (which reads it) is ever reached. """ self._map_auth_error(response) diff --git a/src/keboola_agent_cli/changelog.py b/src/keboola_agent_cli/changelog.py index 427f1e79..39ea7ebb 100644 --- a/src/keboola_agent_cli/changelog.py +++ b/src/keboola_agent_cli/changelog.py @@ -25,6 +25,32 @@ # Ordered newest-first. Each value is a list of brief one-line descriptions. CHANGELOG: dict[str, list[str]] = { "0.84.2": [ + "Fix: credential-minting API calls are no longer replayed on an ambiguous failure " + "(closes #599). `POST /v2/storage/tokens` (`token create`, the SDK's " + "`create_scoped_token`, short-lived component tokens) and the data-app managed-repo " + "`git-credentials-create` are creates, not reads: the shared retry loop treated their " + "500/502/503/504 and read-timeout failures like any other request and replayed them up " + "to 3 times. A server that mints the token and then fails to answer would hand back a " + "second live credential the caller never sees a value for -- and therefore can never " + "revoke. `BaseHttpClient._do_request` now takes `idempotent` (default True, so every " + "existing call site is byte-identical) and those four call sites pass False. Failures " + "that provably never reached the handler stay retryable even then -- a 429 (rejected by " + "the rate limiter before the handler runs), a connect error, a connect timeout -- so the " + "change costs no resilience where replay is safe. `token refresh` is deliberately left " + "retryable: a replayed rotation overwrites the value the lost response carried instead " + "of leaving a second credential behind.", + "Change: a 5xx now says it is upstream and carries the support id. `API error 500 ... " + "Application error.` gave the caller no next step, so a persistent upstream outage read " + "like a broken request and cost the reporter of #599 an investigation across two " + "projects and two tokens before concluding it was not theirs to fix. Server errors now " + "append `This is an upstream Keboola API failure, not a rejected request. If it " + "persists across retries -- and across projects and tokens -- it is an upstream " + "incident: check https://status.keboola.com and report it to Keboola support`, plus " + "`(exceptionId: ...)` whenever the response body carries one (the first thing Keboola " + "support asks for, and previously discarded). 4xx messages are untouched -- a rejected " + "request is the caller's to fix and must not blame upstream. A non-idempotent 5xx also " + "reports `retryable: false` in the `--json` error envelope, so an automated caller does " + "not replay a mint the server may already have honoured.", "New: `kbagent config clone` duplicates a configuration WHOLE (closes #587). " "`--project P --component-id C --config-id ID --name N [--target-project P2] " "[--set PATH=VALUE ...] [--secret PATH=VALUE ...] [--dry-run]`. Until now there was " diff --git a/src/keboola_agent_cli/client/tokens.py b/src/keboola_agent_cli/client/tokens.py index 623cb80d..5d1b2e4f 100644 --- a/src/keboola_agent_cli/client/tokens.py +++ b/src/keboola_agent_cli/client/tokens.py @@ -103,6 +103,7 @@ def create_short_lived_token( "expiresIn": str(expires_in), "componentAccess[]": component_access, }, + idempotent=False, ) return response.json() @@ -156,7 +157,7 @@ def create_scoped_token( data[f"bucketPermissions[{bucket_id}]"] = permission if component_access: data["componentAccess[]"] = component_access - response = self._request("POST", "/v2/storage/tokens", data=data) + response = self._request("POST", "/v2/storage/tokens", data=data, idempotent=False) return response.json() def delete_token(self, token_id: str) -> None: @@ -176,6 +177,11 @@ def refresh_token(self, token_id: str) -> dict[str, Any]: **old** token string becomes immediately invalid (rotation, not additive), so every place using it must be updated. The token id is stable across a refresh. + + Deliberately left retryable, unlike the create calls above: a replayed + rotation overwrites the value the lost response carried instead of + leaving a second live credential behind, so the caller still ends up + holding the only token that authenticates. """ response = self._request( "POST", f"/v2/storage/tokens/{quote(str(token_id), safe='')}/refresh" diff --git a/src/keboola_agent_cli/constants.py b/src/keboola_agent_cli/constants.py index be5e8c2b..d31ddce3 100644 --- a/src/keboola_agent_cli/constants.py +++ b/src/keboola_agent_cli/constants.py @@ -51,6 +51,13 @@ def _resolve_app_name() -> str: # --- HTTP Retry Constants --- RETRYABLE_STATUS_CODES: set[int] = {429, 500, 502, 503, 504} +# Subset of the above that stays retryable when the request is NOT idempotent +# (a create/mint call such as ``POST /v2/storage/tokens``). A 429 is rejected by +# the rate limiter before the handler runs, so replaying it cannot duplicate a +# side effect. Every 5xx is ambiguous -- the server may have completed the write +# and then failed to answer -- so replaying one can mint a second live +# credential the caller never sees and can never revoke (issue #599). +NON_IDEMPOTENT_RETRY_STATUS_CODES: set[int] = {429} MAX_RETRIES: int = 3 BACKOFF_BASE: float = 1.0 # seconds; delays: 1s, 2s, 4s @@ -60,6 +67,29 @@ def _resolve_app_name() -> str: # --- API Error Handling --- MAX_API_ERROR_LENGTH: int = 500 +# Lowest status the server owns: at or above it the request was accepted and +# then failed inside Keboola, so no amount of reshaping it helps the caller. +HTTP_STATUS_SERVER_ERROR_MIN: int = 500 + +# Appended to 5xx errors. A server-side failure is never fixable by reshaping +# the request, and a *persistent* one (identical across retries, projects and +# tokens) is an upstream incident rather than caller error -- saying so turns a +# dead end into a next step. +UPSTREAM_ERROR_HINT: str = ( + "This is an upstream Keboola API failure, not a rejected request. " + "If it persists across retries -- and across projects and tokens -- " + "it is an upstream incident: check https://status.keboola.com and " + "report it to Keboola support" +) + +# Appended when a non-idempotent request fails: it was answered once and not +# replayed, but the server may still have applied it. +NON_IDEMPOTENT_NOT_RETRIED_HINT: str = ( + "Not retried automatically: this request is not idempotent, so it may " + "still have taken effect server-side -- verify the current state before " + "retrying" +) + # --- Developer Portal MFA --- # Challenge type sent on the second `/auth/login` step (after the first call # returns a `session` token). The apiary spec documents `SOFTWARE_TOKEN_MFA` diff --git a/src/keboola_agent_cli/data_science_client.py b/src/keboola_agent_cli/data_science_client.py index f2330e2f..622c1177 100644 --- a/src/keboola_agent_cli/data_science_client.py +++ b/src/keboola_agent_cli/data_science_client.py @@ -350,5 +350,8 @@ def create_git_credential( f"/apps/{quote(str(app_id), safe='')}/git-repo/credentials", content=json.dumps(payload).encode("utf-8"), headers={"Content-Type": "application/json"}, + # Mints a one-time secret: a replayed 5xx would leave a second live + # credential on the repo that the caller never sees (issue #599). + idempotent=False, ) return response.json() diff --git a/src/keboola_agent_cli/http_base.py b/src/keboola_agent_cli/http_base.py index 91b5262f..1a4a1556 100644 --- a/src/keboola_agent_cli/http_base.py +++ b/src/keboola_agent_cli/http_base.py @@ -19,10 +19,14 @@ APP_NAME, BACKOFF_BASE, ENV_CONVERSATION_ID, + HTTP_STATUS_SERVER_ERROR_MIN, MAX_API_ERROR_LENGTH, MAX_RETRIES, MAX_RETRY_AFTER_SECONDS, + NON_IDEMPOTENT_NOT_RETRIED_HINT, + NON_IDEMPOTENT_RETRY_STATUS_CODES, RETRYABLE_STATUS_CODES, + UPSTREAM_ERROR_HINT, ) from .errors import ErrorCode, KeboolaApiError, mask_token @@ -144,6 +148,7 @@ def _do_request( *, client: httpx.Client | None = None, base_url: str | None = None, + idempotent: bool = True, **kwargs: Any, ) -> httpx.Response: """Execute an HTTP request with retry and exponential backoff. @@ -157,6 +162,15 @@ def _do_request( client: Optional httpx.Client to use (defaults to self._client). Useful for subclasses that maintain multiple clients (e.g. queue client). base_url: Optional base URL for error messages (defaults to self._base_url). + idempotent: False for a call whose replay would create a SECOND + resource -- the credential-minting endpoints, above all + ``POST /v2/storage/tokens``. Such a request is not replayed on + an ambiguous failure (a 5xx, or a read/write timeout, either of + which the server may have applied before failing to answer), + because the duplicate it would mint is a live credential the + caller never sees and therefore can never revoke (issue #599). + Failures that provably never reached the handler -- a 429, a + connect error, a connect timeout -- stay retryable regardless. **kwargs: Additional arguments passed to httpx.Client.request(). Returns: @@ -168,6 +182,11 @@ def _do_request( http_client = client or self._client url_label = base_url or self._base_url last_response: httpx.Response | None = None + retryable_statuses = ( + RETRYABLE_STATUS_CODES + if idempotent + else RETRYABLE_STATUS_CODES & NON_IDEMPOTENT_RETRY_STATUS_CODES + ) for attempt in range(MAX_RETRIES): try: @@ -176,7 +195,7 @@ def _do_request( if response.status_code < 400: return response - if response.status_code in RETRYABLE_STATUS_CODES and attempt < MAX_RETRIES - 1: + if response.status_code in retryable_statuses and attempt < MAX_RETRIES - 1: if response.status_code == 429: retry_after = response.headers.get("Retry-After") if retry_after: @@ -201,10 +220,14 @@ def _do_request( last_response = response continue - self._raise_api_error(response, url_label) + self._raise_api_error(response, url_label, idempotent=idempotent) except httpx.TimeoutException as exc: - if attempt < MAX_RETRIES - 1: + # A connect timeout never handed the request to the server, so + # replaying it is safe even for a non-idempotent call; a read or + # write timeout may have been applied and answered too slowly. + timeout_retryable = idempotent or isinstance(exc, httpx.ConnectTimeout) + if timeout_retryable and attempt < MAX_RETRIES - 1: delay = BACKOFF_BASE * (2**attempt) logger.debug( "Retry attempt %d/%d for %s %s (timeout), delay %.1fs", @@ -216,11 +239,16 @@ def _do_request( ) time.sleep(delay) continue + message = ( + f"Request timed out connecting to {url_label} (token: {self._masked_token})" + ) + if not timeout_retryable: + message = f"{message}. {NON_IDEMPOTENT_NOT_RETRIED_HINT}" raise KeboolaApiError( - message=f"Request timed out connecting to {url_label} (token: {self._masked_token})", + message=message, status_code=0, error_code=ErrorCode.TIMEOUT, - retryable=True, + retryable=timeout_retryable, ) from exc except httpx.ConnectError as exc: @@ -244,7 +272,7 @@ def _do_request( ) from exc if last_response is not None: - self._raise_api_error(last_response, url_label) + self._raise_api_error(last_response, url_label, idempotent=idempotent) raise KeboolaApiError( message=f"Request failed after {MAX_RETRIES} retries to {url_label} (token: {self._masked_token})", @@ -253,7 +281,13 @@ def _do_request( retryable=True, ) - def _raise_api_error(self, response: httpx.Response, base_url: str | None = None) -> None: + def _raise_api_error( + self, + response: httpx.Response, + base_url: str | None = None, + *, + idempotent: bool = True, + ) -> None: """Convert an HTTP error response into a KeboolaApiError. Parses the response body for error messages, truncates long messages @@ -263,12 +297,19 @@ def _raise_api_error(self, response: httpx.Response, base_url: str | None = None Args: response: The HTTP error response. base_url: Optional URL label for error messages. + idempotent: See :meth:`_do_request`. False marks a 5xx as "answered + once, not replayed", which is both a different next step for the + caller and a different ``retryable`` verdict on the raised error. Raises: KeboolaApiError: Always raised with appropriate error code and message. """ status = response.status_code url_label = base_url or self._base_url + # Keboola services put a support-traceable id on server-side failures. + # Surfaced when present, silently skipped when not -- it is the first + # thing support asks for and the caller cannot otherwise see it. + exception_id: str | None = None try: body = response.json() @@ -296,6 +337,9 @@ def _raise_api_error(self, response: httpx.Response, base_url: str | None = None ) if not isinstance(api_message, str): api_message = json.dumps(api_message) + raw_exception_id = body.get("exceptionId") if isinstance(body, dict) else None + if raw_exception_id: + exception_id = str(raw_exception_id) except Exception: api_message = response.text @@ -328,8 +372,20 @@ def _raise_api_error(self, response: httpx.Response, base_url: str | None = None ) retryable = status in RETRYABLE_STATUS_CODES + message = ( + f"API error {status} from {url_label} (token: {self._masked_token}): {api_message}" + ) + if exception_id: + message = f"{message} (exceptionId: {exception_id})" + if status >= HTTP_STATUS_SERVER_ERROR_MIN: + message = f"{message}. {UPSTREAM_ERROR_HINT}" + if not idempotent: + # An automated caller reading `retryable` must not replay a mint + # call the server may already have honoured. + retryable = False + message = f"{message}. {NON_IDEMPOTENT_NOT_RETRIED_HINT}" raise KeboolaApiError( - message=f"API error {status} from {url_label} (token: {self._masked_token}): {api_message}", + message=message, status_code=status, error_code=ErrorCode.API_ERROR, retryable=retryable, diff --git a/tests/test_client_device_enrollment.py b/tests/test_client_device_enrollment.py index 547f1c22..e9452dc2 100644 --- a/tests/test_client_device_enrollment.py +++ b/tests/test_client_device_enrollment.py @@ -13,7 +13,10 @@ from typing import Any from urllib.parse import parse_qs +import pytest + from keboola_agent_cli.client import KeboolaClient +from keboola_agent_cli.errors import KeboolaApiError STACK_URL = "https://connection.keboola.com" STREAM_BASE_URL = "https://stream.keboola.com" @@ -123,6 +126,27 @@ def test_can_read_all_file_uploads_and_component_access(self, httpx_mock) -> Non assert "expiresIn" not in body +class TestCreateScopedTokenIsNotReplayed: + """``POST /v2/storage/tokens`` mints a credential -- never replay it (#599).""" + + def test_persistent_500_makes_exactly_one_attempt(self, httpx_mock) -> None: + """The reported EU-GCP 500 must cost one attempt, not three mint tries.""" + httpx_mock.add_response( + url=f"{STACK_URL}/v2/storage/tokens", + status_code=500, + json={"error": "Application error."}, + ) + + client = _make_client() + try: + with pytest.raises(KeboolaApiError) as exc_info: + client.create_scoped_token(description="device 42", expires_in=60) + assert len(httpx_mock.get_requests()) == 1 + assert exc_info.value.retryable is False + finally: + client.close() + + class TestDeleteToken: def test_delete_returns_none(self, httpx_mock) -> None: """DELETE /v2/storage/tokens/{id} -> 204, no body, returns None.""" diff --git a/tests/test_http_base.py b/tests/test_http_base.py index 7f68471b..83b31b56 100644 --- a/tests/test_http_base.py +++ b/tests/test_http_base.py @@ -7,7 +7,13 @@ import httpx import pytest -from keboola_agent_cli.constants import APP_NAME, MAX_API_ERROR_LENGTH, MAX_RETRIES +from keboola_agent_cli.constants import ( + APP_NAME, + MAX_API_ERROR_LENGTH, + MAX_RETRIES, + NON_IDEMPOTENT_NOT_RETRIED_HINT, + UPSTREAM_ERROR_HINT, +) from keboola_agent_cli.errors import KeboolaApiError from keboola_agent_cli.http_base import BaseHttpClient, build_user_agent @@ -222,6 +228,232 @@ def test_alternate_client_parameter(self, httpx_mock) -> None: base_client.close() +class TestNonIdempotentRetryPolicy: + """A credential-minting call must not be replayed on an ambiguous failure. + + Replaying ``POST /v2/storage/tokens`` after a 5xx (or a read timeout) can + mint a SECOND live token the caller never sees a value for and therefore + can never revoke -- issue #599. + """ + + @staticmethod + def _client() -> BaseHttpClient: + return BaseHttpClient( + base_url=STACK_URL, + token=TOKEN, + headers={"Authorization": f"Bearer {TOKEN}"}, + ) + + def test_non_idempotent_500_is_not_retried(self, httpx_mock) -> None: + """A single 500 ends the call: no replay, no second token.""" + httpx_mock.add_response( + url=f"{STACK_URL}/v2/storage/tokens", + status_code=500, + json={"error": "Application error."}, + ) + + client = self._client() + try: + with pytest.raises(KeboolaApiError) as exc_info: + client._do_request("POST", "/v2/storage/tokens", idempotent=False) + assert len(httpx_mock.get_requests()) == 1 + assert exc_info.value.status_code == 500 + # Automated callers read `retryable`; replaying a mint is not safe. + assert exc_info.value.retryable is False + assert NON_IDEMPOTENT_NOT_RETRIED_HINT in exc_info.value.message + finally: + client.close() + + def test_idempotent_500_still_retries(self, httpx_mock) -> None: + """The default path is unchanged: a read still gets its 3 attempts.""" + for _ in range(MAX_RETRIES): + httpx_mock.add_response(url=f"{STACK_URL}/test-path", status_code=500) + + client = self._client() + import keboola_agent_cli.http_base as http_base_module + + original_sleep = http_base_module.time.sleep + http_base_module.time.sleep = _noop_sleep # ty: ignore[invalid-assignment] + try: + with pytest.raises(KeboolaApiError) as exc_info: + client._do_request("GET", "/test-path") + assert len(httpx_mock.get_requests()) == MAX_RETRIES + assert exc_info.value.retryable is True + assert NON_IDEMPOTENT_NOT_RETRIED_HINT not in exc_info.value.message + finally: + http_base_module.time.sleep = original_sleep + client.close() + + def test_non_idempotent_429_still_retries(self, httpx_mock) -> None: + """429 is rejected before the handler runs, so replay cannot duplicate.""" + httpx_mock.add_response(url=f"{STACK_URL}/v2/storage/tokens", status_code=429) + httpx_mock.add_response( + url=f"{STACK_URL}/v2/storage/tokens", status_code=201, json={"id": "1"} + ) + + client = self._client() + import keboola_agent_cli.http_base as http_base_module + + original_sleep = http_base_module.time.sleep + http_base_module.time.sleep = _noop_sleep # ty: ignore[invalid-assignment] + try: + response = client._do_request("POST", "/v2/storage/tokens", idempotent=False) + assert response.status_code == 201 + assert len(httpx_mock.get_requests()) == 2 + finally: + http_base_module.time.sleep = original_sleep + client.close() + + def test_non_idempotent_read_timeout_is_not_retried(self, httpx_mock) -> None: + """A read timeout may have been applied server-side -- do not replay it.""" + httpx_mock.add_exception( + httpx.ReadTimeout("Read timed out"), url=f"{STACK_URL}/v2/storage/tokens" + ) + + client = self._client() + try: + with pytest.raises(KeboolaApiError) as exc_info: + client._do_request("POST", "/v2/storage/tokens", idempotent=False) + assert len(httpx_mock.get_requests()) == 1 + assert exc_info.value.error_code == "TIMEOUT" + assert exc_info.value.retryable is False + assert NON_IDEMPOTENT_NOT_RETRIED_HINT in exc_info.value.message + finally: + client.close() + + def test_non_idempotent_connect_timeout_still_retries(self, httpx_mock) -> None: + """A connect timeout never handed the request over -- replay is safe.""" + httpx_mock.add_exception( + httpx.ConnectTimeout("Connect timed out"), url=f"{STACK_URL}/v2/storage/tokens" + ) + httpx_mock.add_response( + url=f"{STACK_URL}/v2/storage/tokens", status_code=201, json={"id": "1"} + ) + + client = self._client() + import keboola_agent_cli.http_base as http_base_module + + original_sleep = http_base_module.time.sleep + http_base_module.time.sleep = _noop_sleep # ty: ignore[invalid-assignment] + try: + response = client._do_request("POST", "/v2/storage/tokens", idempotent=False) + assert response.status_code == 201 + assert len(httpx_mock.get_requests()) == 2 + finally: + http_base_module.time.sleep = original_sleep + client.close() + + def test_non_idempotent_connect_error_still_retries(self, httpx_mock) -> None: + """A refused connection never reached the handler -- replay is safe.""" + httpx_mock.add_exception( + httpx.ConnectError("Connection refused"), url=f"{STACK_URL}/v2/storage/tokens" + ) + httpx_mock.add_response( + url=f"{STACK_URL}/v2/storage/tokens", status_code=201, json={"id": "1"} + ) + + client = self._client() + import keboola_agent_cli.http_base as http_base_module + + original_sleep = http_base_module.time.sleep + http_base_module.time.sleep = _noop_sleep # ty: ignore[invalid-assignment] + try: + response = client._do_request("POST", "/v2/storage/tokens", idempotent=False) + assert response.status_code == 201 + assert len(httpx_mock.get_requests()) == 2 + finally: + http_base_module.time.sleep = original_sleep + client.close() + + def test_non_idempotent_4xx_is_unchanged(self, httpx_mock) -> None: + """A 403 was never retried and still maps to ACCESS_DENIED, no 5xx hint.""" + httpx_mock.add_response( + url=f"{STACK_URL}/v2/storage/tokens", + status_code=403, + json={"error": "You don't have access to the resource."}, + ) + + client = self._client() + try: + with pytest.raises(KeboolaApiError) as exc_info: + client._do_request("POST", "/v2/storage/tokens", idempotent=False) + assert exc_info.value.error_code == "ACCESS_DENIED" + assert UPSTREAM_ERROR_HINT not in exc_info.value.message + finally: + client.close() + + +class TestUpstreamErrorGuidance: + """A 5xx should tell the caller it is upstream, and carry the support id.""" + + def test_5xx_message_carries_upstream_hint(self, httpx_mock) -> None: + for _ in range(MAX_RETRIES): + httpx_mock.add_response( + url=f"{STACK_URL}/test-path", + status_code=500, + json={"error": "Application error."}, + ) + + client = BaseHttpClient( + base_url=STACK_URL, + token=TOKEN, + headers={"Authorization": f"Bearer {TOKEN}"}, + ) + import keboola_agent_cli.http_base as http_base_module + + original_sleep = http_base_module.time.sleep + http_base_module.time.sleep = _noop_sleep # ty: ignore[invalid-assignment] + try: + with pytest.raises(KeboolaApiError) as exc_info: + client._do_request("GET", "/test-path") + assert "Application error." in exc_info.value.message + assert UPSTREAM_ERROR_HINT in exc_info.value.message + finally: + http_base_module.time.sleep = original_sleep + client.close() + + def test_exception_id_surfaced_when_present(self, httpx_mock) -> None: + """Keboola's support-traceable id is the first thing support asks for.""" + httpx_mock.add_response( + url=f"{STACK_URL}/test-path", + status_code=503, + json={"error": "Application error.", "exceptionId": "storage-api-abc123"}, + ) + + client = BaseHttpClient( + base_url=STACK_URL, + token=TOKEN, + headers={"Authorization": f"Bearer {TOKEN}"}, + ) + try: + with pytest.raises(KeboolaApiError) as exc_info: + client._do_request("POST", "/test-path", idempotent=False) + assert "exceptionId: storage-api-abc123" in exc_info.value.message + finally: + client.close() + + def test_4xx_has_no_upstream_hint(self, httpx_mock) -> None: + """A rejected request is the caller's to fix -- do not blame upstream.""" + httpx_mock.add_response( + url=f"{STACK_URL}/test-path", + status_code=400, + json={"error": "Invalid bucket id"}, + ) + + client = BaseHttpClient( + base_url=STACK_URL, + token=TOKEN, + headers={"Authorization": f"Bearer {TOKEN}"}, + ) + try: + with pytest.raises(KeboolaApiError) as exc_info: + client._do_request("GET", "/test-path") + assert UPSTREAM_ERROR_HINT not in exc_info.value.message + assert exc_info.value.retryable is False + finally: + client.close() + + class TestBaseHttpClientErrorSanitization: """Verify message truncation and error mapping in the base class."""