From 06cb0b428d2b9ef745f739baaf36babd58ce76f4 Mon Sep 17 00:00:00 2001 From: Alon Yeshurun Date: Thu, 16 Jul 2026 09:59:00 +0300 Subject: [PATCH] fix: handle HTTP 429 responses missing the Retry-After header Creating a dataflow (and other long-running, throttling-prone operations) could fail with "[UnexpectedError] retry-after" when Fabric returned a 429 without a Retry-After header. The 429 handler read the header via direct subscript, raising a KeyError that escaped the request exception handler and surfaced as an unexpected error. Use the existing get_polling_interval helper, which reads the header case-insensitively and falls back to the default polling interval when it is missing or non-numeric. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a4b6c3e7-7425-4fa7-9f0a-ac7e87377ee2 --- .../unreleased/fixed-20260716-092645.yaml | 6 ++++ src/fabric_cli/client/fab_api_client.py | 2 +- tests/test_core/test_fab_api_client.py | 35 +++++++++++++++++++ 3 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 .changes/unreleased/fixed-20260716-092645.yaml diff --git a/.changes/unreleased/fixed-20260716-092645.yaml b/.changes/unreleased/fixed-20260716-092645.yaml new file mode 100644 index 00000000..c456bf24 --- /dev/null +++ b/.changes/unreleased/fixed-20260716-092645.yaml @@ -0,0 +1,6 @@ +kind: fixed +body: Handle HTTP 429 responses missing the Retry-After header instead of failing with an unexpected error +time: 2026-07-16T09:26:45+03:00 +custom: + Author: ayeshurun + AuthorLink: https://github.com/ayeshurun diff --git a/src/fabric_cli/client/fab_api_client.py b/src/fabric_cli/client/fab_api_client.py index 99df81c0..be8cf5c0 100644 --- a/src/fabric_cli/client/fab_api_client.py +++ b/src/fabric_cli/client/fab_api_client.py @@ -188,7 +188,7 @@ def do_request( fab_constant.ERROR_NOT_FOUND, ) case 429: - retry_after = int(response.headers["Retry-After"]) + retry_after = get_polling_interval(response.headers) utils_ui.print_info( f"Rate limit exceeded. {attempt}ยบ retrying attemp in {retry_after} seconds" ) diff --git a/tests/test_core/test_fab_api_client.py b/tests/test_core/test_fab_api_client.py index 488a8b0f..2b16c07f 100644 --- a/tests/test_core/test_fab_api_client.py +++ b/tests/test_core/test_fab_api_client.py @@ -310,6 +310,41 @@ def __init__(self): assert "ErrorCode" == excinfo.value.status_code +@patch.object(FabAuth(), "get_access_token", return_value="dummy-token") +def test_do_request_429_without_retry_after_header_retries_with_default_interval( + mock_get_token, +): + """A 429 response missing the Retry-After header should fall back to the default + polling interval and retry instead of raising an unexpected KeyError.""" + + class DummyResponse: + def __init__(self, status_code, headers=None, text=""): + self.status_code = status_code + self.text = text + self.content = text.encode() + self.headers = headers if headers is not None else {} + + # First response: throttled (429) with NO Retry-After header. + # Second response: success (200), so the retry loop can complete. + throttled = DummyResponse(429) + success = DummyResponse(200, text="{}") + + dummy_args = Namespace() + dummy_args.uri = f"workspaces/{str(uuid.uuid4())}/items" + dummy_args.method = "get" + dummy_args.audience = None + + with ( + patch("requests.Session.request", side_effect=[throttled, success]), + patch("fabric_cli.client.fab_api_client.time.sleep") as mock_sleep, + ): + response = do_request(dummy_args, hostname="custom.hostname.com") + + assert response.status_code == 200 + # Retried using the default polling interval (10s) rather than raising. + mock_sleep.assert_called_once_with(10) + + @pytest.mark.parametrize( "host_app_env, host_app_version_env, expected_suffix", [