Skip to content

fix(auth): use the timeout-bounded HTTP client for workspace fetch and auth probe - #233

Merged
eddietejeda merged 2 commits into
mainfrom
fix/timeout-remaining-clients
Jul 20, 2026
Merged

fix(auth): use the timeout-bounded HTTP client for workspace fetch and auth probe#233
eddietejeda merged 2 commits into
mainfrom
fix/timeout-remaining-clients

Conversation

@eddietejeda

Copy link
Copy Markdown
Contributor

Second follow-up from #230. cache_workspaces, api_key_authorized_workspaces, and credentials::check_status used bare reqwest::blocking::Client::new(), which has no request timeout — a stalled server could hang auth login or the 4xx auth-status probe (which runs inside error printing via format_fail_message) indefinitely.

They now share raw_http::build_http_client() (300s cap + TCP keepalive), the same client #230 adopted for the token endpoints — this removes the last un-timeouted production HTTP paths in the CLI. The three remaining Client::new() sites are #[cfg(test)] helpers hitting localhost, left as-is.

Full suite: 308 passed, 0 failed.

eddietejeda and others added 2 commits July 20, 2026 09:19
…d auth probe

Follow-up from #230: cache_workspaces, api_key_authorized_workspaces,
and credentials::check_status used bare reqwest::blocking::Client::new(),
which has no request timeout — a stalled server could hang 'auth login'
or the 4xx auth-status probe (which runs inside error printing)
indefinitely. They now share raw_http::build_http_client() (300s cap +
TCP keepalive) like every other raw-HTTP path. Test-only localhost
clients are left as-is.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean drop-in replacement. Verified the three remaining Client::new() sites are all inside the #[cfg(test)] module, consistent with the PR description.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified: the three replaced sites are the production workspace-fetch/auth-probe paths, and the remaining bare Client::new() calls are all under the #[cfg(test)] module (line 555+). Drop-in type-compatible change that closes the un-timeouted paths. LGTM.

@codecov

codecov Bot commented Jul 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@eddietejeda
eddietejeda merged commit 1abd8ba into main Jul 20, 2026
14 checks passed
@eddietejeda
eddietejeda deleted the fix/timeout-remaining-clients branch July 20, 2026 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant