Skip to content

Retry rate-limited API requests, and stop when an API token's organization can't be looked up - #2407

Merged
jeremybeard merged 1 commit into
mainfrom
setup-errors
Oct 22, 2026
Merged

jeremybeard merged 1 commit into
mainfrom
setup-errors

Conversation

@jeremybeard

@jeremybeard jeremybeard commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Description

When the API token's organization lookup fails, the command now stops with that error, and the v1 and v1alpha1 clients retry 429s up to three times with a jittered 1–2s backoff.

An e2e job in astronomer/deploy-action ran several jobs on one token at once and hit the API's per-token rate limit. ListOrganizations got a 429 from the gateway, so the context was never given an organization. The pre-run hook only logs setup errors, so astro workspace switch went on to request /v1/organizations//workspaces and failed with Invalid URL: organizationId cannot be empty.

Only 429s are retried, since the gateway refuses those before the API sees them, so a write is never sent twice. All other setup errors are still only logged, so a value in ASTRO_API_TOKEN that isn't an API token still falls back to the existing login (#2217).

🎟 Issue(s)

🧪 Functional Testing

Unit tests cover the retry against an httptest server: the body is resent, the final 429 is returned once retries run out, other statuses aren't retried, and a cancelled request stops waiting. They also cover the hook stopping the command on a failed organization lookup and still only logging the other setup errors.

📸 Screenshots

📋 Checklist

  • Rebased from the main (or release if patching) branch (before testing)
  • Ran make test before taking out of draft
  • Ran make lint before taking out of draft
  • Added/updated applicable tests
  • Tested against Astro-API (if necessary).
  • Tested against Houston-API and Astronomer (if necessary).
  • Communicated to/tagged owners of respective clients potentially impacted by these changes.
  • Updated any related documentation

…ation can't be looked up

Co-Authored-By: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added the target:main PR targets main label Oct 21, 2026
@coveralls-official

Copy link
Copy Markdown

Coverage Report for CI Build 39617783146

Coverage increased (+0.03%) to 45.355%

Details

  • Coverage increased (+0.03%) from the base build.
  • Patch coverage: 3 uncovered changes across 2 files (21 of 24 lines covered, 87.5%).
  • No coverage regressions found.

Uncovered Changes

File Changed Covered %
pkg/httputil/retry.go 17 15 88.24%
astro-client-v1/client.go 1 0 0.0%
Total (5 files) 24 21 87.5%

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 60686
Covered Lines: 27524
Line Coverage: 45.35%
Coverage Strength: 10.25 hits per line

💛 - Coveralls

@jeremybeard
jeremybeard marked this pull request as ready for review October 21, 2026 14:42
@jeremybeard
jeremybeard requested a review from a team as a code owner October 21, 2026 14:42
@jeremybeard
jeremybeard merged commit f7b5b11 into main Oct 22, 2026
10 checks passed
@jeremybeard
jeremybeard deleted the setup-errors branch October 22, 2026 01:04
schnie added a commit that referenced this pull request Oct 22, 2026
Retries: a Retry-After wait is capped at 10s rather than taken as given;
the retry check no longer returns an error beside a response, which left
the response unclosed; and the retrying client closes only its own idle
connections, not http.DefaultTransport's, which every client shares.

Organization lookup: checkAPIToken and checkAPIKeys share one lookup, so
an empty list or an organization with no product is
ErrAPITokenOrganization rather than a panic, and API keys stop on a
failed lookup as an API token does. The product cannot be skipped: an
environment login keeps it only for the process, and it is what tells a
hosted organization from a hybrid one.

astro auth token still skips a lookup that failed for want of the API,
but not one the API refused with 401 or 403: that token no longer works.
astro api registry, which is public, needs no login at all.

Co-Authored-By: Claude <noreply@anthropic.com>
schnie added a commit that referenced this pull request Oct 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

target:main PR targets main

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants