Skip to content

Commit 3a36f51

Browse files
hyperpolymathclaude
andcommitted
fix(rgtv-cli): migrate to ureq 3.x API (repairs main; unblocks #87, #88)
PR #82 bumped ureq 2.12.1 → 3.3.0 in /rgtv-cli but did not migrate the call sites, leaving main red since 2026-05-22 (`Check / Fmt / Clippy / Test (rgtv-cli)` failing on every PR including dependabot bumps #87, #88). The TODO in Cargo.toml flagged the remaining work. This commit completes the migration: - AgentBuilder → Agent::config_builder().build().into() - timeout_read/timeout_write → timeout_recv_body/timeout_send_body - http_status_as_error(false) keeps 4xx/5xx as Ok responses so the broker's structured error body is still surfaceable - RequestBuilder::set → RequestBuilder::header - Response::into_string / into_json → body_mut().read_to_string() then serde_json::from_str, factored into parse_response() so the broker error-body fast path lives in one place - POST with no body now uses send_empty() (call() is WithoutBody-only in v3) - ureq_err() simplified to the transport-error path; HTTP-status errors flow through parse_response() Verified locally (matches the Rust CI pipeline): - cargo fmt --check OK - cargo check --all-targets OK - cargo clippy --all-targets -- -D warnings OK - cargo test OK After this lands, `@dependabot rebase` on #87 (nixpkgs) and #88 (rust-overlay) should bring them green; the only remaining failing check on those PRs is `analyze (javascript-typescript, none)` (CodeQL workflow drift, not caused by the dependabot bumps). Foundational follow-up (NOT in this PR, flagged for owner action): the underlying reason a red-CI dependabot PR ever reached main is that `main` branch protection has no `required_status_checks` block — see `gh api repos/.../branches/main/protection`. Adding the Rust CI checks to required status checks would prevent another silent red-bump merge. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 312ebe1 commit 3a36f51

2 files changed

Lines changed: 40 additions & 26 deletions

File tree

rgtv-cli/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ path = "src/main.rs"
1414

1515
[dependencies]
1616
# Synchronous HTTP client — no async overhead needed for a CLI.
17-
ureq = { version = "3", features = ["json"] } # TODO: migrate to ureq 3.x API (AgentBuilder/Error/RequestBuilder all renamed)
17+
ureq = { version = "3", features = ["json"] }
1818
serde = { version = "1.0", features = ["derive"] }
1919
serde_json = "1.0"
2020
# Zeroize heap allocations holding raw credential values after use.

rgtv-cli/src/main.rs

Lines changed: 39 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -109,64 +109,78 @@ impl Config {
109109
// ---------------------------------------------------------------------------
110110

111111
/// A short-timeout ureq agent appropriate for interactive CLI calls.
112+
///
113+
/// `http_status_as_error(false)` keeps 4xx/5xx as `Ok` responses so the
114+
/// broker's structured error body can still be surfaced (see `parse_response`).
112115
fn http_agent() -> ureq::Agent {
113-
ureq::AgentBuilder::new()
114-
.timeout_read(Duration::from_secs(10))
115-
.timeout_write(Duration::from_secs(5))
116+
ureq::Agent::config_builder()
117+
.timeout_recv_body(Some(Duration::from_secs(10)))
118+
.timeout_send_body(Some(Duration::from_secs(5)))
119+
.http_status_as_error(false)
116120
.build()
121+
.into()
117122
}
118123

119124
fn bearer(token: &str) -> String {
120125
format!("Bearer {token}")
121126
}
122127

123-
/// Convert a ureq error into a human-readable string, surfacing the broker's
124-
/// own error message when it returns a structured error body.
128+
/// Convert a transport-level ureq error into a human-readable string.
129+
///
130+
/// HTTP-status errors don't reach this path (see `http_agent`) — they come
131+
/// back as `Ok` responses whose status code is checked by `parse_response`.
125132
fn ureq_err(e: ureq::Error) -> String {
126-
match e {
127-
ureq::Error::Status(code, resp) => {
128-
let body = resp.into_string().unwrap_or_default();
129-
if let Ok(b) = serde_json::from_str::<BrokerError>(&body) {
130-
format!("broker {code}: {}", b.error)
131-
} else {
132-
format!("broker {code}: {body}")
133-
}
133+
format!("transport: {e}")
134+
}
135+
136+
/// Parse a response, surfacing the broker's structured error body on non-2xx.
137+
fn parse_response<T: serde::de::DeserializeOwned>(
138+
mut resp: ureq::http::Response<ureq::Body>,
139+
what: &str,
140+
) -> Result<T, String> {
141+
let status = resp.status().as_u16();
142+
let body = resp
143+
.body_mut()
144+
.read_to_string()
145+
.map_err(|e| format!("read {what} body: {e}"))?;
146+
if !(200..300).contains(&status) {
147+
if let Ok(b) = serde_json::from_str::<BrokerError>(&body) {
148+
return Err(format!("broker {status}: {}", b.error));
134149
}
135-
ureq::Error::Transport(t) => format!("transport: {t}"),
150+
return Err(format!("broker {status}: {body}"));
136151
}
152+
serde_json::from_str(&body).map_err(|e| format!("parse {what}: {e}"))
137153
}
138154

139155
fn get_health(cfg: &Config) -> Result<HealthResponse, String> {
140156
let url = format!("{}/health", cfg.base_url);
141157
let resp = http_agent()
142158
.get(&url)
143-
.set("Authorization", &bearer(&cfg.agent_token))
159+
.header("Authorization", bearer(&cfg.agent_token))
144160
.call()
145161
.map_err(ureq_err)?;
146-
resp.into_json().map_err(|e| format!("parse health: {e}"))
162+
parse_response(resp, "health")
147163
}
148164

149165
fn get_credentials(cfg: &Config) -> Result<CredentialsResponse, String> {
150166
let url = format!("{}/v1/credentials", cfg.base_url);
151167
let resp = http_agent()
152168
.get(&url)
153-
.set("Authorization", &bearer(&cfg.agent_token))
169+
.header("Authorization", bearer(&cfg.agent_token))
154170
.call()
155171
.map_err(ureq_err)?;
156-
resp.into_json()
157-
.map_err(|e| format!("parse credentials: {e}"))
172+
parse_response(resp, "credentials")
158173
}
159174

160175
fn post_grant(cfg: &Config, hint: &str) -> Result<GrantResponse, String> {
161176
let url = format!("{}/v1/grants", cfg.base_url);
162177
let body = serde_json::json!({ "hint": hint });
163178
let resp = http_agent()
164179
.post(&url)
165-
.set("Authorization", &bearer(&cfg.agent_token))
166-
.set("Content-Type", "application/json")
180+
.header("Authorization", bearer(&cfg.agent_token))
167181
.send_json(body)
168182
.map_err(ureq_err)?;
169-
resp.into_json().map_err(|e| format!("parse grant: {e}"))
183+
parse_response(resp, "grant")
170184
}
171185

172186
/// Redeem a grant and return the credential value wrapped in Zeroizing<String>
@@ -175,10 +189,10 @@ fn post_redeem(cfg: &Config, grant_id: &str) -> Result<Zeroizing<String>, String
175189
let url = format!("{}/v1/grants/{grant_id}/redeem", cfg.base_url);
176190
let resp = http_agent()
177191
.post(&url)
178-
.set("Authorization", &bearer(&cfg.agent_token))
179-
.call()
192+
.header("Authorization", bearer(&cfg.agent_token))
193+
.send_empty()
180194
.map_err(ureq_err)?;
181-
let body: RedeemBody = resp.into_json().map_err(|e| format!("parse redeem: {e}"))?;
195+
let body: RedeemBody = parse_response(resp, "redeem")?;
182196
Ok(Zeroizing::new(body.value))
183197
}
184198

0 commit comments

Comments
 (0)