From 04c23b23f2bfdad96064b3b95d5e35752a747727 Mon Sep 17 00:00:00 2001 From: Charles Graham SWT Date: Tue, 24 Mar 2026 13:51:07 -0700 Subject: [PATCH 1/5] Remove 500 generic error from the retry --- cwms/api.py | 1 - 1 file changed, 1 deletion(-) diff --git a/cwms/api.py b/cwms/api.py index e96526e..c1de9b3 100644 --- a/cwms/api.py +++ b/cwms/api.py @@ -55,7 +55,6 @@ status_forcelist=[ 403, 429, - 500, 502, 503, 504, From d71f484bd2df3a5ea1d3b4e6eb75d2f7b02fb253 Mon Sep 17 00:00:00 2001 From: Charles Graham SWT Date: Tue, 24 Mar 2026 16:31:53 -0700 Subject: [PATCH 2/5] Gracefully handle and retry requests errors --- cwms/api.py | 69 +++++++++++++++++++++++++++++++----------- tests/mock/api_test.py | 62 ++++++++++++++++++++++++++++++++++++- 2 files changed, 112 insertions(+), 19 deletions(-) diff --git a/cwms/api.py b/cwms/api.py index c1de9b3..ca32c45 100644 --- a/cwms/api.py +++ b/cwms/api.py @@ -37,6 +37,7 @@ from typing import Any, Optional, cast from requests import Response, adapters +from requests.exceptions import RetryError as RequestsRetryError from requests_toolbelt import sessions # type: ignore from requests_toolbelt.sessions import BaseUrlSession # type: ignore from urllib3.util.retry import Retry @@ -60,6 +61,7 @@ 504, ], # Example: also retry on these HTTP status codes allowed_methods=["GET", "PUT", "POST", "PATCH", "DELETE"], # Methods to retry + raise_on_status=False, ) SESSION = sessions.BaseUrlSession(base_url=API_ROOT) adapter = adapters.HTTPAdapter( @@ -139,6 +141,21 @@ class PermissionError(ApiError): """Raised when the CDA request is not authorized for the current caller.""" +def _unwrap_retry_error(error: RequestsRetryError) -> Exception: + """Return the original retry cause when requests wraps it in RetryError.""" + + current: BaseException = error + while getattr(current, "__cause__", None) is not None: + current = current.__cause__ + + if current is error and error.args: + current = error.args[0] + while getattr(current, "reason", None) is not None: + current = current.reason + + return current if isinstance(current, Exception) else error + + def init_session( *, api_root: Optional[str] = None, @@ -307,11 +324,14 @@ def get( """ headers = {"Accept": api_version_text(api_version)} - with SESSION.get(endpoint, params=params, headers=headers) as response: - if not response.ok: - logging.error(f"CDA Error: response={response}") - raise ApiError(response) - return _process_response(response) + try: + with SESSION.get(endpoint, params=params, headers=headers) as response: + if not response.ok: + logging.error(f"CDA Error: response={response}") + raise ApiError(response) + return _process_response(response) + except RequestsRetryError as error: + raise _unwrap_retry_error(error) from None def get_with_paging( @@ -366,11 +386,16 @@ def _post_function( headers = {"accept": "*/*", "Content-Type": api_version_text(api_version)} if isinstance(data, dict) or isinstance(data, list): data = json.dumps(data) - with SESSION.post(endpoint, params=params, headers=headers, data=data) as response: - if not response.ok: - logging.error(f"CDA Error: response={response}") - raise ApiError(response) - return response + try: + with SESSION.post( + endpoint, params=params, headers=headers, data=data + ) as response: + if not response.ok: + logging.error(f"CDA Error: response={response}") + raise ApiError(response) + return response + except RequestsRetryError as error: + raise _unwrap_retry_error(error) from None def post( @@ -460,10 +485,15 @@ def patch( if data and isinstance(data, dict) or isinstance(data, list): data = json.dumps(data) - with SESSION.patch(endpoint, params=params, headers=headers, data=data) as response: - if not response.ok: - logging.error(f"CDA Error: response={response}") - raise ApiError(response) + try: + with SESSION.patch( + endpoint, params=params, headers=headers, data=data + ) as response: + if not response.ok: + logging.error(f"CDA Error: response={response}") + raise ApiError(response) + except RequestsRetryError as error: + raise _unwrap_retry_error(error) from None def delete( @@ -487,7 +517,10 @@ def delete( """ headers = {"Accept": api_version_text(api_version)} - with SESSION.delete(endpoint, params=params, headers=headers) as response: - if not response.ok: - logging.error(f"CDA Error: response={response}") - raise ApiError(response) + try: + with SESSION.delete(endpoint, params=params, headers=headers) as response: + if not response.ok: + logging.error(f"CDA Error: response={response}") + raise ApiError(response) + except RequestsRetryError as error: + raise _unwrap_retry_error(error) from None diff --git a/tests/mock/api_test.py b/tests/mock/api_test.py index 5680c24..7e57233 100644 --- a/tests/mock/api_test.py +++ b/tests/mock/api_test.py @@ -1,6 +1,9 @@ import pytest +from requests.exceptions import RetryError as RequestsRetryError +from urllib3.exceptions import MaxRetryError, ResponseError -from cwms.api import SESSION, InvalidVersion, api_version_text, init_session +import cwms.api +from cwms.api import SESSION, ApiError, InvalidVersion, api_version_text, init_session def test_session_default(): @@ -53,3 +56,60 @@ def test_api_headers(): version = api_version_text(api_version=2) assert version == "application/json;version=2" + + +def test_retry_strategy_configuration(): + """Verify retry behavior preserves the original CDA error path.""" + + retries = SESSION.adapters["https://"].max_retries + + assert 500 not in retries.status_forcelist + assert retries.raise_on_status is False + + +def test_post_500_raises_api_error(monkeypatch): + """Verify a 500 response is surfaced directly as ApiError.""" + + class ResponseStub: + url = "https://example.com/cwms-data/test-endpoint" + ok = False + status_code = 500 + reason = "Internal Server Error" + content = b"incident identifier 34566432" + + def __enter__(self): + return self + + def __exit__(self, exc_type, exc, tb): + return False + + class SessionStub: + def post(self, endpoint, params=None, headers=None, data=None): + return ResponseStub() + + monkeypatch.setattr(cwms.api, "SESSION", SessionStub()) + + with pytest.raises(ApiError) as error: + cwms.api._post_function(endpoint="/test-endpoint", data={}) + + assert error.value.response.status_code == 500 + assert "Internal Server Error" in str(error.value) + assert "incident identifier 34566432" in str(error.value) + + +def test_retry_error_unwraps_original_cause(monkeypatch): + """Verify wrapped retry failures propagate the underlying cause.""" + + original_error = ResponseError("too many 503 error responses") + wrapped_error = RequestsRetryError( + MaxRetryError(pool=None, url="/test-endpoint", reason=original_error) + ) + + class SessionStub: + def get(self, endpoint, params=None, headers=None): + raise wrapped_error + + monkeypatch.setattr(cwms.api, "SESSION", SessionStub()) + + with pytest.raises(ResponseError, match="too many 503 error responses"): + cwms.api.get("/test-endpoint") From 7a2bb9a0900f79f153b767bfe50363dbae779140 Mon Sep 17 00:00:00 2001 From: Charles Graham SWT Date: Tue, 24 Mar 2026 16:43:14 -0700 Subject: [PATCH 3/5] Fix baseexception import --- cwms/api.py | 1 + 1 file changed, 1 insertion(+) diff --git a/cwms/api.py b/cwms/api.py index ca32c45..0efe814 100644 --- a/cwms/api.py +++ b/cwms/api.py @@ -32,6 +32,7 @@ import base64 import json import logging +from builtins import BaseException from http import HTTPStatus from json import JSONDecodeError from typing import Any, Optional, cast From b4cac0b50bf7cd695ab8d2b8891510abd82fa603 Mon Sep 17 00:00:00 2001 From: Charles Graham SWT Date: Wed, 25 Mar 2026 07:46:24 -0700 Subject: [PATCH 4/5] Remove baseexception --- cwms/api.py | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/cwms/api.py b/cwms/api.py index 0efe814..4f82099 100644 --- a/cwms/api.py +++ b/cwms/api.py @@ -32,7 +32,6 @@ import base64 import json import logging -from builtins import BaseException from http import HTTPStatus from json import JSONDecodeError from typing import Any, Optional, cast @@ -145,16 +144,22 @@ class PermissionError(ApiError): def _unwrap_retry_error(error: RequestsRetryError) -> Exception: """Return the original retry cause when requests wraps it in RetryError.""" - current: BaseException = error - while getattr(current, "__cause__", None) is not None: - current = current.__cause__ + current: Exception = error + cause = error.__cause__ + while isinstance(cause, Exception): + current = cause + cause = cause.__cause__ if current is error and error.args: - current = error.args[0] - while getattr(current, "reason", None) is not None: - current = current.reason - - return current if isinstance(current, Exception) else error + first_arg = error.args[0] + if isinstance(first_arg, Exception): + current = first_arg + reason = getattr(current, "reason", None) + while isinstance(reason, Exception): + current = reason + reason = getattr(current, "reason", None) + + return current def init_session( From 6fe25df4125195bef9722e748cf3883bfd1321cc Mon Sep 17 00:00:00 2001 From: Charles Graham SWT Date: Fri, 27 Mar 2026 09:55:28 -0700 Subject: [PATCH 5/5] DRY - test endpoint --- tests/mock/api_test.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/mock/api_test.py b/tests/mock/api_test.py index 7e57233..1b27d4e 100644 --- a/tests/mock/api_test.py +++ b/tests/mock/api_test.py @@ -3,7 +3,9 @@ from urllib3.exceptions import MaxRetryError, ResponseError import cwms.api -from cwms.api import SESSION, ApiError, InvalidVersion, api_version_text, init_session +from cwms.api import SESSION, ApiError, api_version_text, init_session + +TEST_ENDPOINT = "/test-endpoint" def test_session_default(): @@ -90,7 +92,7 @@ def post(self, endpoint, params=None, headers=None, data=None): monkeypatch.setattr(cwms.api, "SESSION", SessionStub()) with pytest.raises(ApiError) as error: - cwms.api._post_function(endpoint="/test-endpoint", data={}) + cwms.api._post_function(endpoint=TEST_ENDPOINT, data={}) assert error.value.response.status_code == 500 assert "Internal Server Error" in str(error.value) @@ -102,7 +104,7 @@ def test_retry_error_unwraps_original_cause(monkeypatch): original_error = ResponseError("too many 503 error responses") wrapped_error = RequestsRetryError( - MaxRetryError(pool=None, url="/test-endpoint", reason=original_error) + MaxRetryError(pool=None, url=TEST_ENDPOINT, reason=original_error) ) class SessionStub: @@ -112,4 +114,4 @@ def get(self, endpoint, params=None, headers=None): monkeypatch.setattr(cwms.api, "SESSION", SessionStub()) with pytest.raises(ResponseError, match="too many 503 error responses"): - cwms.api.get("/test-endpoint") + cwms.api.get(TEST_ENDPOINT)