Skip to content

Commit 1cbf237

Browse files
committed
change to sandbox api
1 parent 6b81535 commit 1cbf237

2 files changed

Lines changed: 151 additions & 18 deletions

File tree

‎src/api.rs‎

Lines changed: 143 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use crate::auth;
12
use crate::config;
23
use crate::util;
34
use crossterm::style::Stylize;
@@ -57,7 +58,9 @@ impl ApiClient {
5758
headers.push(("X-Workspace-Id", ws.clone()));
5859
}
5960
if let Some(ref sid) = self.sandbox_id {
61+
// Send both headers during the session→sandbox migration window.
6062
headers.push(("X-Session-Id", sid.clone()));
63+
headers.push(("X-Sandbox-Id", sid.clone()));
6164
}
6265
headers
6366
}
@@ -68,14 +71,29 @@ impl ApiClient {
6871
util::debug_request(method, url, &header_refs, body);
6972
}
7073

74+
/// Prints an error for a non-2xx response and exits. On 4xx, first re-probes
75+
/// the API key: if it's actually invalid, a clear re-auth hint is shown
76+
/// instead of whatever cryptic body the primary endpoint returned.
77+
fn fail_response(&self, status: reqwest::StatusCode, body: String) -> ! {
78+
let auth_status = if status.is_client_error() {
79+
config::load("default").ok().map(|pc| auth::check_status(&pc))
80+
} else {
81+
None
82+
};
83+
eprintln!("{}", format_fail_message(status, &body, auth_status.as_ref()).red());
84+
std::process::exit(1);
85+
}
86+
7187
fn build_request(&self, method: reqwest::Method, url: &str) -> reqwest::blocking::RequestBuilder {
7288
let mut req = self.client.request(method, url)
7389
.header("Authorization", format!("Bearer {}", self.api_key));
7490
if let Some(ref ws) = self.workspace_id {
7591
req = req.header("X-Workspace-Id", ws);
7692
}
7793
if let Some(ref sid) = self.sandbox_id {
94+
// Send both headers during the session→sandbox migration window.
7895
req = req.header("X-Session-Id", sid);
96+
req = req.header("X-Sandbox-Id", sid);
7997
}
8098
req
8199
}
@@ -99,8 +117,7 @@ impl ApiClient {
99117

100118
let (status, body) = util::debug_response(resp);
101119
if !status.is_success() {
102-
eprintln!("{}", util::api_error(body).red());
103-
std::process::exit(1);
120+
self.fail_response(status, body);
104121
}
105122

106123
match serde_json::from_str(&body) {
@@ -127,8 +144,7 @@ impl ApiClient {
127144

128145
let (status, body) = util::debug_response(resp);
129146
if !status.is_success() {
130-
eprintln!("{}", util::api_error(body).red());
131-
std::process::exit(1);
147+
self.fail_response(status, body);
132148
}
133149

134150
match serde_json::from_str(&body) {
@@ -158,8 +174,7 @@ impl ApiClient {
158174

159175
let (status, resp_body) = util::debug_response(resp);
160176
if !status.is_success() {
161-
eprintln!("{}", util::api_error(resp_body).red());
162-
std::process::exit(1);
177+
self.fail_response(status, resp_body);
163178
}
164179

165180
match serde_json::from_str(&resp_body) {
@@ -226,8 +241,7 @@ impl ApiClient {
226241

227242
let (status, resp_body) = util::debug_response(resp);
228243
if !status.is_success() {
229-
eprintln!("{}", util::api_error(resp_body).red());
230-
std::process::exit(1);
244+
self.fail_response(status, resp_body);
231245
}
232246

233247
match serde_json::from_str(&resp_body) {
@@ -269,3 +283,124 @@ impl ApiClient {
269283
}
270284

271285
}
286+
287+
/// Decide what error text to print for a failed response. Pulled out as a pure
288+
/// function so the 4xx-to-re-auth-hint logic can be unit-tested without
289+
/// making real HTTP calls or touching `std::process::exit`.
290+
fn format_fail_message(
291+
status: reqwest::StatusCode,
292+
body: &str,
293+
auth_status: Option<&auth::AuthStatus>,
294+
) -> String {
295+
if status.is_client_error() {
296+
if let Some(auth::AuthStatus::Invalid(_)) = auth_status {
297+
return "error: API key is invalid. Run 'hotdata auth' to re-authenticate.".to_string();
298+
}
299+
}
300+
util::api_error(body.to_string())
301+
}
302+
303+
#[cfg(test)]
304+
mod tests {
305+
use super::*;
306+
use auth::AuthStatus;
307+
308+
#[test]
309+
fn format_fail_message_401_with_invalid_key_shows_reauth_hint() {
310+
let msg = format_fail_message(
311+
reqwest::StatusCode::UNAUTHORIZED,
312+
"",
313+
Some(&AuthStatus::Invalid(401)),
314+
);
315+
assert!(msg.contains("API key is invalid"));
316+
assert!(msg.contains("hotdata auth"));
317+
}
318+
319+
#[test]
320+
fn format_fail_message_404_with_invalid_key_shows_reauth_hint() {
321+
// This is the user-reported scenario: the server masks an auth failure
322+
// behind a 404 with an empty body. The re-auth probe catches it.
323+
let msg = format_fail_message(
324+
reqwest::StatusCode::NOT_FOUND,
325+
"",
326+
Some(&AuthStatus::Invalid(401)),
327+
);
328+
assert!(msg.contains("API key is invalid"), "got: {msg}");
329+
}
330+
331+
#[test]
332+
fn format_fail_message_404_with_valid_key_shows_real_error() {
333+
// If the auth probe says the key is fine, surface the upstream body.
334+
let body = r#"{"error":{"message":"Query run 'qrun_notreal' not found"}}"#;
335+
let msg = format_fail_message(
336+
reqwest::StatusCode::NOT_FOUND,
337+
body,
338+
Some(&AuthStatus::Authenticated),
339+
);
340+
assert!(!msg.contains("API key is invalid"));
341+
assert!(msg.contains("Query run 'qrun_notreal' not found"));
342+
}
343+
344+
#[test]
345+
fn format_fail_message_400_with_valid_key_shows_real_error() {
346+
let body = r#"{"error":{"message":"invalid_sql"}}"#;
347+
let msg = format_fail_message(
348+
reqwest::StatusCode::BAD_REQUEST,
349+
body,
350+
Some(&AuthStatus::Authenticated),
351+
);
352+
assert_eq!(msg, "invalid_sql");
353+
}
354+
355+
#[test]
356+
fn format_fail_message_5xx_never_shows_reauth_hint() {
357+
// 5xx is not a client error — the auth probe is not even run, so
358+
// `auth_status` is None from the caller and we just surface the body.
359+
let msg = format_fail_message(
360+
reqwest::StatusCode::INTERNAL_SERVER_ERROR,
361+
"server exploded",
362+
None,
363+
);
364+
assert!(!msg.contains("API key is invalid"));
365+
assert_eq!(msg, "server exploded");
366+
}
367+
368+
#[test]
369+
fn format_fail_message_4xx_connection_error_on_probe_falls_through() {
370+
// If the probe itself couldn't reach the API, we can't claim the key
371+
// is invalid — surface the original body instead.
372+
let body = r#"{"error":{"message":"forbidden"}}"#;
373+
let msg = format_fail_message(
374+
reqwest::StatusCode::FORBIDDEN,
375+
body,
376+
Some(&AuthStatus::ConnectionError("tcp reset".to_string())),
377+
);
378+
assert!(!msg.contains("API key is invalid"));
379+
assert_eq!(msg, "forbidden");
380+
}
381+
382+
#[test]
383+
fn format_fail_message_4xx_no_probe_result_falls_through() {
384+
// Caller couldn't load config (None) — still surface the upstream error.
385+
let body = "plain body";
386+
let msg = format_fail_message(
387+
reqwest::StatusCode::NOT_FOUND,
388+
body,
389+
None,
390+
);
391+
assert!(!msg.contains("API key is invalid"));
392+
assert_eq!(msg, "plain body");
393+
}
394+
395+
#[test]
396+
fn format_fail_message_4xx_authenticated_probe_shows_server_message() {
397+
// Valid key but a genuine client error — upstream message wins.
398+
let body = r#"{"error":{"message":"workspace_not_found"}}"#;
399+
let msg = format_fail_message(
400+
reqwest::StatusCode::NOT_FOUND,
401+
body,
402+
Some(&AuthStatus::Authenticated),
403+
);
404+
assert_eq!(msg, "workspace_not_found");
405+
}
406+
}

‎src/sandbox.rs‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,19 +14,17 @@ struct Sandbox {
1414

1515
#[derive(Deserialize)]
1616
struct ListResponse {
17-
#[serde(rename = "sessions")]
1817
sandboxes: Vec<Sandbox>,
1918
}
2019

2120
#[derive(Deserialize)]
2221
struct DetailResponse {
23-
#[serde(rename = "session")]
2422
sandbox: Sandbox,
2523
}
2624

2725
pub fn list(workspace_id: &str, format: &str) {
2826
let api = ApiClient::new(Some(workspace_id));
29-
let body: ListResponse = api.get("/sessions");
27+
let body: ListResponse = api.get("/sandboxes");
3028

3129
let current_sandbox = std::env::var("HOTDATA_SANDBOX")
3230
.ok()
@@ -57,7 +55,7 @@ pub fn list(workspace_id: &str, format: &str) {
5755

5856
pub fn get(sandbox_id: &str, workspace_id: &str, format: &str) {
5957
let api = ApiClient::new(Some(workspace_id));
60-
let path = format!("/sessions/{sandbox_id}");
58+
let path = format!("/sandboxes/{sandbox_id}");
6159
let body: DetailResponse = api.get(&path);
6260
let s = &body.sandbox;
6361

@@ -82,7 +80,7 @@ pub fn get(sandbox_id: &str, workspace_id: &str, format: &str) {
8280

8381
pub fn read(sandbox_id: &str, workspace_id: &str) {
8482
let api = ApiClient::new(Some(workspace_id));
85-
let path = format!("/sessions/{sandbox_id}");
83+
let path = format!("/sandboxes/{sandbox_id}");
8684
let body: DetailResponse = api.get(&path);
8785
if body.sandbox.markdown.is_empty() {
8886
eprintln!("{}", "Sandbox markdown is empty.".dark_grey());
@@ -139,7 +137,7 @@ pub fn new(workspace_id: &str, name: Option<&str>, format: &str) {
139137
body["name"] = serde_json::json!(n);
140138
}
141139

142-
let resp: DetailResponse = api.post("/sessions", &body);
140+
let resp: DetailResponse = api.post("/sandboxes", &body);
143141
let s = &resp.sandbox;
144142

145143
// Set as the active sandbox in config
@@ -173,7 +171,7 @@ pub fn update(workspace_id: &str, sandbox_id: &str, name: Option<&str>, markdown
173171
if let Some(n) = name { body["name"] = serde_json::json!(n); }
174172
if let Some(m) = markdown { body["markdown"] = serde_json::json!(m); }
175173

176-
let path = format!("/sessions/{sandbox_id}");
174+
let path = format!("/sandboxes/{sandbox_id}");
177175
let resp: DetailResponse = api.patch(&path, &body);
178176
let s = &resp.sandbox;
179177

@@ -197,7 +195,7 @@ pub fn run(sandbox_id: Option<&str>, workspace_id: &str, name: Option<&str>, cmd
197195
Some(id) => {
198196
// Verify the sandbox exists
199197
let api = ApiClient::new(Some(workspace_id));
200-
let path = format!("/sessions/{id}");
198+
let path = format!("/sandboxes/{id}");
201199
let _: DetailResponse = api.get(&path);
202200
id.to_string()
203201
}
@@ -208,7 +206,7 @@ pub fn run(sandbox_id: Option<&str>, workspace_id: &str, name: Option<&str>, cmd
208206
if let Some(n) = name {
209207
body["name"] = serde_json::json!(n);
210208
}
211-
let resp: DetailResponse = api.post("/sessions", &body);
209+
let resp: DetailResponse = api.post("/sandboxes", &body);
212210
resp.sandbox.public_id
213211
}
214212
};
@@ -254,7 +252,7 @@ pub fn set(sandbox_id: Option<&str>, workspace_id: &str) {
254252
Some(id) => {
255253
// Verify the sandbox exists by fetching it
256254
let api = ApiClient::new(Some(workspace_id));
257-
let path = format!("/sessions/{id}");
255+
let path = format!("/sandboxes/{id}");
258256
let _: DetailResponse = api.get(&path);
259257

260258
if let Err(e) = config::save_sandbox("default", id) {

0 commit comments

Comments
 (0)