Skip to content

refactor(api): consolidate HTTP boilerplate, thread &Client through #15

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
I-am-nothing merged 1 commit into from
May 4, 2026
Merged

refactor(api): consolidate HTTP boilerplate, thread &Client through #15

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
166 changes: 116 additions & 50 deletions src/api.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
//! Authenticated calls to the api-platform account service. Endpoints
//! that require a session expect `Authorization: Bearer <access_token>`.

use std::io::Read;
use std::time::Duration;

//! Authenticated calls to the api-platform account and applications
//! services. Endpoints that require a session expect
//! `Authorization: Bearer <access_token>`.
//!
//! Functions accept a `&Client` so a single command builds one client
//! and reuses its connection pool across multiple calls (e.g. `me` +
//! `get_module` + `create_module` from `module init`).

use reqwest::blocking::{Client, Response};
use serde::{Deserialize, Serialize};
use thiserror::Error;

use crate::http;

const MAX_RESPONSE_BYTES: u64 = 64 * 1024;

#[derive(Debug, Deserialize)]
#[allow(dead_code)] // profile_url is part of the API surface; whoami doesn't print it yet
pub struct Identity {
Expand Down Expand Up @@ -75,9 +76,8 @@ pub struct CreateModuleInput<'a> {
}

/// GET /v1/auth/me — returns the authenticated user's identity.
pub fn me(api_base: &str, access_token: &str) -> Result<Identity, ApiError> {
pub fn me(http: &Client, api_base: &str, access_token: &str) -> Result<Identity, ApiError> {
let endpoint = format!("{}/v1/auth/me", api_base.trim_end_matches('/'));
let http = http::client(Duration::from_secs(15))?;

let resp = http
.get(&endpoint)
Expand All @@ -86,26 +86,13 @@ pub fn me(api_base: &str, access_token: &str) -> Result<Identity, ApiError> {
.send()?;

let status = resp.status();

if status.is_success() {
return Ok(resp.json::<Identity>()?);
}
if status == reqwest::StatusCode::UNAUTHORIZED || status == reqwest::StatusCode::FORBIDDEN {
return Err(ApiError::Unauthenticated);
}
// Bound the error-path body so a hostile endpoint can't OOM us.
let mut body = Vec::with_capacity(1024);
resp.take(MAX_RESPONSE_BYTES)
.read_to_end(&mut body)
.map_err(|e| ApiError::Unexpected {
status: status.as_u16(),
body: format!("(read body failed: {e})"),
})?;
let body = String::from_utf8_lossy(&body).into_owned();
Err(ApiError::Unexpected {
status: status.as_u16(),
body,
})
Err(unexpected_body_error(resp))
}

/// GET /v1/modules/{slug} — returns the caller's module by slug.
Expand All @@ -114,14 +101,14 @@ pub fn me(api_base: &str, access_token: &str) -> Result<Identity, ApiError> {
/// per-owner, so 404 here does NOT mean the platform-wide name is unique
/// (reserved/format checks still happen on POST).
pub fn get_module(
http: &Client,
apps_base: &str,
access_token: &str,
slug: &str,
) -> Result<Option<Module>, ApiError> {
// Slug is pre-validated against `^[a-z][a-z0-9-]{1,38}[a-z0-9]$` before
// this call, so it's URL-safe with no encoding needed.
let endpoint = format!("{}/v1/modules/{}", apps_base.trim_end_matches('/'), slug);
let http = http::client(Duration::from_secs(15))?;

let resp = http
.get(&endpoint)
Expand All @@ -139,27 +126,17 @@ pub fn get_module(
if status == reqwest::StatusCode::UNAUTHORIZED || status == reqwest::StatusCode::FORBIDDEN {
return Err(ApiError::Unauthenticated);
}
let mut body = Vec::with_capacity(1024);
resp.take(MAX_RESPONSE_BYTES)
.read_to_end(&mut body)
.map_err(|e| ApiError::Unexpected {
status: status.as_u16(),
body: format!("(read body failed: {e})"),
})?;
Err(ApiError::Unexpected {
status: status.as_u16(),
body: String::from_utf8_lossy(&body).into_owned(),
})
Err(unexpected_body_error(resp))
}

/// POST /v1/modules — create a developer-owned module.
pub fn create_module(
http: &Client,
apps_base: &str,
access_token: &str,
input: &CreateModuleInput,
) -> Result<Module, ApiError> {
let endpoint = format!("{}/v1/modules", apps_base.trim_end_matches('/'));
let http = http::client(Duration::from_secs(15))?;

let resp = http
.post(&endpoint)
Expand All @@ -179,33 +156,59 @@ pub fn create_module(
// 4xx with platform error-envelope: surface code + message so callers
// can branch on `slug_taken` / `slug_reserved` / `slug_invalid` without
// re-parsing the body.
let mut body = Vec::with_capacity(1024);
resp.take(MAX_RESPONSE_BYTES)
.read_to_end(&mut body)
.map_err(|e| ApiError::Unexpected {
status: status.as_u16(),
body: format!("(read body failed: {e})"),
})?;
let status_u16 = status.as_u16();
let body = match http::read_capped(resp) {
Ok(b) => b,
Err(e) => {
return Err(ApiError::Unexpected {
status: status_u16,
body: format!("(read body failed: {e})"),
});
}
};
if let Ok(env) = serde_json::from_slice::<ErrorEnvelope>(&body) {
return Err(ApiError::Server {
status: status.as_u16(),
status: status_u16,
code: env.error.code,
message: env.error.message,
});
}
Err(ApiError::Unexpected {
status: status.as_u16(),
status: status_u16,
body: String::from_utf8_lossy(&body).into_owned(),
})
}

/// Common tail for unexpected (non-success, non-typed) responses: read
/// the body with [`http::read_capped`] and wrap as `ApiError::Unexpected`.
/// If reading fails, the io error is folded into the body for diagnosis.
fn unexpected_body_error(resp: Response) -> ApiError {
let status = resp.status().as_u16();
match http::read_capped(resp) {
Ok(bytes) => ApiError::Unexpected {
status,
body: String::from_utf8_lossy(&bytes).into_owned(),
},
Err(e) => ApiError::Unexpected {
status,
body: format!("(read body failed: {e})"),
},
}
}

#[cfg(test)]
mod tests {
use super::*;

use std::time::Duration;

use mockito::Server;
use serde_json::json;

fn test_client() -> Client {
http::client(Duration::from_secs(15)).expect("client")
}

#[test]
fn me_success() {
let mut server = Server::new();
Expand All @@ -225,7 +228,7 @@ mod tests {
)
.create();

let id = me(&server.url(), "AT").expect("ok");
let id = me(&test_client(), &server.url(), "AT").expect("ok");
assert_eq!(id.email, "user@example.com");
assert_eq!(id.slug.as_deref(), Some("test-user"));
}
Expand All @@ -239,10 +242,29 @@ mod tests {
.with_body(r#"{"error":{"code":"token_invalid"}}"#)
.create();

let err = me(&server.url(), "expired").unwrap_err();
let err = me(&test_client(), &server.url(), "expired").unwrap_err();
assert!(matches!(err, ApiError::Unauthenticated), "got {err:?}");
}

#[test]
fn me_5xx_is_unexpected_with_body() {
let mut server = Server::new();
let _m = server
.mock("GET", "/v1/auth/me")
.with_status(503)
.with_body("upstream timeout")
.create();

let err = me(&test_client(), &server.url(), "AT").unwrap_err();
match err {
ApiError::Unexpected { status, body } => {
assert_eq!(status, 503);
assert!(body.contains("upstream timeout"), "got body {body:?}");
}
other => panic!("expected Unexpected, got {other:?}"),
}
}

#[test]
fn create_module_success() {
let mut server = Server::new();
Expand All @@ -266,6 +288,7 @@ mod tests {
.create();

let m = create_module(
&test_client(),
&server.url(),
"AT",
&CreateModuleInput {
Expand Down Expand Up @@ -296,7 +319,7 @@ mod tests {
)
.create();

let m = get_module(&server.url(), "AT", "media").expect("ok");
let m = get_module(&test_client(), &server.url(), "AT", "media").expect("ok");
assert!(m.is_some());
assert_eq!(m.unwrap().slug, "media");
}
Expand All @@ -306,13 +329,27 @@ mod tests {
let mut server = Server::new();
let _m = server
.mock("GET", "/v1/modules/none")
.match_header("authorization", "Bearer AT")
.with_status(404)
.create();

let m = get_module(&server.url(), "AT", "none").expect("ok");
let m = get_module(&test_client(), &server.url(), "AT", "none").expect("ok");
assert!(m.is_none());
}

#[test]
fn get_module_401_is_unauthenticated() {
let mut server = Server::new();
let _m = server
.mock("GET", "/v1/modules/forbidden-slug")
.with_status(401)
.create();

let err =
get_module(&test_client(), &server.url(), "expired", "forbidden-slug").unwrap_err();
assert!(matches!(err, ApiError::Unauthenticated), "got {err:?}");
}

#[test]
fn create_module_409_surfaces_code() {
let mut server = Server::new();
Expand All @@ -325,6 +362,7 @@ mod tests {
.create();

let err = create_module(
&test_client(),
&server.url(),
"AT",
&CreateModuleInput {
Expand All @@ -341,4 +379,32 @@ mod tests {
other => panic!("expected Server, got {other:?}"),
}
}

#[test]
fn create_module_4xx_without_envelope_is_unexpected() {
let mut server = Server::new();
let _m = server
.mock("POST", "/v1/modules")
.with_status(400)
.with_body("not json")
.create();

let err = create_module(
&test_client(),
&server.url(),
"AT",
&CreateModuleInput {
name: "x",
slug: "x",
},
)
.unwrap_err();
match err {
ApiError::Unexpected { status, body } => {
assert_eq!(status, 400);
assert!(body.contains("not json"), "got body {body:?}");
}
other => panic!("expected Unexpected, got {other:?}"),
}
}
}
18 changes: 4 additions & 14 deletions src/auth/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@
//! displayed on the consent page back into the terminal. Custom-scheme
//! and per-OS handler registration is the follow-up tracked at #7.

use std::io::Read;

use base64::Engine;
use base64::engine::general_purpose::URL_SAFE_NO_PAD;
use rand::TryRngCore;
Expand All @@ -17,10 +15,7 @@ use sha2::{Digest, Sha256};
use thiserror::Error;
use url::Url;

/// Cap on response bodies we'll deserialize. /token and /me return
/// sub-1KB JSON in practice; 64 KiB is generous slack and protects
/// against a hostile endpoint trying to OOM the CLI.
const MAX_RESPONSE_BYTES: u64 = 64 * 1024;
use crate::http;

/// Identifies the CLI to the platform; matches the seed in
/// api-platform migration `011_oauth.up.sql`.
Expand Down Expand Up @@ -176,15 +171,10 @@ fn random_b64url(n: usize) -> Result<String, AuthError> {
Ok(URL_SAFE_NO_PAD.encode(buf))
}

/// Read a response body into memory with a hard size cap. Truncated
/// bodies are returned as-is — the JSON parse downstream will fail
/// loudly rather than silently misinterpret a partial document.
/// Wrap [`http::read_capped`] with auth-specific error mapping.
fn read_capped(resp: Response) -> Result<String, AuthError> {
let mut buf = Vec::with_capacity(1024);
resp.take(MAX_RESPONSE_BYTES)
.read_to_end(&mut buf)
.map_err(|e| AuthError::Decode(e.to_string()))?;
String::from_utf8(buf).map_err(|e| AuthError::Decode(e.to_string()))
let bytes = http::read_capped(resp).map_err(|e| AuthError::Decode(e.to_string()))?;
String::from_utf8(bytes).map_err(|e| AuthError::Decode(e.to_string()))
}

#[cfg(test)]
Expand Down
7 changes: 5 additions & 2 deletions src/commands/module.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use indicatif::{ProgressBar, ProgressStyle};

use crate::api::{self, ApiError, CreateModuleInput};
use crate::credentials;
use crate::http;

use super::{
DEFAULT_API_BASE, DEFAULT_APPS_API_BASE, DEFAULT_WEB_BASE, ENV_API_URL, ENV_APPS_API_URL,
Expand Down Expand Up @@ -68,11 +69,12 @@ fn init(args: InitArgs) -> Result<()> {
let api_base = resolve_base(ENV_API_URL, DEFAULT_API_BASE);
let apps_base = resolve_base(ENV_APPS_API_URL, DEFAULT_APPS_API_BASE);
let web_base = resolve_base(ENV_WEB_URL, DEFAULT_WEB_BASE);
let client = http::client(Duration::from_secs(15))?;

// The platform stores ownership by user id, but the CLI surfaces the
// full namespaced `@<username>/<slug>` — so we refuse to POST until the
// caller has claimed a username, and point them at the web flow.
let identity = match api::me(&api_base, &creds.access_token) {
let identity = match api::me(&client, &api_base, &creds.access_token) {
Ok(id) => id,
Err(ApiError::Unauthenticated) => return Err(session_expired()),
Err(e) => return Err(e.into()),
Expand All @@ -95,7 +97,7 @@ fn init(args: InitArgs) -> Result<()> {
// the common "I forgot I made this last week" case before we POST.
// Reserved/invalid still surface server-side from the POST below.
let pre_check = with_spinner("Checking availability…", || {
api::get_module(&apps_base, &creds.access_token, &slug)
api::get_module(&client, &apps_base, &creds.access_token, &slug)
});
match pre_check {
Ok(Some(existing)) if args.used => {
Expand Down Expand Up @@ -132,6 +134,7 @@ fn init(args: InitArgs) -> Result<()> {

let create_result = with_spinner("Creating module…", || {
api::create_module(
&client,
&apps_base,
&creds.access_token,
&CreateModuleInput {
Expand Down
Loading
Loading