From 1cbf2375aab25971790b04da4e17ba64ac4e3b7d Mon Sep 17 00:00:00 2001 From: Paul Thurlow Date: Fri, 17 Apr 2026 17:05:02 -0700 Subject: [PATCH] change to sandbox api --- src/api.rs | 151 ++++++++++++++++++++++++++++++++++++++++++++++--- src/sandbox.rs | 18 +++--- 2 files changed, 151 insertions(+), 18 deletions(-) diff --git a/src/api.rs b/src/api.rs index a3a5a7e..4a1a99f 100644 --- a/src/api.rs +++ b/src/api.rs @@ -1,3 +1,4 @@ +use crate::auth; use crate::config; use crate::util; use crossterm::style::Stylize; @@ -57,7 +58,9 @@ impl ApiClient { headers.push(("X-Workspace-Id", ws.clone())); } if let Some(ref sid) = self.sandbox_id { + // Send both headers during the session→sandbox migration window. headers.push(("X-Session-Id", sid.clone())); + headers.push(("X-Sandbox-Id", sid.clone())); } headers } @@ -68,6 +71,19 @@ impl ApiClient { util::debug_request(method, url, &header_refs, body); } + /// Prints an error for a non-2xx response and exits. On 4xx, first re-probes + /// the API key: if it's actually invalid, a clear re-auth hint is shown + /// instead of whatever cryptic body the primary endpoint returned. + fn fail_response(&self, status: reqwest::StatusCode, body: String) -> ! { + let auth_status = if status.is_client_error() { + config::load("default").ok().map(|pc| auth::check_status(&pc)) + } else { + None + }; + eprintln!("{}", format_fail_message(status, &body, auth_status.as_ref()).red()); + std::process::exit(1); + } + fn build_request(&self, method: reqwest::Method, url: &str) -> reqwest::blocking::RequestBuilder { let mut req = self.client.request(method, url) .header("Authorization", format!("Bearer {}", self.api_key)); @@ -75,7 +91,9 @@ impl ApiClient { req = req.header("X-Workspace-Id", ws); } if let Some(ref sid) = self.sandbox_id { + // Send both headers during the session→sandbox migration window. req = req.header("X-Session-Id", sid); + req = req.header("X-Sandbox-Id", sid); } req } @@ -99,8 +117,7 @@ impl ApiClient { let (status, body) = util::debug_response(resp); if !status.is_success() { - eprintln!("{}", util::api_error(body).red()); - std::process::exit(1); + self.fail_response(status, body); } match serde_json::from_str(&body) { @@ -127,8 +144,7 @@ impl ApiClient { let (status, body) = util::debug_response(resp); if !status.is_success() { - eprintln!("{}", util::api_error(body).red()); - std::process::exit(1); + self.fail_response(status, body); } match serde_json::from_str(&body) { @@ -158,8 +174,7 @@ impl ApiClient { let (status, resp_body) = util::debug_response(resp); if !status.is_success() { - eprintln!("{}", util::api_error(resp_body).red()); - std::process::exit(1); + self.fail_response(status, resp_body); } match serde_json::from_str(&resp_body) { @@ -226,8 +241,7 @@ impl ApiClient { let (status, resp_body) = util::debug_response(resp); if !status.is_success() { - eprintln!("{}", util::api_error(resp_body).red()); - std::process::exit(1); + self.fail_response(status, resp_body); } match serde_json::from_str(&resp_body) { @@ -269,3 +283,124 @@ impl ApiClient { } } + +/// Decide what error text to print for a failed response. Pulled out as a pure +/// function so the 4xx-to-re-auth-hint logic can be unit-tested without +/// making real HTTP calls or touching `std::process::exit`. +fn format_fail_message( + status: reqwest::StatusCode, + body: &str, + auth_status: Option<&auth::AuthStatus>, +) -> String { + if status.is_client_error() { + if let Some(auth::AuthStatus::Invalid(_)) = auth_status { + return "error: API key is invalid. Run 'hotdata auth' to re-authenticate.".to_string(); + } + } + util::api_error(body.to_string()) +} + +#[cfg(test)] +mod tests { + use super::*; + use auth::AuthStatus; + + #[test] + fn format_fail_message_401_with_invalid_key_shows_reauth_hint() { + let msg = format_fail_message( + reqwest::StatusCode::UNAUTHORIZED, + "", + Some(&AuthStatus::Invalid(401)), + ); + assert!(msg.contains("API key is invalid")); + assert!(msg.contains("hotdata auth")); + } + + #[test] + fn format_fail_message_404_with_invalid_key_shows_reauth_hint() { + // This is the user-reported scenario: the server masks an auth failure + // behind a 404 with an empty body. The re-auth probe catches it. + let msg = format_fail_message( + reqwest::StatusCode::NOT_FOUND, + "", + Some(&AuthStatus::Invalid(401)), + ); + assert!(msg.contains("API key is invalid"), "got: {msg}"); + } + + #[test] + fn format_fail_message_404_with_valid_key_shows_real_error() { + // If the auth probe says the key is fine, surface the upstream body. + let body = r#"{"error":{"message":"Query run 'qrun_notreal' not found"}}"#; + let msg = format_fail_message( + reqwest::StatusCode::NOT_FOUND, + body, + Some(&AuthStatus::Authenticated), + ); + assert!(!msg.contains("API key is invalid")); + assert!(msg.contains("Query run 'qrun_notreal' not found")); + } + + #[test] + fn format_fail_message_400_with_valid_key_shows_real_error() { + let body = r#"{"error":{"message":"invalid_sql"}}"#; + let msg = format_fail_message( + reqwest::StatusCode::BAD_REQUEST, + body, + Some(&AuthStatus::Authenticated), + ); + assert_eq!(msg, "invalid_sql"); + } + + #[test] + fn format_fail_message_5xx_never_shows_reauth_hint() { + // 5xx is not a client error — the auth probe is not even run, so + // `auth_status` is None from the caller and we just surface the body. + let msg = format_fail_message( + reqwest::StatusCode::INTERNAL_SERVER_ERROR, + "server exploded", + None, + ); + assert!(!msg.contains("API key is invalid")); + assert_eq!(msg, "server exploded"); + } + + #[test] + fn format_fail_message_4xx_connection_error_on_probe_falls_through() { + // If the probe itself couldn't reach the API, we can't claim the key + // is invalid — surface the original body instead. + let body = r#"{"error":{"message":"forbidden"}}"#; + let msg = format_fail_message( + reqwest::StatusCode::FORBIDDEN, + body, + Some(&AuthStatus::ConnectionError("tcp reset".to_string())), + ); + assert!(!msg.contains("API key is invalid")); + assert_eq!(msg, "forbidden"); + } + + #[test] + fn format_fail_message_4xx_no_probe_result_falls_through() { + // Caller couldn't load config (None) — still surface the upstream error. + let body = "plain body"; + let msg = format_fail_message( + reqwest::StatusCode::NOT_FOUND, + body, + None, + ); + assert!(!msg.contains("API key is invalid")); + assert_eq!(msg, "plain body"); + } + + #[test] + fn format_fail_message_4xx_authenticated_probe_shows_server_message() { + // Valid key but a genuine client error — upstream message wins. + let body = r#"{"error":{"message":"workspace_not_found"}}"#; + let msg = format_fail_message( + reqwest::StatusCode::NOT_FOUND, + body, + Some(&AuthStatus::Authenticated), + ); + assert_eq!(msg, "workspace_not_found"); + } +} diff --git a/src/sandbox.rs b/src/sandbox.rs index d1251dc..e25f144 100644 --- a/src/sandbox.rs +++ b/src/sandbox.rs @@ -14,19 +14,17 @@ struct Sandbox { #[derive(Deserialize)] struct ListResponse { - #[serde(rename = "sessions")] sandboxes: Vec, } #[derive(Deserialize)] struct DetailResponse { - #[serde(rename = "session")] sandbox: Sandbox, } pub fn list(workspace_id: &str, format: &str) { let api = ApiClient::new(Some(workspace_id)); - let body: ListResponse = api.get("/sessions"); + let body: ListResponse = api.get("/sandboxes"); let current_sandbox = std::env::var("HOTDATA_SANDBOX") .ok() @@ -57,7 +55,7 @@ pub fn list(workspace_id: &str, format: &str) { pub fn get(sandbox_id: &str, workspace_id: &str, format: &str) { let api = ApiClient::new(Some(workspace_id)); - let path = format!("/sessions/{sandbox_id}"); + let path = format!("/sandboxes/{sandbox_id}"); let body: DetailResponse = api.get(&path); let s = &body.sandbox; @@ -82,7 +80,7 @@ pub fn get(sandbox_id: &str, workspace_id: &str, format: &str) { pub fn read(sandbox_id: &str, workspace_id: &str) { let api = ApiClient::new(Some(workspace_id)); - let path = format!("/sessions/{sandbox_id}"); + let path = format!("/sandboxes/{sandbox_id}"); let body: DetailResponse = api.get(&path); if body.sandbox.markdown.is_empty() { eprintln!("{}", "Sandbox markdown is empty.".dark_grey()); @@ -139,7 +137,7 @@ pub fn new(workspace_id: &str, name: Option<&str>, format: &str) { body["name"] = serde_json::json!(n); } - let resp: DetailResponse = api.post("/sessions", &body); + let resp: DetailResponse = api.post("/sandboxes", &body); let s = &resp.sandbox; // Set as the active sandbox in config @@ -173,7 +171,7 @@ pub fn update(workspace_id: &str, sandbox_id: &str, name: Option<&str>, markdown if let Some(n) = name { body["name"] = serde_json::json!(n); } if let Some(m) = markdown { body["markdown"] = serde_json::json!(m); } - let path = format!("/sessions/{sandbox_id}"); + let path = format!("/sandboxes/{sandbox_id}"); let resp: DetailResponse = api.patch(&path, &body); let s = &resp.sandbox; @@ -197,7 +195,7 @@ pub fn run(sandbox_id: Option<&str>, workspace_id: &str, name: Option<&str>, cmd Some(id) => { // Verify the sandbox exists let api = ApiClient::new(Some(workspace_id)); - let path = format!("/sessions/{id}"); + let path = format!("/sandboxes/{id}"); let _: DetailResponse = api.get(&path); id.to_string() } @@ -208,7 +206,7 @@ pub fn run(sandbox_id: Option<&str>, workspace_id: &str, name: Option<&str>, cmd if let Some(n) = name { body["name"] = serde_json::json!(n); } - let resp: DetailResponse = api.post("/sessions", &body); + let resp: DetailResponse = api.post("/sandboxes", &body); resp.sandbox.public_id } }; @@ -254,7 +252,7 @@ pub fn set(sandbox_id: Option<&str>, workspace_id: &str) { Some(id) => { // Verify the sandbox exists by fetching it let api = ApiClient::new(Some(workspace_id)); - let path = format!("/sessions/{id}"); + let path = format!("/sandboxes/{id}"); let _: DetailResponse = api.get(&path); if let Err(e) = config::save_sandbox("default", id) {