diff --git a/docs/guides/error-handling.md b/docs/guides/error-handling.md index 9b13aacb..9457f2d2 100644 --- a/docs/guides/error-handling.md +++ b/docs/guides/error-handling.md @@ -10,6 +10,7 @@ OtfError │ └── RetryableOtfRequestError ├── OtfAuthenticationError ├── OtfTransportError +├── OtfConfigurationError ├── BookingError │ ├── AlreadyBookedError │ ├── ConflictingBookingError @@ -30,6 +31,7 @@ from otf_api.exceptions import ( RetryableOtfRequestError, OtfAuthenticationError, OtfTransportError, + OtfConfigurationError, BookingError, AlreadyBookedError, ConflictingBookingError, @@ -112,6 +114,21 @@ except OtfTransportError as e: # The original httpx exception is available as e.__cause__ ``` +### OtfConfigurationError + +Raised when the underlying boto3/botocore client is misconfigured or invoked with invalid parameters — for example, an invalid `AWS_PROFILE` or a parameter validation failure. Unlike `OtfTransportError`, these failures are not retryable: retrying won't fix a bad configuration. Wraps the underlying `botocore` error so consumers don't need to import `botocore`. + +```python +from otf_api import OtfUser +from otf_api.exceptions import OtfConfigurationError + +try: + user = OtfUser(username="user@example.com", password="correct") +except OtfConfigurationError as e: + print(f"Configuration error: {e}") + # The original botocore.exceptions.BotoCoreError is available as e.__cause__ +``` + ### BookingError Base class for all booking-related errors. Carries identifiers for the affected booking. diff --git a/src/otf_api/auth/auth.py b/src/otf_api/auth/auth.py index ad4e327e..6f084ebf 100644 --- a/src/otf_api/auth/auth.py +++ b/src/otf_api/auth/auth.py @@ -6,7 +6,7 @@ from functools import cached_property from logging import getLogger from time import sleep -from typing import Any, ClassVar +from typing import Any, ClassVar, NoReturn import httpx import jwt @@ -16,12 +16,18 @@ from botocore.awsrequest import AWSRequest from botocore.config import Config from botocore.credentials import Credentials -from botocore.exceptions import ClientError +from botocore.exceptions import BotoCoreError, ClientError, HTTPClientError +from botocore.exceptions import ConnectionError as BotoConnectionError from pycognito import AWSSRP, Cognito from pycognito.aws_srp import generate_hash_device from otf_api.cache import get_cache -from otf_api.exceptions import NoCredentialsError, OtfAuthenticationError +from otf_api.exceptions import ( + NoCredentialsError, + OtfAuthenticationError, + OtfConfigurationError, + OtfTransportError, +) if typing.TYPE_CHECKING: from mypy_boto3_cognito_identity import CognitoIdentityClient @@ -40,6 +46,33 @@ CACHE = get_cache() +def raise_for_botocore_error(e: BotoCoreError, action: str) -> NoReturn: + """Log and re-raise a caught BotoCoreError as the correct public Otf* exception. + + ClientError is a sibling of BotoCoreError, not a subclass, so this is never called for + API-level failures. ConnectionError and HTTPClientError are the two transport-failure + branches of BotoCoreError's hierarchy (connectivity, timeout, failure to reach an + already-resolved endpoint) — genuinely retryable, so they're reported as + OtfTransportError. Everything else under BotoCoreError (ParamValidationError, + ProfileNotFound, NoRegionError, etc.) is a configuration/validation failure that a + retry cannot fix, so it's reported as OtfConfigurationError. + + Args: + e: The caught BotoCoreError. + action: Present-tense description of what was happening, for the log message + (e.g. "authenticating with Cognito"). + + Raises: + OtfTransportError: If e is a connectivity, timeout, or endpoint-connection failure. + OtfConfigurationError: If e is any other BotoCoreError. + """ + if isinstance(e, BotoConnectionError | HTTPClientError): + LOGGER.exception("Transport error while %s", action) + raise OtfTransportError("OTF transport error") from e + LOGGER.exception("Configuration error while %s", action) + raise OtfConfigurationError("OTF configuration error") from e + + class OtfCognito(Cognito): """A subclass of the pycognito Cognito class that adds the device_key to the auth_params. @@ -319,6 +352,9 @@ def check_token(self, renew: bool = True) -> bool: Raises: AttributeError: If access_token is not set NoCredentialsError: If refresh token has expired + OtfAuthenticationError: If token refresh fails for another Cognito-reported reason + OtfTransportError: If a connectivity, timeout, or endpoint failure occurs during refresh + OtfConfigurationError: If a boto3/botocore configuration or validation failure occurs during refresh Returns: bool: True if the access_token has expired, False otherwise @@ -330,7 +366,10 @@ def check_token(self, renew: bool = True) -> bool: LOGGER.warning("Tokens expired, attempting to login with username and password") CACHE.clear() raise NoCredentialsError("Cached tokens expired, please login again") from e - raise OtfAuthenticationError(str(e)) from e + LOGGER.exception("Failed to refresh Cognito tokens") + raise OtfAuthenticationError("OTF authentication failed") from e + except BotoCoreError as e: + raise_for_botocore_error(e, "refreshing Cognito tokens") def renew_access_token(self) -> None: """Sets a new access token on the User using the cached refresh token and device metadata. diff --git a/src/otf_api/auth/user.py b/src/otf_api/auth/user.py index 1c8acafd..b7341b72 100644 --- a/src/otf_api/auth/user.py +++ b/src/otf_api/auth/user.py @@ -1,9 +1,9 @@ from logging import getLogger import attrs -from botocore.exceptions import ClientError +from botocore.exceptions import BotoCoreError, ClientError -from otf_api.auth.auth import HttpxCognitoAuth, OtfCognito +from otf_api.auth.auth import HttpxCognitoAuth, OtfCognito, raise_for_botocore_error from otf_api.auth.utils import get_username_password from otf_api.exceptions import NoCredentialsError, OtfAuthenticationError @@ -27,7 +27,9 @@ def _create_cognito(username: str | None, password: str | None, **kwargs: str | raise except ClientError as e: _log_initial_auth_error(e, username) - raise OtfAuthenticationError(str(e)) from e + raise OtfAuthenticationError("OTF authentication failed") from e + except BotoCoreError as e: + raise_for_botocore_error(e, "authenticating with Cognito") except Exception: LOGGER.exception("Failed to authenticate with Cognito") raise diff --git a/src/otf_api/exceptions.py b/src/otf_api/exceptions.py index 2d96d583..a78a8e68 100644 --- a/src/otf_api/exceptions.py +++ b/src/otf_api/exceptions.py @@ -12,6 +12,7 @@ "ConflictingBookingError", "NoCredentialsError", "OtfAuthenticationError", + "OtfConfigurationError", "OtfError", "OtfRequestError", "OtfTransportError", @@ -100,5 +101,14 @@ class OtfTransportError(OtfError): """ +class OtfConfigurationError(OtfError): + """Raised when the boto3/botocore client is misconfigured or invoked with invalid parameters. + + Covers non-network BotoCoreError failures (e.g. a missing AWS profile, an unresolvable region, + or a parameter validation error) that a retry cannot fix. The original error is available via + ``__cause__``. + """ + + class NoCredentialsError(OtfError): """Raised when no credentials are provided and no cached tokens are available.""" diff --git a/tests/test_api/test_errors.py b/tests/test_api/test_errors.py index 0e6157c6..66c874aa 100644 --- a/tests/test_api/test_errors.py +++ b/tests/test_api/test_errors.py @@ -6,11 +6,17 @@ import httpx import pytest import respx -from botocore.exceptions import ClientError +from botocore.exceptions import ClientError, EndpointConnectionError, ParamValidationError from otf_api.api.client import API_BASE_URL, OtfClient from otf_api.auth.user import OtfUser -from otf_api.exceptions import NoCredentialsError, OtfAuthenticationError, OtfError, OtfTransportError +from otf_api.exceptions import ( + NoCredentialsError, + OtfAuthenticationError, + OtfConfigurationError, + OtfError, + OtfTransportError, +) def _make_client_error(code: str = "NotAuthorizedException", message: str = "bad creds") -> ClientError: @@ -51,6 +57,17 @@ def cognito_side_effect(**kwargs: object) -> Never: assert exc_info.value.__cause__ is error + def test_auth_error_message_is_fixed_and_safe(self): + error = _make_client_error(message="secret provider detail") + with ( + patch("otf_api.auth.user.OtfCognito", side_effect=error), + pytest.raises(OtfAuthenticationError) as exc_info, + ): + OtfUser(username="test@example.com", password="wrong") + + assert str(exc_info.value) == "OTF authentication failed" + assert "secret provider detail" not in str(exc_info.value) + def test_auth_error_is_subclass_of_otf_error(self): assert issubclass(OtfAuthenticationError, OtfError) @@ -94,3 +111,30 @@ def test_read_error_raises_transport_error(self, mock_user): def test_transport_error_is_subclass_of_otf_error(self): assert issubclass(OtfTransportError, OtfError) + + def test_construction_time_connectivity_failure_raises_transport_error(self): + error = EndpointConnectionError(endpoint_url="https://cognito-idp.example.com") + with ( + patch("otf_api.auth.user.OtfCognito", side_effect=error), + pytest.raises(OtfTransportError) as exc_info, + ): + OtfUser(username="test@example.com", password="wrong") + + assert str(exc_info.value) == "OTF transport error" + assert exc_info.value.__cause__ is error + + +class TestOtfConfigurationError: + def test_construction_time_config_failure_raises_configuration_error(self): + error = ParamValidationError(report="bad params") + with ( + patch("otf_api.auth.user.OtfCognito", side_effect=error), + pytest.raises(OtfConfigurationError) as exc_info, + ): + OtfUser(username="test@example.com", password="wrong") + + assert str(exc_info.value) == "OTF configuration error" + assert exc_info.value.__cause__ is error + + def test_configuration_error_is_subclass_of_otf_error(self): + assert issubclass(OtfConfigurationError, OtfError) diff --git a/tests/test_auth/test_otf_cognito.py b/tests/test_auth/test_otf_cognito.py index b89f3aa4..7b47a63d 100644 --- a/tests/test_auth/test_otf_cognito.py +++ b/tests/test_auth/test_otf_cognito.py @@ -3,9 +3,10 @@ from unittest.mock import MagicMock, PropertyMock, patch import pytest -from botocore.exceptions import ClientError +from botocore.exceptions import ClientError, EndpointConnectionError, ParamValidationError from otf_api.auth.auth import NoCredentialsError, OtfCognito +from otf_api.exceptions import OtfAuthenticationError, OtfConfigurationError, OtfTransportError from .conftest import fake_tokens @@ -231,6 +232,64 @@ def test_check_token_not_authorized(mock_cache, mock_verify_token): mock_clear.assert_called_once() +def test_check_token_other_client_error_raises_auth_error(mock_cache, mock_verify_token): + """check_token raises OtfAuthenticationError with a fixed, safe message for non-auth-expiry ClientErrors.""" + mock_cache.write_token_data_to_cache({"access_token": _ACCESS_TOKEN, "id_token": "id", "refresh_token": "rt"}) + mock_cache.write_device_data_to_cache({"device_key": "dk", "device_group_key": "dgk", "device_password": "dp"}) + + with patch("pycognito.Cognito.check_token", return_value=False): + cognito = OtfCognito(username="user@test.com") + + error = _client_error("InternalErrorException", message="secret provider detail") + with ( + patch("pycognito.Cognito.check_token", side_effect=error), + pytest.raises(OtfAuthenticationError) as exc_info, + ): + cognito.check_token() + + assert str(exc_info.value) == "OTF authentication failed" + assert "secret provider detail" not in str(exc_info.value) + assert exc_info.value.__cause__ is error + + +def test_check_token_connectivity_failure_raises_transport_error(mock_cache, mock_verify_token): + """check_token raises OtfTransportError with a fixed, safe message on refresh-time connectivity failures.""" + mock_cache.write_token_data_to_cache({"access_token": _ACCESS_TOKEN, "id_token": "id", "refresh_token": "rt"}) + mock_cache.write_device_data_to_cache({"device_key": "dk", "device_group_key": "dgk", "device_password": "dp"}) + + with patch("pycognito.Cognito.check_token", return_value=False): + cognito = OtfCognito(username="user@test.com") + + error = EndpointConnectionError(endpoint_url="https://cognito-idp.example.com") + with ( + patch("pycognito.Cognito.check_token", side_effect=error), + pytest.raises(OtfTransportError) as exc_info, + ): + cognito.check_token() + + assert str(exc_info.value) == "OTF transport error" + assert exc_info.value.__cause__ is error + + +def test_check_token_configuration_error_raises_configuration_error(mock_cache, mock_verify_token): + """check_token raises OtfConfigurationError with a fixed, safe message on non-transport BotoCoreErrors.""" + mock_cache.write_token_data_to_cache({"access_token": _ACCESS_TOKEN, "id_token": "id", "refresh_token": "rt"}) + mock_cache.write_device_data_to_cache({"device_key": "dk", "device_group_key": "dgk", "device_password": "dp"}) + + with patch("pycognito.Cognito.check_token", return_value=False): + cognito = OtfCognito(username="user@test.com") + + error = ParamValidationError(report="bad params") + with ( + patch("pycognito.Cognito.check_token", side_effect=error), + pytest.raises(OtfConfigurationError) as exc_info, + ): + cognito.check_token() + + assert str(exc_info.value) == "OTF configuration error" + assert exc_info.value.__cause__ is error + + # --------------------------------------------------------------------------- # Subtask 12 — _set_tokens happy path # ---------------------------------------------------------------------------