fix: handle HTTP 429 responses missing the Retry-After header#266
Merged
ayeshurun merged 1 commit intoJul 16, 2026
Merged
Conversation
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
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a throttling edge case in fab_api_client.do_request() where HTTP 429 responses without a Retry-After header previously caused an uncaught KeyError("retry-after"), surfacing as an [UnexpectedError] retry-after during long-running/polling operations.
Changes:
- Update the 429 handling path to use the shared
get_polling_interval()helper (header case-insensitive + default fallback). - Add a regression test covering a 429 response missing
Retry-After, asserting the default interval is used and the request retries. - Add a changelog entry documenting the fix.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/fabric_cli/client/fab_api_client.py |
Switch 429 handling from direct header subscript to get_polling_interval() to avoid KeyError and fallback to defaults. |
tests/test_core/test_fab_api_client.py |
Add test ensuring 429 without Retry-After sleeps for default (10s) and retries successfully. |
.changes/unreleased/fixed-20260716-092645.yaml |
Record the behavior fix in unreleased changelog. |
ohadedry
approved these changes
Jul 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
✨ Description of new changes
Summary: Long-running operations failing with
x [UnexpectedError] retry-after.Context: When Fabric throttles a request but returns an HTTP 429 without a
Retry-Afterheader, the rate-limit handler infab_api_client.pyread the header via direct subscript (response.headers["Retry-After"]). Because requests'CaseInsensitiveDictlowercases keys, this raisedKeyError("retry-after"). That error was not caught by therequests.RequestExceptionhandler, so it bubbled up tomain.pyand surfaced as[UnexpectedError] retry-after. Creating a dataflow is a long-running, polling operation that is especially prone to hitting throttling, which is why it reproduced there.The fix reuses the existing
get_polling_interval()helper, which reads theretry-afterheader case-insensitively and falls back to the default polling interval (10s) when the header is missing or non-numeric. This makes the client resilient to throttled 429s and lets it retry as intended instead of erroring out.