diff --git a/CHANGELOG.md b/CHANGELOG.md index 2fa94118f..97e4af23f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1126,6 +1126,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- `ErrorResponse` adds an optional typed `code: ErrorCode` (`"create_timeout"`, + `"exec_timeout"`, `"vm_not_found"`), `vm_id`, and `timeout_secs` on the HTTP + error wire contract, with service-side exec timeouts returning HTTP 504 + (`StatusCode::GATEWAY_TIMEOUT`). The Python SDK adds host gateway discovery + (`discover_gateway`, `capsem_run_dir`, `GatewayEndpoint`, + `Hypervisor.connect`, and optional `url`/`token`/`run_dir` on `Hypervisor` + and `VM` resolving `CAPSEM_GATEWAY_URL`, `CAPSEM_GATEWAY_TOKEN`, and + `/gateway.{port,token}`) and structured exceptions + (`CapsemError`, `CapsemTimeoutError`, `CreateTimeoutError`, + `ExecTimeoutError`, and `VmNotFoundError`) dispatched from + `HttpError.code` (`models.ErrorCode`). The Python and TypeScript SDKs also + export `MAX_REQUEST_BODY_BYTES` and `CREATE_READY_SECS` guarded by Citadel + against the service and API constants. - `POST /vms/create` (`ProvisionRequest`) and `POST /vms/{id}/fork` (`ForkRequest`) accept optional advisory `labels` (`<= 64` entries, ASCII keys `1..=64` matching the VM-name rule `[A-Za-z0-9][A-Za-z0-9_-]{0,63}`, values `<= 255` diff --git a/crates/capsem-api/src/files.rs b/crates/capsem-api/src/files.rs index e9700d1d6..d876211d7 100644 --- a/crates/capsem-api/src/files.rs +++ b/crates/capsem-api/src/files.rs @@ -63,7 +63,21 @@ pub struct LogsResponse { pub process_logs: Option, } -#[derive(Serialize, Deserialize, Debug, Clone, ToSchema)] +#[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq, Eq, ToSchema)] +#[serde(rename_all = "snake_case")] +pub enum ErrorCode { + VmNotFound, + CreateTimeout, + ExecTimeout, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, Default, ToSchema)] pub struct ErrorResponse { pub error: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub code: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub vm_id: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub timeout_secs: Option, } diff --git a/crates/capsem-service/src/active_policy.rs b/crates/capsem-service/src/active_policy.rs index 63a6a170f..5b9ffcc53 100644 --- a/crates/capsem-service/src/active_policy.rs +++ b/crates/capsem-service/src/active_policy.rs @@ -120,7 +120,7 @@ pub(crate) async fn push_policy_to_running_instances( if failures.is_empty() { Ok(total) } else { - Err(AppError( + Err(AppError::new( StatusCode::INTERNAL_SERVER_ERROR, format!( "policy applied to {} of {total} running VMs; not applied: {}", diff --git a/crates/capsem-service/src/api/tests.rs b/crates/capsem-service/src/api/tests.rs index d0b996a95..489f7e28e 100644 --- a/crates/capsem-service/src/api/tests.rs +++ b/crates/capsem-service/src/api/tests.rs @@ -269,10 +269,16 @@ fn logs_response_roundtrip() { fn error_response_roundtrip() { let r = ErrorResponse { error: "sandbox not found".into(), + code: Some(ErrorCode::VmNotFound), + vm_id: Some("vm-1".into()), + timeout_secs: None, }; let json = serde_json::to_string(&r).unwrap(); + assert!(!json.contains("timeout_secs")); let r2: ErrorResponse = serde_json::from_str(&json).unwrap(); assert!(r2.error.contains("not found")); + assert_eq!(r2.code, Some(ErrorCode::VmNotFound)); + assert_eq!(r2.vm_id.as_deref(), Some("vm-1")); } #[test] diff --git a/crates/capsem-service/src/asset_routes.rs b/crates/capsem-service/src/asset_routes.rs index c52c816f6..c6c33bf76 100644 --- a/crates/capsem-service/src/asset_routes.rs +++ b/crates/capsem-service/src/asset_routes.rs @@ -223,7 +223,7 @@ pub(super) fn asset_manifest_status(state: &ServiceState) -> Result, id: &str, generation: u64, spec: Contain { return Ok(()); } - let uds_path = running_uds_path(state, id).map_err(|e| e.1)?; + let uds_path = running_uds_path(state, id).map_err(AppError::into_message)?; let reply = send_ipc_command( &uds_path, ServiceToProcess::Exec { @@ -532,7 +532,10 @@ pub(crate) async fn grant_surface(state: &Arc, id: &str, generatio } let exposure = match crate::router_runtime::exposures::create_exposure(state, id, request).await { Ok(exposure) => exposure, - Err(AppError(status, error)) => { + Err(AppError { + status, + body: ErrorResponse { error, .. }, + }) => { warn!(vm_id = id, %status, %error, "container surface exposure refused"); return; } @@ -550,7 +553,11 @@ pub(crate) async fn grant_surface(state: &Arc, id: &str, generatio Path((id.to_owned(), exposure.id.clone())), ) .await; - if let Err(AppError(status, error)) = revoked { + if let Err(AppError { + status, + body: ErrorResponse { error, .. }, + }) = revoked + { if status != StatusCode::NOT_FOUND { warn!(vm_id = id, exposure_id = exposure.id.as_str(), %status, %error, "late container exposure could not be revoked"); } @@ -650,18 +657,12 @@ pub(crate) async fn wait_for_create( /// Create, resume and carried-image clones share the same workload barrier. /// A timeout leaves setup with its service owner; terminal failures are errors. pub(crate) async fn require_ready(state: &Arc, id: &str) -> Result<(), AppError> { - let status = wait_for_create(state, id).await.map_err(|timed_out| { - warn!(vm_id = id, attempts = timed_out.attempts, "container workload readiness timed out"); - AppError( - StatusCode::GATEWAY_TIMEOUT, - format!( - "container workload for VM {id} did not become ready before the HTTP deadline; setup continues under service ownership" - ), - ) - })?; + let status = wait_for_create(state, id) + .await + .map_err(|t| AppError::create_timeout(id, t.attempts))?; match status.state { ContainerState::Running | ContainerState::Staged => Ok(()), - ContainerState::Exited | ContainerState::Failed => Err(AppError( + ContainerState::Exited | ContainerState::Failed => Err(AppError::new( StatusCode::INTERNAL_SERVER_ERROR, format!( "container workload for VM {id} reached {:?}: {}", @@ -700,7 +701,7 @@ async fn wait_observed( } pub(crate) fn record_launched(state: &ServiceState, id: &str) -> Result<(), String> { - let session_dir = resolve_session_dir(state, id).map_err(|e| e.1)?; + let session_dir = resolve_session_dir(state, id).map_err(AppError::into_message)?; let Some(record) = state.containers.launch_record(id) else { return Ok(()); }; @@ -744,7 +745,7 @@ pub(crate) async fn handle_container_status( .off_worker(move |state| observe(&state, &id, live)) .await?? .map(Json) - .ok_or_else(|| AppError(StatusCode::NOT_FOUND, "VM has no container workload".into())) + .ok_or_else(|| AppError::new(StatusCode::NOT_FOUND, "VM has no container workload".into())) } fn observe( @@ -820,7 +821,7 @@ async fn share_image( .containers .work_lease(id, generation) .ok_or("container share cancelled")?; - let session_dir = resolve_session_dir(state, id).map_err(|e| e.1)?; + let session_dir = resolve_session_dir(state, id).map_err(AppError::into_message)?; let (root, files) = (image.root.clone(), image.files.clone()); tokio::task::spawn_blocking(move || -> anyhow::Result<_> { let _lease = lease; @@ -878,14 +879,14 @@ async fn stage_file( Ok::<_, AppError>((parent, name, lease)) }) .await - .map_err(|e| e.1)? - .map_err(|e| e.1)?; + .map_err(AppError::into_message)? + .map_err(AppError::into_message)?; let preview = file_security_preview_bytes(&file.bytes); let size = file.bytes.len() as u64; - let uds_path = running_uds_path(state, id).map_err(|e| e.1)?; + let uds_path = running_uds_path(state, id).map_err(AppError::into_message)?; if log_file_boundary_on_owner(state, &uds_path, FileBoundaryAction::Import, path, preview, size, None) .await - .map_err(|e| e.1)? + .map_err(AppError::into_message)? .is_some() { return Err(format!("file import policy rewrote staged image file {}", file.name)); diff --git a/crates/capsem-service/src/container_setup/images.rs b/crates/capsem-service/src/container_setup/images.rs index 7be1db300..fe55adf72 100644 --- a/crates/capsem-service/src/container_setup/images.rs +++ b/crates/capsem-service/src/container_setup/images.rs @@ -145,7 +145,7 @@ impl From for AppError { ImageError::Refused(_) => StatusCode::FORBIDDEN, ImageError::Failed(_) => StatusCode::BAD_GATEWAY, }; - AppError(status, error.to_string()) + AppError::new(status, error.to_string()) } } @@ -360,8 +360,8 @@ pub(crate) async fn handle_list_images( images: Vec::new(), })); }; - let architecture = - stage::catalog_architecture().map_err(|e| AppError(StatusCode::INTERNAL_SERVER_ERROR, format!("{e:#}")))?; + let architecture = stage::catalog_architecture() + .map_err(|e| AppError::new(StatusCode::INTERNAL_SERVER_ERROR, format!("{e:#}")))?; let mut images: Vec = loaded .catalog .entries() diff --git a/crates/capsem-service/src/container_setup/relaunch/tests.rs b/crates/capsem-service/src/container_setup/relaunch/tests.rs index 89fb9ff24..4ee9b076c 100644 --- a/crates/capsem-service/src/container_setup/relaunch/tests.rs +++ b/crates/capsem-service/src/container_setup/relaunch/tests.rs @@ -185,8 +185,8 @@ async fn restored_workload_failure_is_not_lifecycle_success() { ); insert_fake_instance_with_session_dir(&state, "box", 1, session); let error = restore_ready(&state, "box").await.unwrap_err(); - assert_eq!(error.0, StatusCode::INTERNAL_SERVER_ERROR); - assert!(error.1.contains("Failed") && error.1.contains("did not start")); + assert_eq!(error.status, StatusCode::INTERNAL_SERVER_ERROR); + assert!(error.body.error.contains("Failed") && error.body.error.contains("did not start")); } #[tokio::test] diff --git a/crates/capsem-service/src/container_setup/tests.rs b/crates/capsem-service/src/container_setup/tests.rs index 781a40e64..a3c5449db 100644 --- a/crates/capsem-service/src/container_setup/tests.rs +++ b/crates/capsem-service/src/container_setup/tests.rs @@ -941,3 +941,53 @@ fn admission_takes_either_content_address_and_nothing_else() { let refused = admit(&policy, &requested, &pulled(&b, &c)).unwrap_err(); assert!(refused.contains("not admitted"), "{refused}"); } + +#[tokio::test] +async fn container_helpers_propagate_missing_vm_and_boundary_errors() { + let fx = fixture(images()); + assert!(record_launched(&fx.state, "missing").is_err()); + let generation = fx.state.containers.begin("missing", "registry.example/app:1"); + let pulled = PulledImage { + root: PathBuf::new(), + files: Vec::new(), + digest: "sha256:a".into(), + image_digest: "sha256:a".into(), + cache_key: None, + _hold: Box::new(()), + }; + assert!(share_image(&fx.state, "missing", generation, &pulled, None) + .await + .is_err()); + let mk_file = || stage::StagedFile { + name: "spec.json".to_string(), + bytes: vec![b'{', b'}'], + }; + assert!(stage_file(&fx.state, "missing", generation, mk_file()).await.is_err()); + + let stopped_dir = fx._dir.path().join("stopped-session"); + std::fs::create_dir_all(stopped_dir.join("guest/workspace")).unwrap(); + let entry = crate::tests::test_persistent_entry("stopped-box", stopped_dir); + let stopped_id = entry.id.clone(); + fx.state + .persistent_registry + .lock() + .unwrap() + .data + .vms + .insert("stopped-box".into(), entry); + let gen2 = fx.state.containers.begin(&stopped_id, "registry.example/app:1"); + assert!(stage_file(&fx.state, &stopped_id, gen2, mk_file()).await.is_err()); + + let bad_uds = std::path::Path::new("/nonexistent/uds.sock"); + assert!(log_file_boundary_on_owner( + &fx.state, + bad_uds, + FileBoundaryAction::Import, + "a".into(), + vec![], + 0, + None + ) + .await + .is_err()); +} diff --git a/crates/capsem-service/src/credential_routes.rs b/crates/capsem-service/src/credential_routes.rs index 36b872e43..af95a5297 100644 --- a/crates/capsem-service/src/credential_routes.rs +++ b/crates/capsem-service/src/credential_routes.rs @@ -9,7 +9,7 @@ pub(crate) async fn inject( payload: Result, axum::extract::rejection::JsonRejection>, ) -> Result, AppError> { let Json(request) = - payload.map_err(|_| AppError(StatusCode::BAD_REQUEST, "invalid credential injection request".into()))?; + payload.map_err(|_| AppError::new(StatusCode::BAD_REQUEST, "invalid credential injection request".into()))?; let storage = request.storage; let reference = state .off_worker(move |_| { @@ -25,7 +25,7 @@ pub(crate) async fn inject( CredentialStore::global().inject(provider, &request.value, persistence) }) .await? - .map_err(|error| AppError(StatusCode::BAD_REQUEST, error))?; + .map_err(|error| AppError::new(StatusCode::BAD_REQUEST, error))?; if storage == api::CredentialStorage::Memory { let paths: Vec<_> = state .instances @@ -36,7 +36,7 @@ pub(crate) async fn inject( .collect(); for path in paths { sync_memory(&state, &path).await.map_err(|_| { - AppError( + AppError::new( StatusCode::SERVICE_UNAVAILABLE, "credential stored in service memory; a VM owner has not acknowledged injection; retry explicitly" .into(), diff --git a/crates/capsem-service/src/errors.rs b/crates/capsem-service/src/errors.rs index 60fbe0e33..6068a777f 100644 --- a/crates/capsem-service/src/errors.rs +++ b/crates/capsem-service/src/errors.rs @@ -7,11 +7,11 @@ use axum::http::StatusCode; use axum::response::IntoResponse; use axum::Json; -pub use crate::api::ErrorResponse; +pub use crate::api::{ErrorCode, ErrorResponse}; -/// Tuple of (HTTP status, error message). Implements `IntoResponse` so handlers -/// can `?` against `Result` and get a JSON `{"error": "..."}` -/// body with the right status code. +/// Structured `(HTTP status, ErrorResponse)` returned by service route handlers. +/// Implements `IntoResponse` so handlers can `?` against `Result` +/// and get a JSON `{"error": "..."}` body with the right status code. /// /// Every `AppError` is automatically logged as it goes out the door (see the /// `IntoResponse` impl below). 5xx → `tracing::error!`, 4xx → `tracing::warn!`, @@ -19,12 +19,73 @@ pub use crate::api::ErrorResponse; /// for every error response without per-site work. Pre-W3.5: the operator /// got a 500 in the response and nothing in the log to trace back from. #[derive(Debug)] -pub struct AppError(pub StatusCode, pub String); +pub struct AppError { + pub status: StatusCode, + pub body: ErrorResponse, +} + +impl AppError { + /// Construct a plain `(status, error)` response without structured metadata. + pub fn new(status: StatusCode, error: String) -> Self { + Self { + status, + body: ErrorResponse { + error, + code: None, + vm_id: None, + timeout_secs: None, + }, + } + } + + pub fn into_message(self) -> String { + self.body.error + } + + pub fn with_code(mut self, code: ErrorCode) -> Self { + self.body.code = Some(code); + self + } + + pub fn with_vm_id(mut self, vm_id: impl Into) -> Self { + self.body.vm_id = Some(vm_id.into()); + self + } + + pub fn with_timeout_secs(mut self, timeout_secs: u64) -> Self { + self.body.timeout_secs = Some(timeout_secs); + self + } + + pub fn vm_not_found(id: &str) -> Self { + Self::new(StatusCode::NOT_FOUND, format!("sandbox not found: {id}")) + .with_code(ErrorCode::VmNotFound) + .with_vm_id(id) + } + + pub fn exec_timeout(error: impl Into, timeout_secs: u64) -> Self { + Self::new(StatusCode::GATEWAY_TIMEOUT, error.into()) + .with_code(ErrorCode::ExecTimeout) + .with_timeout_secs(timeout_secs) + } + + pub fn create_timeout(id: &str, attempts: u32) -> Self { + ::tracing::warn!(vm_id = id, attempts, "container create readiness timed out"); + Self::new( + StatusCode::GATEWAY_TIMEOUT, + format!( + "container workload for VM {id} did not become ready before the HTTP deadline; setup continues under service ownership" + ), + ) + .with_code(ErrorCode::CreateTimeout) + .with_vm_id(id) + } +} impl IntoResponse for AppError { fn into_response(self) -> axum::response::Response { - let status = self.0; - let msg = self.1.as_str(); + let status = self.status; + let msg = self.body.error.as_str(); if status.is_server_error() { ::tracing::error!( target: "service", @@ -48,7 +109,7 @@ impl IntoResponse for AppError { ); } - (self.0, Json(ErrorResponse { error: self.1 })).into_response() + (self.status, Json(self.body)).into_response() } } @@ -71,7 +132,7 @@ macro_rules! app_error_logged { status = $status.as_u16(), "{}", __msg ); - $crate::errors::AppError($status, __msg) + $crate::errors::AppError::new($status, __msg) }}; } diff --git a/crates/capsem-service/src/errors/tests.rs b/crates/capsem-service/src/errors/tests.rs index 1436508e0..aeb6e434a 100644 --- a/crates/capsem-service/src/errors/tests.rs +++ b/crates/capsem-service/src/errors/tests.rs @@ -10,7 +10,7 @@ use axum::http::StatusCode; #[tokio::test] async fn app_error_formats_json() { - let err = AppError(StatusCode::BAD_REQUEST, "invalid sandbox name".into()); + let err = AppError::new(StatusCode::BAD_REQUEST, "invalid sandbox name".into()); let response = err.into_response(); assert_eq!(response.status(), StatusCode::BAD_REQUEST); let body = to_bytes(response.into_body(), 1024).await.unwrap(); @@ -20,7 +20,7 @@ async fn app_error_formats_json() { #[tokio::test] async fn app_error_internal_server() { - let err = AppError(StatusCode::INTERNAL_SERVER_ERROR, "db connection failed".into()); + let err = AppError::new(StatusCode::INTERNAL_SERVER_ERROR, "db connection failed".into()); let response = err.into_response(); assert_eq!(response.status(), StatusCode::INTERNAL_SERVER_ERROR); let body = to_bytes(response.into_body(), 1024).await.unwrap(); @@ -30,7 +30,7 @@ async fn app_error_internal_server() { #[tokio::test] async fn app_error_conflict() { - let err = AppError(StatusCode::CONFLICT, "sandbox already exists".into()); + let err = AppError::new(StatusCode::CONFLICT, "sandbox already exists".into()); let response = err.into_response(); assert_eq!(response.status(), StatusCode::CONFLICT); let body = to_bytes(response.into_body(), 1024).await.unwrap(); @@ -40,7 +40,7 @@ async fn app_error_conflict() { #[tokio::test] async fn app_error_preserves_arbitrary_status() { - let err = AppError(StatusCode::IM_A_TEAPOT, "no coffee here".into()); + let err = AppError::new(StatusCode::IM_A_TEAPOT, "no coffee here".into()); let response = err.into_response(); assert_eq!(response.status(), StatusCode::IM_A_TEAPOT); let body = to_bytes(response.into_body(), 1024).await.unwrap(); @@ -50,7 +50,7 @@ async fn app_error_preserves_arbitrary_status() { #[tokio::test] async fn app_error_preserves_empty_message() { - let err = AppError(StatusCode::FORBIDDEN, String::new()); + let err = AppError::new(StatusCode::FORBIDDEN, String::new()); let response = err.into_response(); assert_eq!(response.status(), StatusCode::FORBIDDEN); let body = to_bytes(response.into_body(), 1024).await.unwrap(); @@ -66,6 +66,42 @@ fn app_error_logged_bare_form_builds_correct_appe() { StatusCode::INTERNAL_SERVER_ERROR, "exec failed for {id}: io error" ); - assert_eq!(err.0, StatusCode::INTERNAL_SERVER_ERROR); - assert!(err.1.contains("vm-test")); + assert_eq!(err.status, StatusCode::INTERNAL_SERVER_ERROR); + assert!(err.body.error.contains("vm-test")); +} + +#[tokio::test] +async fn app_error_structured_builders_serialize_fields() { + let err = AppError::vm_not_found("vm-42"); + let response = err.into_response(); + assert_eq!(response.status(), StatusCode::NOT_FOUND); + let body = to_bytes(response.into_body(), 1024).await.unwrap(); + let json: serde_json::Value = serde_json::from_slice(&body).unwrap(); + assert_eq!(json["error"], "sandbox not found: vm-42"); + assert_eq!(json["code"], "vm_not_found"); + assert_eq!(json["vm_id"], "vm-42"); + assert!(json.get("timeout_secs").is_none()); +} + +#[tokio::test] +async fn app_error_exec_timeout_serializes_fields_and_504() { + let err = AppError::exec_timeout("command timed out after 30s", 30); + let response = err.into_response(); + assert_eq!(response.status(), StatusCode::GATEWAY_TIMEOUT); + let body = to_bytes(response.into_body(), 1024).await.unwrap(); + let json: serde_json::Value = serde_json::from_slice(&body).unwrap(); + assert_eq!(json["error"], "command timed out after 30s"); + assert_eq!(json["code"], "exec_timeout"); + assert_eq!(json["timeout_secs"], 30); + assert!(json.get("vm_id").is_none()); +} + +#[tokio::test] +async fn app_error_create_timeout_and_into_message() { + let err = AppError::create_timeout("vm-99", 4); + assert_eq!(err.status, StatusCode::GATEWAY_TIMEOUT); + assert_eq!(err.body.code, Some(ErrorCode::CreateTimeout)); + assert_eq!(err.body.vm_id.as_deref(), Some("vm-99")); + let msg = err.into_message(); + assert!(msg.contains("vm-99")); } diff --git a/crates/capsem-service/src/fs_utils.rs b/crates/capsem-service/src/fs_utils.rs index 9f7c013c4..557bc94b4 100644 --- a/crates/capsem-service/src/fs_utils.rs +++ b/crates/capsem-service/src/fs_utils.rs @@ -34,13 +34,13 @@ pub fn sanitize_file_path(raw: &str) -> Result { } let trimmed = collapsed.trim_start_matches('/'); if trimmed.is_empty() { - return Err(AppError( + return Err(AppError::new( StatusCode::BAD_REQUEST, "empty path after sanitization".into(), )); } if trimmed.contains("..") { - return Err(AppError(StatusCode::BAD_REQUEST, "path traversal rejected".into())); + return Err(AppError::new(StatusCode::BAD_REQUEST, "path traversal rejected".into())); } Ok(trimmed.to_string()) } @@ -112,14 +112,14 @@ fn workspace_relative(raw: &str, exact: bool, container: bool) -> Result<&str, A }; match raw.strip_prefix(root) { Some(rest) if rest.is_empty() || rest.starts_with('/') => Ok(rest), - _ if container => Err(AppError( + _ if container => Err(AppError::new( StatusCode::BAD_REQUEST, format!( "{raw} is not reachable: this VM runs a container, which sees the workspace at {root}; \ pass a path under {root}, a relative path, or exact=true to place the path as written in the workspace" ), )), - _ => Err(AppError( + _ => Err(AppError::new( StatusCode::BAD_REQUEST, format!( "{raw} is outside the workspace: only paths under {root} are reachable; \ diff --git a/crates/capsem-service/src/fs_utils/tests.rs b/crates/capsem-service/src/fs_utils/tests.rs index 8f3b05700..50e34436a 100644 --- a/crates/capsem-service/src/fs_utils/tests.rs +++ b/crates/capsem-service/src/fs_utils/tests.rs @@ -26,13 +26,13 @@ fn sanitize_strips_unicode() { #[test] fn sanitize_rejects_dot_dot() { let err = sanitize_file_path("../etc/passwd").unwrap_err(); - assert_eq!(err.0, StatusCode::BAD_REQUEST); + assert_eq!(err.status, StatusCode::BAD_REQUEST); } #[test] fn sanitize_rejects_embedded_dot_dot() { let err = sanitize_file_path("foo/../bar").unwrap_err(); - assert_eq!(err.0, StatusCode::BAD_REQUEST); + assert_eq!(err.status, StatusCode::BAD_REQUEST); } #[test] @@ -50,7 +50,7 @@ fn sanitize_strips_leading_slash() { #[test] fn sanitize_rejects_empty() { let err = sanitize_file_path("").unwrap_err(); - assert_eq!(err.0, StatusCode::BAD_REQUEST); + assert_eq!(err.status, StatusCode::BAD_REQUEST); } #[test] @@ -72,8 +72,8 @@ fn sanitize_rejects_only_slashes() { // Several slashes collapse + leading-strip to empty, then the empty // check fires. Confirms the order: collapse → strip → reject empty. let err = sanitize_file_path("///").unwrap_err(); - assert_eq!(err.0, StatusCode::BAD_REQUEST); - assert_eq!(err.1, "empty path after sanitization"); + assert_eq!(err.status, StatusCode::BAD_REQUEST); + assert_eq!(err.body.error, "empty path after sanitization"); } #[test] @@ -82,8 +82,8 @@ fn sanitize_rejects_dot_dot_after_filter() { // collapses to `..` and is correctly rejected as traversal -- proves // the filter runs before the traversal check, not after it. let err = sanitize_file_path(".<>.").unwrap_err(); - assert_eq!(err.0, StatusCode::BAD_REQUEST); - assert_eq!(err.1, "path traversal rejected"); + assert_eq!(err.status, StatusCode::BAD_REQUEST); + assert_eq!(err.body.error, "path traversal rejected"); } // ---- file typing ---- @@ -166,9 +166,9 @@ mod paths { } fn refused(raw: &str, container: bool) -> String { - let AppError(status, message) = resolve_file_path(raw, false, container).unwrap_err(); - assert_eq!(status, StatusCode::BAD_REQUEST); - message + let err = resolve_file_path(raw, false, container).unwrap_err(); + assert_eq!(err.status, StatusCode::BAD_REQUEST); + err.body.error } #[test] diff --git a/crates/capsem-service/src/instance/tests.rs b/crates/capsem-service/src/instance/tests.rs index 95615f4ed..17b52b3cd 100644 --- a/crates/capsem-service/src/instance/tests.rs +++ b/crates/capsem-service/src/instance/tests.rs @@ -21,7 +21,7 @@ async fn stale_shutdown_binding_leaves_replacement_socket_and_ledger_intact() { .unwrap() .insert("replacement".into(), Arc::clone(&ledger)); let result = shutdown_vm_process(&state, "replacement", ShutdownMode::Discard, Some(uuid::Uuid::new_v4())).await; - assert_eq!(result.unwrap_err().0, StatusCode::CONFLICT); + assert_eq!(result.unwrap_err().status, StatusCode::CONFLICT); assert_eq!( state.instances.lock().unwrap().get("replacement").unwrap().generation, generation diff --git a/crates/capsem-service/src/ledger_routes.rs b/crates/capsem-service/src/ledger_routes.rs index 9f269c053..949cf8b7e 100644 --- a/crates/capsem-service/src/ledger_routes.rs +++ b/crates/capsem-service/src/ledger_routes.rs @@ -47,7 +47,7 @@ pub(super) async fn handle_security_latest( "security_latest" ); let body = serde_json::to_vec(&rows).map_err(|error| { - AppError( + AppError::new( StatusCode::INTERNAL_SERVER_ERROR, format!("failed to serialize security latest response: {error}"), ) @@ -72,7 +72,7 @@ pub(super) async fn handle_detection_latest( }; let rows = security_latest_for_vm(&state, &id, limit, true).await?; let body = serde_json::to_vec(&rows).map_err(|error| { - AppError( + AppError::new( StatusCode::INTERNAL_SERVER_ERROR, format!("failed to serialize detection latest response: {error}"), ) @@ -137,7 +137,7 @@ pub(super) fn ledger_route_error( error = %error, "session ledger route DB operation failed" ); - AppError( + AppError::new( StatusCode::INTERNAL_SERVER_ERROR, format!("failed to {operation} {ledger} ledger for {vm_id}: {error}"), ) diff --git a/crates/capsem-service/src/ledger_routes/bodies.rs b/crates/capsem-service/src/ledger_routes/bodies.rs index 731f067d9..81e036e48 100644 --- a/crates/capsem-service/src/ledger_routes/bodies.rs +++ b/crates/capsem-service/src/ledger_routes/bodies.rs @@ -129,7 +129,7 @@ pub(crate) fn validate_event_id(raw: &str) -> Result<&str, AppError> { if hex { return Ok(raw); } - Err(AppError( + Err(AppError::new( StatusCode::BAD_REQUEST, format!( "event id must be exactly {EVENT_ID_LEN} lowercase hex characters; got {} character(s)", diff --git a/crates/capsem-service/src/ledger_routes/rows.rs b/crates/capsem-service/src/ledger_routes/rows.rs index 81d8361f6..8cf19c938 100644 --- a/crates/capsem-service/src/ledger_routes/rows.rs +++ b/crates/capsem-service/src/ledger_routes/rows.rs @@ -51,7 +51,7 @@ pub(super) fn route_query_objects( error = %error, "session ledger route DB query returned invalid JSON" ); - AppError( + AppError::new( StatusCode::INTERNAL_SERVER_ERROR, format!("{ledger} ledger query {query_name} returned invalid json for {vm_id}: {error}"), ) @@ -89,7 +89,7 @@ pub(super) async fn query_route_objects( error = %error, "session ledger route DB query failed" ); - AppError( + AppError::new( StatusCode::INTERNAL_SERVER_ERROR, format!("{ledger} ledger query {query_name} failed for {vm_id}: {error}"), ) @@ -123,7 +123,7 @@ where error = %error, "session ledger route DB query mapping failed" ); - AppError( + AppError::new( StatusCode::INTERNAL_SERVER_ERROR, format!("{ledger} ledger query {query_name} mapping failed for {vm_id}: {error}"), ) diff --git a/crates/capsem-service/src/ledger_routes/timeline.rs b/crates/capsem-service/src/ledger_routes/timeline.rs index fa04d68d9..50ee67973 100644 --- a/crates/capsem-service/src/ledger_routes/timeline.rs +++ b/crates/capsem-service/src/ledger_routes/timeline.rs @@ -16,7 +16,7 @@ pub(crate) async fn handle_timeline( ) -> Result, AppError> { let requested = params.layers.unwrap_or_else(|| LAYERS.to_vec()); if requested.is_empty() { - return Err(AppError(StatusCode::BAD_REQUEST, "no layers selected".into())); + return Err(AppError::new(StatusCode::BAD_REQUEST, "no layers selected".into())); } let layers: Vec = LAYERS.into_iter().filter(|layer| requested.contains(layer)).collect(); let cutoff = params @@ -27,7 +27,7 @@ pub(crate) async fn handle_timeline( .and_then(|time| time.duration_since(std::time::UNIX_EPOCH).ok()) .map(|duration| ledger_cutoff(duration.as_secs())) .ok_or_else(|| { - AppError( + AppError::new( StatusCode::BAD_REQUEST, "since must be a duration or RFC3339 timestamp".into(), ) diff --git a/crates/capsem-service/src/ledger_routes/vm_info.rs b/crates/capsem-service/src/ledger_routes/vm_info.rs index 3c82f7714..048911ed1 100644 --- a/crates/capsem-service/src/ledger_routes/vm_info.rs +++ b/crates/capsem-service/src/ledger_routes/vm_info.rs @@ -52,7 +52,7 @@ async fn session_counters( } let counters = match session_db(state, &info.id, "session status", db_path).await { Ok(db) => db.ledger_counters().await, - Err(error) => Err(error.1), + Err(error) => Err(error.body.error), }; match counters { Ok(counters) => { diff --git a/crates/capsem-service/src/managed_sessions.rs b/crates/capsem-service/src/managed_sessions.rs index 7ebb8f07e..d3dc77d29 100644 --- a/crates/capsem-service/src/managed_sessions.rs +++ b/crates/capsem-service/src/managed_sessions.rs @@ -384,7 +384,7 @@ impl Effects for ManagedLifecycle { } fn app_error(error: AppError) -> anyhow::Error { - anyhow::anyhow!("service lifecycle operation failed ({})", error.0) + anyhow::anyhow!("service lifecycle operation failed ({})", error.status) } #[cfg(test)] diff --git a/crates/capsem-service/src/managed_sessions/tests.rs b/crates/capsem-service/src/managed_sessions/tests.rs index 3b471594f..c1cb3771c 100644 --- a/crates/capsem-service/src/managed_sessions/tests.rs +++ b/crates/capsem-service/src/managed_sessions/tests.rs @@ -236,6 +236,9 @@ fn managed_adapter_refuses_named_or_persistent_requests_before_any_effect() { assert!(ManagedLifecycle::new(Arc::clone(&state), request, Arc::new(RefuseGrants)).is_err()); } assert!(state.instances.lock().unwrap().is_empty()); + assert!(super::app_error(AppError::new(StatusCode::CONFLICT, "busy".into())) + .to_string() + .contains("409")); } #[tokio::test] diff --git a/crates/capsem-service/src/mcp_routes.rs b/crates/capsem-service/src/mcp_routes.rs index d7f3ff236..6c4e29275 100644 --- a/crates/capsem-service/src/mcp_routes.rs +++ b/crates/capsem-service/src/mcp_routes.rs @@ -14,7 +14,7 @@ pub(super) struct McpPermissionEditRequest { /// The settings and corp files the MCP routes answer from. A settings.toml /// that does not load is an error here, never an empty configuration. fn policy_files() -> Result<(SettingsFile, SettingsFile), AppError> { - capsem_core::net::policy_config::load_policy_files().map_err(|error| AppError(StatusCode::BAD_REQUEST, error)) + capsem_core::net::policy_config::load_policy_files().map_err(|error| AppError::new(StatusCode::BAD_REQUEST, error)) } fn merged_mcp(settings: &SettingsFile, corp: &SettingsFile) -> McpConfig { @@ -23,7 +23,7 @@ fn merged_mcp(settings: &SettingsFile, corp: &SettingsFile) -> McpConfig { fn ensure_server_configured(mcp: &McpConfig, server_id: &str) -> Result<(), AppError> { if server_id.is_empty() { - return Err(AppError( + return Err(AppError::new( StatusCode::BAD_REQUEST, "MCP server id must not be empty".to_string(), )); @@ -31,7 +31,7 @@ fn ensure_server_configured(mcp: &McpConfig, server_id: &str) -> Result<(), AppE if mcp_server_configured(Some(mcp), server_id) { Ok(()) } else { - Err(AppError( + Err(AppError::new( StatusCode::NOT_FOUND, format!("MCP server not configured: {server_id}"), )) @@ -40,14 +40,14 @@ fn ensure_server_configured(mcp: &McpConfig, server_id: &str) -> Result<(), AppE pub(super) fn resolve_mcp_tool_id(server_id: &str, tool_id: &str) -> Result { if server_id.is_empty() || tool_id.is_empty() { - return Err(AppError( + return Err(AppError::new( StatusCode::BAD_REQUEST, "server id and tool id must not be empty".to_string(), )); } if let Some((prefix, _)) = tool_id.split_once("__") { if prefix != server_id { - return Err(AppError( + return Err(AppError::new( StatusCode::BAD_REQUEST, format!("tool id {tool_id} does not belong to MCP server {server_id}"), )); @@ -107,7 +107,7 @@ pub(super) async fn handle_mcp_default_info( ) -> Result, AppError> { let (settings, corp) = state.off_worker(|_| policy_files()).await??; let permission = - mcp_default_permission(&settings, &corp).map_err(|error| AppError(StatusCode::BAD_REQUEST, error))?; + mcp_default_permission(&settings, &corp).map_err(|error| AppError::new(StatusCode::BAD_REQUEST, error))?; Ok(Json(api::McpDefaultPermissionResponse { action: api::mcp_permission_action(permission.action), source: permission.source, @@ -140,7 +140,7 @@ pub(super) async fn handle_mcp_server_tools( .map(|entry| { let permission = mcp_tool_permission(&settings, &corp, &server_id, &entry.original_name).map_err(|error| { - AppError( + AppError::new( StatusCode::BAD_REQUEST, format!( "resolve MCP tool permission {}/{}: {error}", @@ -261,10 +261,10 @@ pub(super) async fn handle_mcp_tool_call( .values() .next() .map(|info| info.uds_path.clone()) - .ok_or_else(|| AppError(StatusCode::SERVICE_UNAVAILABLE, "no running sessions".into()))?; + .ok_or_else(|| AppError::new(StatusCode::SERVICE_UNAVAILABLE, "no running sessions".into()))?; let arguments_json = serde_json::to_string(&arguments) - .map_err(|e| AppError(StatusCode::BAD_REQUEST, format!("invalid arguments: {e}")))?; + .map_err(|e| AppError::new(StatusCode::BAD_REQUEST, format!("invalid arguments: {e}")))?; let msg = ServiceToProcess::McpCallTool { id: state.next_job_id(), namespaced_name, @@ -272,19 +272,21 @@ pub(super) async fn handle_mcp_tool_call( }; match send_ipc_command(&uds_path, msg, Some(60)) .await - .map_err(|e| AppError(StatusCode::BAD_GATEWAY, e))? + .map_err(|e| AppError::new(StatusCode::BAD_GATEWAY, e.to_string()))? { - ProcessToService::McpCallToolResult { error: Some(err), .. } => Err(AppError(StatusCode::BAD_GATEWAY, err)), + ProcessToService::McpCallToolResult { error: Some(err), .. } => { + Err(AppError::new(StatusCode::BAD_GATEWAY, err)) + } ProcessToService::McpCallToolResult { result_json, .. } => match result_json { Some(s) => serde_json::from_str(&s).map(Json).map_err(|e| { - AppError( + AppError::new( StatusCode::INTERNAL_SERVER_ERROR, format!("bad result_json from process: {e}"), ) }), None => Ok(Json(serde_json::Value::Null)), }, - _ => Err(AppError( + _ => Err(AppError::new( StatusCode::INTERNAL_SERVER_ERROR, "unexpected IPC response".into(), )), diff --git a/crates/capsem-service/src/network_routes.rs b/crates/capsem-service/src/network_routes.rs index 7c219b6f8..1c737b621 100644 --- a/crates/capsem-service/src/network_routes.rs +++ b/crates/capsem-service/src/network_routes.rs @@ -57,11 +57,11 @@ pub(super) fn network_error(error: NetworkError) -> AppError { NetworkError::NotFound(_) | NetworkError::NotAMember { .. } => StatusCode::NOT_FOUND, NetworkError::Database { .. } => StatusCode::INTERNAL_SERVER_ERROR, }; - AppError(status, error.to_string()) + AppError::new(status, error.to_string()) } fn parse_network_id(id: &str) -> Result { - Uuid::parse_str(id).map_err(|_| AppError(StatusCode::NOT_FOUND, format!("network not found: {id}"))) + Uuid::parse_str(id).map_err(|_| AppError::new(StatusCode::NOT_FOUND, format!("network not found: {id}"))) } fn network_info(registry: &NetworkRegistry, id: Uuid) -> Option { @@ -102,7 +102,7 @@ pub(super) fn resolve_network_names(registry: &NetworkRegistry, names: &[String] .map(|name| { registry .find(name) - .ok_or_else(|| AppError(StatusCode::BAD_REQUEST, format!("unknown network: {name}"))) + .ok_or_else(|| AppError::new(StatusCode::BAD_REQUEST, format!("unknown network: {name}"))) }) .collect() } @@ -193,7 +193,7 @@ pub(super) async fn handle_network_attach( let known = state.instances.lock().unwrap().contains_key(&vm_id) || vm_lifecycle::find_persistent_entry_by_route_id(&state, &vm_id).is_some(); if !known { - return Err(AppError(StatusCode::NOT_FOUND, format!("sandbox not found: {vm_id}"))); + return Err(AppError::vm_not_found(&vm_id)); } state .networks @@ -247,7 +247,7 @@ pub(super) async fn handle_network_logs( // The ledger wrote this JSON; a row that no longer parses is a broken // database, reported as such rather than shown as an empty event. let parsed = serde_json::from_str(&event.event_json).map_err(|error| { - AppError( + AppError::new( StatusCode::INTERNAL_SERVER_ERROR, format!("network {network} event {} is not JSON: {error}", event.event_id), ) diff --git a/crates/capsem-service/src/plugin_routes.rs b/crates/capsem-service/src/plugin_routes.rs index 83d08f742..bd96b3a80 100644 --- a/crates/capsem-service/src/plugin_routes.rs +++ b/crates/capsem-service/src/plugin_routes.rs @@ -195,7 +195,7 @@ type EffectivePluginPolicy = (BTreeMap, BTreeSet Result { let (settings, corp) = capsem_core::net::policy_config::load_policy_files() - .map_err(|error| AppError(StatusCode::BAD_REQUEST, error))?; + .map_err(|error| AppError::new(StatusCode::BAD_REQUEST, error))?; let mut policy: BTreeMap<_, _> = plugin_catalog() .iter() .map(|(id, entry)| (id.clone(), entry.default_config)) @@ -223,7 +223,10 @@ async fn plugin_info_from( include_runtime: bool, ) -> Result { let Some(catalog_entry) = plugin_catalog().get(plugin_id).copied() else { - return Err(AppError(StatusCode::NOT_FOUND, format!("unknown plugin: {plugin_id}"))); + return Err(AppError::new( + StatusCode::NOT_FOUND, + format!("unknown plugin: {plugin_id}"), + )); }; let config = policy.get(plugin_id).copied().unwrap_or(catalog_entry.default_config); let runtime = if include_runtime { @@ -308,7 +311,7 @@ async fn plugin_runtime_status( let snapshots = match activity::session_counters(state).await { Ok(snapshots) => snapshots, Err(error) => { - status.last_error = Some(format!("failed to read security ledger: {}", error.1)); + status.last_error = Some(format!("failed to read security ledger: {}", error.body.error)); return status; } }; @@ -497,7 +500,10 @@ pub(super) async fn handle_plugin_update( Json(update): Json, ) -> Result, AppError> { let Some(catalog_entry) = plugin_catalog().get(&plugin_id).copied() else { - return Err(AppError(StatusCode::NOT_FOUND, format!("unknown plugin: {plugin_id}"))); + return Err(AppError::new( + StatusCode::NOT_FOUND, + format!("unknown plugin: {plugin_id}"), + )); }; let route = MutationRoute { name: "plugin_edit", diff --git a/crates/capsem-service/src/policy_mutation.rs b/crates/capsem-service/src/policy_mutation.rs index 8e24fb857..7ac12879a 100644 --- a/crates/capsem-service/src/policy_mutation.rs +++ b/crates/capsem-service/src/policy_mutation.rs @@ -45,7 +45,7 @@ impl PolicyMutation<'_> { /// one out, so no route can read-modify-write outside the boundary. fn settings_edit(&self) -> Result { let path = capsem_core::net::policy_config::settings_config_path() - .ok_or_else(|| AppError(StatusCode::INTERNAL_SERVER_ERROR, "HOME not set".to_string()))?; + .ok_or_else(|| AppError::new(StatusCode::INTERNAL_SERVER_ERROR, "HOME not set".to_string()))?; SettingsPolicyEdit::open(&path).map_err(bad_request) } } @@ -83,6 +83,7 @@ pub(crate) async fn apply_policy_mutation( "policy mutation route requested" ); let rejected = |error: &AppError| { + let error = error.body.error.as_str(); warn!( target: "capsem.policy_mutation", route = name, @@ -90,7 +91,7 @@ pub(crate) async fn apply_policy_mutation( target_key, operation, actor = "service-api", - error = error.1.as_str(), + error, "policy mutation route rejected" ) }; @@ -119,7 +120,7 @@ pub(crate) async fn apply_policy_mutation( // rather than let a failed push read as a failed edit. push_policy_to_running_instances(state, &mutation) .await - .map_err(|AppError(status, error)| AppError(status, format!("edit saved and recorded; {error}")))?; + .map_err(|err| AppError::new(err.status, format!("edit saved and recorded; {}", err.body.error)))?; // Held through the VM acknowledgement: that is the end of the mutation. drop(mutation); Ok(event) @@ -151,7 +152,7 @@ async fn write_policy_mutation_event( error = %error, "policy mutation ledger write failed" ); - AppError( + AppError::new( StatusCode::INTERNAL_SERVER_ERROR, format!("policy mutation ledger write failed: {error}"), ) @@ -167,5 +168,5 @@ pub(crate) fn unix_timestamp_ms() -> i64 { } pub(crate) fn bad_request(error: String) -> AppError { - AppError(StatusCode::BAD_REQUEST, error) + AppError::new(StatusCode::BAD_REQUEST, error) } diff --git a/crates/capsem-service/src/private_routes.rs b/crates/capsem-service/src/private_routes.rs index 79de69094..d20bc3d8c 100644 --- a/crates/capsem-service/src/private_routes.rs +++ b/crates/capsem-service/src/private_routes.rs @@ -49,7 +49,7 @@ fn owner_of(state: &ServiceState, vm: &str, secret: &str) -> Result<(), AppError .map(|instance| instance.owner_secret.clone()); match expected { Some(minted) if secrets_match(secret, &minted) => Ok(()), - _ => Err(AppError( + _ => Err(AppError::new( StatusCode::FORBIDDEN, format!("VM {vm} has no running owner presenting this secret"), )), @@ -137,13 +137,13 @@ pub(super) async fn handle_private_resolve( } (None, Some(address)) => visible.iter().find(|member| member.address == address), _ => { - return Err(AppError( + return Err(AppError::new( StatusCode::BAD_REQUEST, "resolve names exactly one of a name or an address".into(), )) } }; - let member = found.ok_or_else(|| AppError(StatusCode::NOT_FOUND, "no such member".into()))?; + let member = found.ok_or_else(|| AppError::new(StatusCode::NOT_FOUND, "no such member".into()))?; Ok(Json(PrivateResolveResponse { name: member.full_name(), address: member.address, diff --git a/crates/capsem-service/src/router_runtime.rs b/crates/capsem-service/src/router_runtime.rs index 49bef61e5..71f6712c4 100644 --- a/crates/capsem-service/src/router_runtime.rs +++ b/crates/capsem-service/src/router_runtime.rs @@ -174,13 +174,13 @@ pub(super) async fn handle_system_status( }) .await??; capsem_assets::asset_manager::ManifestV2::from_json(&serde_json::to_string(&manifest).map_err(|error| { - AppError( + AppError::new( StatusCode::INTERNAL_SERVER_ERROR, format!("serialize installed manifest for validation: {error}"), ) })?) .map_err(|error| { - AppError( + AppError::new( StatusCode::INTERNAL_SERVER_ERROR, format!("installed manifest is invalid: {error:#}"), ) @@ -199,13 +199,13 @@ pub(super) async fn handle_system_status( pub(super) fn read_installed_status_document(path: &StdPath) -> Result { let content = std::fs::read_to_string(path).map_err(|error| { - AppError( + AppError::new( StatusCode::INTERNAL_SERVER_ERROR, format!("read installed status document {}: {error}", path.display()), ) })?; serde_json::from_str(&content).map_err(|error| { - AppError( + AppError::new( StatusCode::INTERNAL_SERVER_ERROR, format!("parse installed status document {}: {error}", path.display()), ) @@ -215,7 +215,7 @@ pub(super) fn read_installed_status_document(path: &StdPath) -> Result Result { let value = read_installed_status_document(path)?; if value.get("schema").and_then(serde_json::Value::as_str) != Some("capsem.manifest_metadata.v1") { - return Err(AppError( + return Err(AppError::new( StatusCode::INTERNAL_SERVER_ERROR, "installed manifest metadata must use schema capsem.manifest_metadata.v1".to_string(), )); @@ -243,7 +243,7 @@ pub(super) async fn handle_update_apply( return Ok(Json(planned_update_response(plan))); } if !request.confirmed { - return Err(AppError( + return Err(AppError::new( StatusCode::BAD_REQUEST, "update apply requires confirmed=true or dry_run=true".to_string(), )); @@ -289,7 +289,7 @@ pub(super) async fn execute_update_command_unlocked( .output() .await .map_err(|error| { - AppError( + AppError::new( StatusCode::INTERNAL_SERVER_ERROR, format!("failed to start update command: {error}"), ) @@ -373,7 +373,7 @@ pub(super) async fn run_automatic_update_once(state: &ServiceState) -> Automatic let plan = update_command_plan(UpdateCommandKind::Apply); let response = match execute_update_command_unlocked(plan).await { Ok(response) => response, - Err(error) => return AutomaticUpdateOutcome::Failed(error.1), + Err(error) => return AutomaticUpdateOutcome::Failed(error.body.error), }; if response.status != api::UpdateActionStatus::Succeeded { let detail = response @@ -386,7 +386,7 @@ pub(super) async fn run_automatic_update_once(state: &ServiceState) -> Automatic } match reload_activated_update_runtime(state) { Ok(disposition) => AutomaticUpdateOutcome::Succeeded(disposition), - Err(error) => AutomaticUpdateOutcome::Failed(error.1), + Err(error) => AutomaticUpdateOutcome::Failed(error.body.error), } } @@ -439,13 +439,13 @@ pub(super) fn should_start_automatic_update_loop(parent_pid: Option) -> boo pub(super) fn reload_activated_update_runtime(state: &ServiceState) -> Result { let path = state.assets_dir.join("manifest.json"); let content = std::fs::read_to_string(&path).map_err(|error| { - AppError( + AppError::new( StatusCode::INTERNAL_SERVER_ERROR, format!("read activated update manifest {}: {error}", path.display()), ) })?; let manifest = capsem_assets::asset_manager::ManifestV2::from_json(&content).map_err(|error| { - AppError( + AppError::new( StatusCode::INTERNAL_SERVER_ERROR, format!("validate activated update manifest {}: {error:#}", path.display()), ) @@ -455,7 +455,7 @@ pub(super) fn reload_activated_update_runtime(state: &ServiceState) -> Result let uds_path = running_uds_path(state, id)?; send_ipc_command(&uds_path, request, Some(OWNER_TIMEOUT_SECS)) .await - .map_err(|e| AppError(StatusCode::BAD_GATEWAY, format!("VM owner unavailable: {e}"))) + .map_err(|e| AppError::new(StatusCode::BAD_GATEWAY, format!("VM owner unavailable: {e}"))) } fn unexpected(reply: &ProcessToService) -> AppError { - AppError( + AppError::new( StatusCode::INTERNAL_SERVER_ERROR, format!("unexpected VM owner reply: {:?}", std::mem::discriminant(reply)), ) @@ -78,7 +78,7 @@ pub(crate) async fn create_exposure( ) -> Result { let target = proto_target(request.target); if !target.admits(request.guest_port) { - return Err(AppError( + return Err(AppError::new( StatusCode::BAD_REQUEST, format!( "guest port {} cannot be exposed from the {:?} namespace", @@ -95,7 +95,7 @@ pub(crate) async fn create_exposure( }, ExposureAccess::HttpPreview => { if request.host_port != 0 { - return Err(AppError( + return Err(AppError::new( StatusCode::BAD_REQUEST, "http_preview does not accept host_port".into(), )); @@ -108,7 +108,7 @@ pub(crate) async fn create_exposure( .filter(|port| *port != 0) }) .await? - .ok_or_else(|| AppError(StatusCode::SERVICE_UNAVAILABLE, "preview listener unavailable".into()))?; + .ok_or_else(|| AppError::new(StatusCode::SERVICE_UNAVAILABLE, "preview listener unavailable".into()))?; ServiceToProcess::DeclarePreview { id: state.next_job_id(), listener_port, @@ -128,8 +128,8 @@ pub(crate) async fn create_exposure( error: Some(error), policy_refused: true, .. - } => Err(AppError(StatusCode::FORBIDDEN, error)), - ProcessToService::PortPublished { error: Some(error), .. } => Err(AppError(StatusCode::CONFLICT, error)), + } => Err(AppError::new(StatusCode::FORBIDDEN, error)), + ProcessToService::PortPublished { error: Some(error), .. } => Err(AppError::new(StatusCode::CONFLICT, error)), other => Err(unexpected(&other)), } } @@ -165,7 +165,7 @@ pub(crate) async fn handle_delete_exposure( State(state): State>, Path((id, exposure_id)): Path<(String, String)>, ) -> Result, AppError> { - let not_found = || AppError(StatusCode::NOT_FOUND, format!("exposure not found: {exposure_id}")); + let not_found = || AppError::new(StatusCode::NOT_FOUND, format!("exposure not found: {exposure_id}")); if exposure_id.is_empty() || exposure_id.len() > 64 { return Err(not_found()); } @@ -176,7 +176,7 @@ pub(crate) async fn handle_delete_exposure( match ask_owner(&state, &id, request).await? { ProcessToService::ExposureRevoked { revoked: true, .. } => Ok(Json(api::VmActionResponse { success: true })), ProcessToService::ExposureRevoked { error: Some(error), .. } => { - Err(AppError(StatusCode::INTERNAL_SERVER_ERROR, error)) + Err(AppError::new(StatusCode::INTERNAL_SERVER_ERROR, error)) } ProcessToService::ExposureRevoked { .. } => Err(not_found()), other => Err(unexpected(&other)), @@ -207,7 +207,7 @@ async fn preview_publication( .map(api_publication) .map(|exposure| (generation, exposure)) .ok_or_else(|| { - AppError( + AppError::new( StatusCode::NOT_FOUND, format!("preview exposure not found: {exposure_id}"), ) @@ -243,7 +243,7 @@ pub(crate) async fn handle_create_preview_session( expires_in_seconds, })), ProcessToService::PreviewSessionCreated { error: Some(error), .. } => { - Err(AppError(StatusCode::UNAUTHORIZED, error)) + Err(AppError::new(StatusCode::UNAUTHORIZED, error)) } other => Err(unexpected(&other)), } @@ -269,7 +269,7 @@ pub(crate) async fn handle_revoke_preview_sessions( revoked, error: None, .. } => Ok(Json(PreviewSessionsRevokedResponse { revoked })), ProcessToService::PreviewSessionsRevoked { error: Some(error), .. } => { - Err(AppError(StatusCode::NOT_FOUND, error)) + Err(AppError::new(StatusCode::NOT_FOUND, error)) } other => Err(unexpected(&other)), } @@ -301,7 +301,7 @@ pub(crate) async fn handle_exchange_preview_bootstrap( expires_in_seconds, })), ProcessToService::PreviewBootstrapExchanged { error: Some(error), .. } => { - Err(AppError(StatusCode::UNAUTHORIZED, error)) + Err(AppError::new(StatusCode::UNAUTHORIZED, error)) } other => Err(unexpected(&other)), } @@ -340,7 +340,7 @@ pub(crate) async fn handle_admit_preview_connection( owner_generation: owner_generation.to_string(), })), ProcessToService::PreviewConnectionAdmitted { error: Some(error), .. } => { - Err(AppError(StatusCode::UNAUTHORIZED, error)) + Err(AppError::new(StatusCode::UNAUTHORIZED, error)) } other => Err(unexpected(&other)), } diff --git a/crates/capsem-service/src/router_runtime/restart.rs b/crates/capsem-service/src/router_runtime/restart.rs index 0f23f487a..07462e7f7 100644 --- a/crates/capsem-service/src/router_runtime/restart.rs +++ b/crates/capsem-service/src/router_runtime/restart.rs @@ -8,7 +8,7 @@ pub(super) async fn handle_restart( let _update = state .update_lock .try_lock() - .map_err(|_| AppError(StatusCode::CONFLICT, "an update is in progress".to_string()))?; + .map_err(|_| AppError::new(StatusCode::CONFLICT, "an update is in progress".to_string()))?; let manager = capsem_service::management::managed_service().await; accept_restart(&state, manager) } @@ -18,7 +18,7 @@ pub(crate) fn accept_restart( manager: Option, ) -> Result<(StatusCode, Json), AppError> { let manager = manager.ok_or_else(|| { - AppError( + AppError::new( StatusCode::SERVICE_UNAVAILABLE, "restart requires a launchd or systemd service configured to restart clean exits".to_string(), ) @@ -26,7 +26,7 @@ pub(crate) fn accept_restart( state .lifecycle .begin_restart(|| !state.instances.lock().unwrap().is_empty()) - .map_err(|error| AppError(StatusCode::CONFLICT, error.to_string()))?; + .map_err(|error| AppError::new(StatusCode::CONFLICT, error.to_string()))?; state.update_restart.notify_one(); Ok(( StatusCode::ACCEPTED, diff --git a/crates/capsem-service/src/router_runtime/streams.rs b/crates/capsem-service/src/router_runtime/streams.rs index 615a9befc..164ac0449 100644 --- a/crates/capsem-service/src/router_runtime/streams.rs +++ b/crates/capsem-service/src/router_runtime/streams.rs @@ -33,7 +33,7 @@ pub(crate) async fn handle_stream( ) -> Result { let upgrade = upgrade.protocols([stream::STREAM_SUBPROTOCOL]); if upgrade.selected_protocol().is_none() { - return Err(AppError( + return Err(AppError::new( StatusCode::BAD_REQUEST, format!( "stream requires the {} WebSocket subprotocol", diff --git a/crates/capsem-service/src/service_runtime/telemetry_export.rs b/crates/capsem-service/src/service_runtime/telemetry_export.rs index afe85284e..4a7cdb790 100644 --- a/crates/capsem-service/src/service_runtime/telemetry_export.rs +++ b/crates/capsem-service/src/service_runtime/telemetry_export.rs @@ -175,7 +175,7 @@ pub(crate) fn spawn_refresh(state: Arc, table: SessionTable) { loop { match collect(&state).await { Ok(totals) => *table.write().unwrap_or_else(|poisoned| poisoned.into_inner()) = totals, - Err(error) => warn!(error = %error.1, "session metric refresh failed"), + Err(error) => warn!(error = %error.body.error, "session metric refresh failed"), } tokio::time::sleep(REFRESH_INTERVAL).await; } diff --git a/crates/capsem-service/src/session_cleanup.rs b/crates/capsem-service/src/session_cleanup.rs index ff65a1daa..b519dd051 100644 --- a/crates/capsem-service/src/session_cleanup.rs +++ b/crates/capsem-service/src/session_cleanup.rs @@ -15,7 +15,7 @@ pub(super) async fn preserve_failed_run_shutdown_result( return Ok(None); }; if persistent { - return Err(AppError( + return Err(AppError::new( StatusCode::INTERNAL_SERVER_ERROR, format!("one-shot session {id} unexpectedly used persistent storage"), )); @@ -23,7 +23,7 @@ pub(super) async fn preserve_failed_run_shutdown_result( tokio::task::spawn_blocking(move || state.preserve_failed_session_dir(&session_dir, &id)) .await .map_err(|error| { - AppError( + AppError::new( StatusCode::INTERNAL_SERVER_ERROR, format!("failed session preservation task: {error}"), ) @@ -53,7 +53,7 @@ async fn ensure_failed_session_preserved( }) .await .map_err(|error| { - AppError( + AppError::new( StatusCode::INTERNAL_SERVER_ERROR, format!("failed session preservation task: {error}"), ) @@ -78,13 +78,13 @@ pub(super) async fn finalize_one_shot_session( tokio::task::spawn_blocking(move || state.delete_session_dir(&session_dir)) .await .map_err(|error| { - AppError( + AppError::new( StatusCode::INTERNAL_SERVER_ERROR, format!("one-shot cleanup task failed: {error}"), ) })? .map_err(|error| { - AppError( + AppError::new( StatusCode::INTERNAL_SERVER_ERROR, format!("one-shot cleanup failed: {error:#}"), ) @@ -99,6 +99,6 @@ pub(super) async fn handle_preserve_failure( let shutdown_result = shutdown_vm_process(&state, &id, ShutdownMode::Retain, None).await?; let _preserved = ensure_failed_session_preserved(Arc::clone(&state), id.clone(), shutdown_result) .await? - .ok_or_else(|| AppError(StatusCode::NOT_FOUND, format!("sandbox not found: {id}")))?; + .ok_or_else(|| AppError::vm_not_found(&id))?; Ok(Json(json!({ "success": true }))) } diff --git a/crates/capsem-service/src/settings_routes.rs b/crates/capsem-service/src/settings_routes.rs index 99761d6e0..0f1e8f317 100644 --- a/crates/capsem-service/src/settings_routes.rs +++ b/crates/capsem-service/src/settings_routes.rs @@ -25,7 +25,7 @@ pub(super) async fn handle_save_settings( ) -> Result, AppError> { let _mutation = state.policy_mutation.begin().await; capsem_core::net::policy_config::batch_update_settings_json(&raw) - .map_err(|e| AppError(StatusCode::BAD_REQUEST, e))?; + .map_err(|e| AppError::new(StatusCode::BAD_REQUEST, e))?; let resp = capsem_core::net::policy_config::load_settings_response(); Ok(Json(serde_json::to_value(resp).unwrap_or_default())) } @@ -38,19 +38,19 @@ pub(super) async fn handle_corp_config( use capsem_core::net::policy_config::corp_provision; let _mutation = state.policy_mutation.begin().await; let capsem_dir = capsem_foundation::paths::capsem_home_opt() - .ok_or(AppError(StatusCode::INTERNAL_SERVER_ERROR, "HOME not set".into()))?; + .ok_or(AppError::new(StatusCode::INTERNAL_SERVER_ERROR, "HOME not set".into()))?; if let Some(source) = &payload.source { corp_provision::provision_from_source(&capsem_dir, source) .await - .map_err(|e| AppError(StatusCode::BAD_REQUEST, e.to_string()))?; + .map_err(|e| AppError::new(StatusCode::BAD_REQUEST, e.to_string()))?; } else if let Some(toml_content) = &payload.toml { corp_provision::validate_corp_toml(toml_content) - .map_err(|e| AppError(StatusCode::BAD_REQUEST, e.to_string()))?; + .map_err(|e| AppError::new(StatusCode::BAD_REQUEST, e.to_string()))?; corp_provision::install_inline_corp_config(&capsem_dir, toml_content) - .map_err(|e| AppError(StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; + .map_err(|e| AppError::new(StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; } else { - return Err(AppError( + return Err(AppError::new( StatusCode::BAD_REQUEST, "provide either 'source' (URL) or 'toml' (inline content)".into(), )); @@ -68,7 +68,7 @@ pub(super) fn corp_info_value() -> Result { use capsem_core::net::policy_config::{corp_config_paths, corp_provision}; let capsem_dir = capsem_foundation::paths::capsem_home_opt() - .ok_or(AppError(StatusCode::INTERNAL_SERVER_ERROR, "HOME not set".into()))?; + .ok_or(AppError::new(StatusCode::INTERNAL_SERVER_ERROR, "HOME not set".into()))?; let paths: Vec<_> = corp_config_paths() .into_iter() .map(|path| { @@ -96,12 +96,12 @@ pub(super) async fn handle_corp_validate( let client = reqwest::Client::new(); corp_provision::fetch_corp_config(&client, source) .await - .map_err(|e| AppError(StatusCode::BAD_REQUEST, e.to_string()))?; + .map_err(|e| AppError::new(StatusCode::BAD_REQUEST, e.to_string()))?; } else if let Some(toml_content) = &payload.toml { corp_provision::validate_corp_toml(toml_content) - .map_err(|e| AppError(StatusCode::BAD_REQUEST, e.to_string()))?; + .map_err(|e| AppError::new(StatusCode::BAD_REQUEST, e.to_string()))?; } else { - return Err(AppError( + return Err(AppError::new( StatusCode::BAD_REQUEST, "provide either 'source' (URL) or 'toml' (inline content)".into(), )); @@ -117,7 +117,7 @@ pub(super) async fn handle_corp_reload( use capsem_core::net::policy_config::corp_provision; let capsem_dir = capsem_foundation::paths::capsem_home_opt() - .ok_or(AppError(StatusCode::INTERNAL_SERVER_ERROR, "HOME not set".into()))?; + .ok_or(AppError::new(StatusCode::INTERNAL_SERVER_ERROR, "HOME not set".into()))?; corp_provision::refresh_corp_config_if_stale(capsem_dir).await; handle_reload_config(State(state)).await } diff --git a/crates/capsem-service/src/tests/exec_target.rs b/crates/capsem-service/src/tests/exec_target.rs index 313e98ec9..8410c0582 100644 --- a/crates/capsem-service/src/tests/exec_target.rs +++ b/crates/capsem-service/src/tests/exec_target.rs @@ -84,8 +84,12 @@ async fn a_session_without_a_workload_execs_in_the_vm_and_refuses_a_workload_tar let refused = exec(&state, "plain-vm", Some(capsem_api::ExecTarget::Workload)) .await .unwrap_err(); - assert_eq!(refused.0, StatusCode::BAD_REQUEST); - assert!(refused.1.contains("no container workload"), "{}", refused.1); + assert_eq!(refused.status, StatusCode::BAD_REQUEST); + assert!( + refused.body.error.contains("no container workload"), + "{}", + refused.body.error + ); exec(&state, "plain-vm", None).await.unwrap(); // The refused request never reached the VM owner. @@ -94,3 +98,109 @@ async fn a_session_without_a_workload_execs_in_the_vm_and_refuses_a_workload_tar vec![("id -u".to_string(), Wire::Vm)] ); } + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn exec_timeout_and_missing_vm_routes_return_structured_error_codes() { + let _env_lock = SETTINGS_ENV_LOCK.lock().await; + let settings_dir = tempfile::tempdir().unwrap(); + let (_settings_guard, _, _) = install_empty_settings_env(&settings_dir); + let dir = tempfile::tempdir().unwrap(); + let mut state = make_test_state_owned(); + let sleep_bin = state.run_dir.join("sleep-proc.sh"); + std::fs::write(&sleep_bin, "#!/bin/sh\nsleep 2\n").unwrap(); + #[cfg(unix)] + std::fs::set_permissions(&sleep_bin, std::os::unix::fs::PermissionsExt::from_mode(0o755)).unwrap(); + state.process_binary = sleep_bin; + let state = Arc::new(state); + install_test_runtime_assets(&state); + let st = || State(Arc::clone(&state)); + let sleep_handler = |msg: &ServiceToProcess| -> FakeProcessReply { + let sleep = matches!(msg, ServiceToProcess::Exec { command, .. } if command == "sleep"); + Box::pin(async move { + if sleep { + tokio::time::sleep(std::time::Duration::from_millis(1200)).await; + } + None + }) + }; + let watcher = Arc::clone(&state); + let run_owner = tokio::spawn(async move { + let uds = loop { + if let Some(info) = watcher.instances.lock().unwrap().values().next() { + break info.uds_path.clone(); + } + tokio::time::sleep(std::time::Duration::from_millis(10)).await; + }; + spawn_fake_process(&uds, 2, sleep_handler).await.unwrap() + }); + let run_req: RunRequest = serde_json::from_value(json!({"command": "sleep", "timeout_secs": 1})).unwrap(); + let run_err = handle_run(st(), Json(run_req)).await.unwrap_err(); + assert_eq!(run_err.status, StatusCode::GATEWAY_TIMEOUT); + assert_eq!(run_err.body.code, Some(ErrorCode::ExecTimeout)); + assert_eq!(run_err.body.timeout_secs, Some(1)); + assert_eq!(run_err.body.error, "exec failed: IPC command timed out after 1s"); + let _ = run_owner.await; + + let uds_path = dir.path().join("exec.sock"); + let owner = spawn_fake_process(&uds_path, 2, sleep_handler); + setup_vm_with_workspace_and_uds(&state, dir.path(), "plain-vm", uds_path.clone()); + let plain = || Path("plain-vm".to_string()); + let req = |cmd: &str, t: u64| { + let body: ExecRequest = serde_json::from_value(json!({"command": cmd, "timeout_secs": t})).unwrap(); + handle_exec(st(), plain(), Json(body)) + }; + let timeout_err = req("sleep", 1).await.unwrap_err(); + assert_eq!(timeout_err.status, StatusCode::GATEWAY_TIMEOUT); + assert_eq!(timeout_err.body.code, Some(ErrorCode::ExecTimeout)); + assert_eq!(timeout_err.body.timeout_secs, Some(1)); + assert_eq!(timeout_err.body.error, "exec failed: IPC command timed out after 1s"); + let failed_err = req("closed", 5).await.unwrap_err(); + assert_eq!(failed_err.status, StatusCode::INTERNAL_SERVER_ERROR); + assert!(failed_err.body.error.starts_with("IPC connection closed")); + let _ = owner.await; + let _ = std::fs::remove_file(&uds_path); + let fork_req = |n: &str| Json(serde_json::from_value::(json!({"name": n})).unwrap()); + let fork_ipc_err = handle_fork(st(), plain(), fork_req("f")).await.unwrap_err(); + assert_eq!(fork_ipc_err.status, StatusCode::INTERNAL_SERVER_ERROR); + let existing = test_persistent_entry("unresumable", dir.path().join("unresumable")); + let existing_id = existing.id.clone(); + state + .persistent_registry + .lock() + .unwrap() + .data + .vms + .insert("unresumable".into(), existing); + let resume_existing_err = handle_resume(st(), Path(existing_id)).await.unwrap_err(); + assert_eq!(resume_existing_err.status, StatusCode::NOT_FOUND); + assert_eq!(resume_existing_err.body.code, None); + assert_eq!(resume_existing_err.body.vm_id, None); + std::fs::create_dir_all(dir.path().join("session.db")).unwrap(); + let _ = handle_vm_status(st(), plain()).await; + let missing = || Path("missing-vm".to_string()); + let net_id = "00000000-0000-0000-0000-000000000001".to_string(); + let net_path = Path((net_id, "missing-vm".to_string())); + let log_q: LogQuery = serde_json::from_str("{}").unwrap(); + let persist_req = Json(serde_json::from_str::(r#"{"name":"s"}"#).unwrap()); + let attach = crate::network_routes::handle_network_attach; + let not_found_errors = [ + attach(st(), net_path).await.unwrap_err(), + handle_preserve_failure(st(), missing()).await.unwrap_err(), + handle_logs(st(), missing(), Query(log_q)).await.unwrap_err(), + handle_info(st(), missing()).await.unwrap_err(), + handle_vm_status(st(), missing()).await.unwrap_err(), + running_uds_path(&state, "missing-vm").unwrap_err(), + resolve_session_dir(&state, "missing-vm").unwrap_err(), + handle_suspend(st(), missing()).await.unwrap_err(), + handle_stop(st(), missing()).await.unwrap_err(), + handle_delete(st(), missing()).await.unwrap_err(), + handle_persist(st(), missing(), persist_req).await.unwrap_err(), + handle_resume(st(), missing()).await.unwrap_err(), + handle_fork(st(), missing(), fork_req("forked")).await.unwrap_err(), + ]; + for err in not_found_errors { + assert_eq!(err.status, StatusCode::NOT_FOUND); + assert_eq!(err.body.code, Some(ErrorCode::VmNotFound)); + assert_eq!(err.body.vm_id.as_deref(), Some("missing-vm")); + } +} diff --git a/crates/capsem-service/src/tests/files_api.rs b/crates/capsem-service/src/tests/files_api.rs index 96a1f09d6..d2e34f3f7 100644 --- a/crates/capsem-service/src/tests/files_api.rs +++ b/crates/capsem-service/src/tests/files_api.rs @@ -423,8 +423,8 @@ async fn upload_does_not_write_workspace_file_when_import_ledger_fails() { .await .expect_err("failed import ledger write must fail closed"); - assert_eq!(err.0, StatusCode::INTERNAL_SERVER_ERROR); - assert!(err.1.contains("security ledger rejected import")); + assert_eq!(err.status, StatusCode::INTERNAL_SERVER_ERROR); + assert!(err.body.error.contains("security ledger rejected import")); let msg = ipc.await.unwrap(); assert!(matches!(msg, ServiceToProcess::LogFileBoundary { .. })); assert!( @@ -546,7 +546,7 @@ fn download_nonexistent_file_resolves_but_does_not_open() { let (parent, name) = resolve_workspace_target(&state, "404-vm", "nonexistent.txt", false).unwrap(); assert_eq!(parent.entry_kind(&name).unwrap(), None); let err = parent.open_file(&name, ContainedOpenOptions::read_only()).unwrap_err(); - assert_eq!(workspace_io_error(err).0, StatusCode::NOT_FOUND); + assert_eq!(workspace_io_error(err).status, StatusCode::NOT_FOUND); } // is_launchd_cleanup_transient identifies the misleading "missing @@ -607,7 +607,7 @@ async fn upload_refuses_a_dangling_symlink_target() { .await .expect_err("an upload through a guest symlink must be refused"); - assert_eq!(err.0, StatusCode::FORBIDDEN, "{}", err.1); + assert_eq!(err.status, StatusCode::FORBIDDEN, "{}", err.body.error); assert!(!planted.exists(), "the symlink target must not be created on the host"); } @@ -630,7 +630,7 @@ async fn upload_refuses_a_symlinked_parent_even_when_the_leaf_directory_is_missi .await .expect_err("an upload below a guest symlink must be refused"); - assert_eq!(err.0, StatusCode::FORBIDDEN, "{}", err.1); + assert_eq!(err.status, StatusCode::FORBIDDEN, "{}", err.body.error); assert!( !tree.outside.join("sub").exists(), "no directory may be created outside the workspace" @@ -655,7 +655,7 @@ async fn download_refuses_a_symlink_to_a_host_file() { .await .expect_err("a download through a guest symlink must be refused"); - assert_eq!(err.0, StatusCode::FORBIDDEN, "{}", err.1); + assert_eq!(err.status, StatusCode::FORBIDDEN, "{}", err.body.error); } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] @@ -676,7 +676,7 @@ async fn listing_neither_follows_nor_shows_a_symlinked_directory() { ) .await .expect_err("listing through a guest symlink must be refused"); - assert_eq!(err.0, StatusCode::FORBIDDEN, "{}", err.1); + assert_eq!(err.status, StatusCode::FORBIDDEN, "{}", err.body.error); let root = handle_list_files( State(state), @@ -955,10 +955,10 @@ async fn handle_provision_rejects_invalid_labels_with_bad_request() { .await .expect_err("invalid labels must be rejected"); assert_eq!( - err.0, + err.status, StatusCode::BAD_REQUEST, "expected 400 for {bad_labels:?}: {}", - err.1 + err.body.error ); } } diff --git a/crates/capsem-service/src/tests/fork.rs b/crates/capsem-service/src/tests/fork.rs index 8d329dbb2..4f8895e64 100644 --- a/crates/capsem-service/src/tests/fork.rs +++ b/crates/capsem-service/src/tests/fork.rs @@ -84,7 +84,7 @@ async fn a_fork_whose_guest_will_not_freeze_fails_and_leaves_nothing() { ) .await .unwrap_err(); - assert!(error.1.contains("SnapshotReady"), "{}", error.1); + assert!(error.body.error.contains("SnapshotReady"), "{}", error.body.error); assert!(state.persistent_registry.lock().unwrap().get("my-fork").is_none()); let persistent = state.run_dir.join("persistent"); let leftovers = std::fs::read_dir(&persistent).map(|dir| dir.count()).unwrap_or(0); @@ -111,7 +111,7 @@ async fn handle_fork_not_found() { ) .await .unwrap_err(); - assert_eq!(err.0, StatusCode::NOT_FOUND); + assert_eq!(err.status, StatusCode::NOT_FOUND); } #[tokio::test] @@ -158,7 +158,7 @@ async fn handle_fork_duplicate_returns_conflict() { ) .await .unwrap_err(); - assert_eq!(err.0, StatusCode::CONFLICT); + assert_eq!(err.status, StatusCode::CONFLICT); } #[tokio::test] @@ -267,7 +267,7 @@ async fn handle_fork_from_persistent_registry() { ) .await .unwrap_err(); - assert_eq!(bad.0, StatusCode::BAD_REQUEST, "{}", bad.1); + assert_eq!(bad.status, StatusCode::BAD_REQUEST, "{}", bad.body.error); } /// A VM from before profiles were removed is refused, not laundered into a @@ -302,7 +302,11 @@ async fn handle_fork_refuses_a_profile_era_vm() { ) .await .unwrap_err(); - assert_eq!(err.0, StatusCode::PRECONDITION_FAILED); - assert!(err.1.contains("profiles no longer exist"), "{}", err.1); + assert_eq!(err.status, StatusCode::PRECONDITION_FAILED); + assert!( + err.body.error.contains("profiles no longer exist"), + "{}", + err.body.error + ); assert!(!state.persistent_registry.lock().unwrap().contains("blocked-fork")); } diff --git a/crates/capsem-service/src/tests/ledger_routes/bodies.rs b/crates/capsem-service/src/tests/ledger_routes/bodies.rs index 1dfd66c82..2ee2d63c8 100644 --- a/crates/capsem-service/src/tests/ledger_routes/bodies.rs +++ b/crates/capsem-service/src/tests/ledger_routes/bodies.rs @@ -46,11 +46,12 @@ fn an_event_id_is_twelve_lowercase_hex_characters_or_a_bad_request() { "0123456789ag", // 'g' is not hex "0123456789 b", ] { - let AppError(status, message) = validate_event_id(rejected).expect_err(rejected); - assert_eq!(status, StatusCode::BAD_REQUEST, "{rejected}"); + let err = validate_event_id(rejected).expect_err(rejected); + assert_eq!(err.status, StatusCode::BAD_REQUEST, "{rejected}"); assert!( - message.contains("12 lowercase hex characters"), - "the refusal must name the constraint, got {message:?}" + err.body.error.contains("12 lowercase hex characters"), + "the refusal must name the constraint, got {:?}", + err.body.error ); } } diff --git a/crates/capsem-service/src/tests/lifecycle.rs b/crates/capsem-service/src/tests/lifecycle.rs index 21cbb5c0f..7e9e01211 100644 --- a/crates/capsem-service/src/tests/lifecycle.rs +++ b/crates/capsem-service/src/tests/lifecycle.rs @@ -410,7 +410,7 @@ async fn handle_vm_operation_status_rejects_unknown_vm() { let err = handle_vm_save_status(State(state), Path("missing-vm".into())) .await .unwrap_err(); - assert_eq!(err.0, StatusCode::NOT_FOUND); + assert_eq!(err.status, StatusCode::NOT_FOUND); } #[tokio::test] @@ -435,8 +435,8 @@ async fn handle_suspend_rejects_ephemeral_vm() { let result = handle_suspend(State(state), Path("eph-vm".into())).await; let err = result.unwrap_err(); - assert_eq!(err.0, StatusCode::BAD_REQUEST); - assert!(err.1.contains("ephemeral")); + assert_eq!(err.status, StatusCode::BAD_REQUEST); + assert!(err.body.error.contains("ephemeral")); } #[tokio::test] @@ -444,7 +444,7 @@ async fn handle_suspend_returns_not_found_for_missing_vm() { let (state, _dir) = make_test_state_with_tempdir(); let result = handle_suspend(State(state), Path("nonexistent".into())).await; let err = result.unwrap_err(); - assert_eq!(err.0, StatusCode::NOT_FOUND); + assert_eq!(err.status, StatusCode::NOT_FOUND); } #[test] diff --git a/crates/capsem-service/src/tests/mcp_routes.rs b/crates/capsem-service/src/tests/mcp_routes.rs index 5a08633c5..fb9f8c7c8 100644 --- a/crates/capsem-service/src/tests/mcp_routes.rs +++ b/crates/capsem-service/src/tests/mcp_routes.rs @@ -188,6 +188,16 @@ async fn an_unconfigured_server_is_not_found() { "{error}" ); } + + insert_fake_instance_with_session_dir(&_state, "box", 1, dir.path().join("session")); + let (status, _error) = route_request( + app.clone(), + axum::http::Method::POST, + "/mcp/servers/local/tools/fetch_http/call", + Some(json!({})), + ) + .await; + assert_eq!(status, StatusCode::BAD_GATEWAY); } #[test] @@ -201,5 +211,5 @@ fn a_tool_id_must_belong_to_its_server() { "wiki__search" ); let error = crate::mcp_routes::resolve_mcp_tool_id("wiki", "notes__search").unwrap_err(); - assert_eq!(error.0, StatusCode::BAD_REQUEST); + assert_eq!(error.status, StatusCode::BAD_REQUEST); } diff --git a/crates/capsem-service/src/tests/persist_purge.rs b/crates/capsem-service/src/tests/persist_purge.rs index 7dd646dfe..5b4ad9764 100644 --- a/crates/capsem-service/src/tests/persist_purge.rs +++ b/crates/capsem-service/src/tests/persist_purge.rs @@ -100,7 +100,7 @@ async fn persist_refuses_a_duplicate_id_and_leaves_the_instance_ephemeral() { let error = persist(&state, "persist-src", "fresh-name") .await .expect_err("duplicate id must be refused"); - assert_eq!(error.0, StatusCode::INTERNAL_SERVER_ERROR, "{}", error.1); + assert_eq!(error.status, StatusCode::INTERNAL_SERVER_ERROR, "{}", error.body.error); assert_still_ephemeral(&state, "persist-src", &session_dir); let (claimed, entries) = { @@ -130,10 +130,10 @@ async fn racing_persists_on_one_name_leave_one_entry_and_no_orphan() { Ok(_) => winners.push(id), Err(error) => { assert_eq!( - error.0, + error.status, StatusCode::CONFLICT, "loser must see a name conflict: {}", - error.1 + error.body.error ); losers.push(id); } diff --git a/crates/capsem-service/src/tests/policy_push.rs b/crates/capsem-service/src/tests/policy_push.rs index 02588e05d..640eaa911 100644 --- a/crates/capsem-service/src/tests/policy_push.rs +++ b/crates/capsem-service/src/tests/policy_push.rs @@ -120,8 +120,12 @@ async fn an_edit_of_a_corp_owned_setting_is_refused() { .await .expect_err("corp owns this plugin"); - assert_eq!(error.0, StatusCode::BAD_REQUEST); - assert!(error.1.contains("set by the corp config"), "{}", error.1); + assert_eq!(error.status, StatusCode::BAD_REQUEST); + assert!( + error.body.error.contains("set by the corp config"), + "{}", + error.body.error + ); assert_eq!( std::fs::read(dir.path().join("settings.toml")).unwrap(), settings_before @@ -152,13 +156,14 @@ async fn a_reload_acknowledging_another_active_policy_fails_the_push() { let error = handle_mcp_default_edit(State(Arc::clone(&state)), permission(SecurityRuleAction::Block)) .await .expect_err("an acknowledgement of another policy is not this edit applied"); - assert_eq!(error.0, StatusCode::INTERNAL_SERVER_ERROR); + assert_eq!(error.status, StatusCode::INTERNAL_SERVER_ERROR); assert!( error - .1 + .body + .error .contains("stale-ack-vm: applied active policy blake3:some-other-policy, expected blake3:"), "{}", - error.1 + error.body.error ); process.await.unwrap(); } @@ -211,7 +216,7 @@ async fn race_edit_b_against_held_edit_a( () = held.notified() => {} finished = &mut edit_a => panic!( "edit A finished before its VM acknowledged it: {:?}", - finished.unwrap().err().map(|error| error.1) + finished.unwrap().err().map(|error| error.body.error) ), } let published_by_a = std::fs::read(&active_path).unwrap(); @@ -290,13 +295,14 @@ async fn partial_apply_reaches_every_healthy_vm_and_names_the_failed_one() { .await .expect_err("an edit that did not reach every running VM is not a success"); - assert_eq!(error.0, StatusCode::INTERNAL_SERVER_ERROR); + assert_eq!(error.status, StatusCode::INTERNAL_SERVER_ERROR); assert!( error - .1 + .body + .error .starts_with("edit saved and recorded; policy applied to 1 of 2 running VMs; not applied: broken-vm: "), "{}", - error.1 + error.body.error ); assert!(only_reloads(&healthy.await.unwrap(), 1), "the healthy VM is reloaded"); assert_eq!( @@ -346,5 +352,5 @@ async fn a_tool_of_an_undeclared_server_is_refused() { .await .expect_err("undeclared server"); - assert_eq!(error.0, StatusCode::BAD_REQUEST); + assert_eq!(error.status, StatusCode::BAD_REQUEST); } diff --git a/crates/capsem-service/src/tests/restart.rs b/crates/capsem-service/src/tests/restart.rs index 73348d345..ba261b032 100644 --- a/crates/capsem-service/src/tests/restart.rs +++ b/crates/capsem-service/src/tests/restart.rs @@ -16,7 +16,7 @@ async fn restart_acknowledges_acceptance_and_requires_new_gateway_credentials() assert_eq!( accept_restart(&state, Some(api::ServiceManager::Launchd)) .unwrap_err() - .0, + .status, StatusCode::CONFLICT ); } @@ -25,14 +25,14 @@ async fn restart_acknowledges_acceptance_and_requires_new_gateway_credentials() async fn restart_refuses_unmanaged_service_or_active_launch_without_notifying_shutdown() { let state = make_test_state(); assert_eq!( - accept_restart(&state, None).unwrap_err().0, + accept_restart(&state, None).unwrap_err().status, StatusCode::SERVICE_UNAVAILABLE ); let launch = state.lifecycle.admit().unwrap(); assert_eq!( accept_restart(&state, Some(api::ServiceManager::Systemd)) .unwrap_err() - .0, + .status, StatusCode::CONFLICT ); drop(launch); @@ -57,7 +57,7 @@ async fn restart_refuses_active_vms_without_changing_their_registry() { assert_eq!( accept_restart(&state, Some(api::ServiceManager::Systemd)) .unwrap_err() - .0, + .status, StatusCode::CONFLICT ); assert_eq!(state.instances.lock().unwrap()["active"].pid, before); diff --git a/crates/capsem-service/src/tests/settings_files.rs b/crates/capsem-service/src/tests/settings_files.rs index 43f044902..2a4effd83 100644 --- a/crates/capsem-service/src/tests/settings_files.rs +++ b/crates/capsem-service/src/tests/settings_files.rs @@ -113,7 +113,7 @@ async fn handle_save_settings_rejects_unknown_key() { let result = handle_save_settings(State(state), Json(changes)).await; assert!(result.is_err()); let err = result.unwrap_err(); - assert_eq!(err.0, StatusCode::BAD_REQUEST); + assert_eq!(err.status, StatusCode::BAD_REQUEST); } #[tokio::test] @@ -140,11 +140,11 @@ async fn handle_save_settings_rejects_retired_policy_rule_keys_atomically() { .await .expect_err("retired policy rule key should be rejected by settings handler"); - assert_eq!(err.0, StatusCode::BAD_REQUEST); + assert_eq!(err.status, StatusCode::BAD_REQUEST); assert!( - err.1.contains(&format!("unknown setting: {retired_key}")), + err.body.error.contains(&format!("unknown setting: {retired_key}")), "error should point to the retired policy key, got: {}", - err.1 + err.body.error ); let loaded = capsem_core::net::policy_config::load_settings_file(&user_path).unwrap(); assert!( @@ -435,8 +435,9 @@ fn classify_launchd_transient_routes_to_retry() { fn classify_boot_crash_bails_with_500_and_tail() { let tail = "Error: failed to boot VM\n\nCaused by:\n bogus".to_string(); match classify_attempt_decision(ProvisionAttemptOutcome::BootCrash { tail: tail.clone() }, "vm-4") { - AttemptDecision::BailWithError(AppError(status, msg)) => { - assert_eq!(status, StatusCode::INTERNAL_SERVER_ERROR); + AttemptDecision::BailWithError(err) => { + let msg = &err.body.error; + assert_eq!(err.status, StatusCode::INTERNAL_SERVER_ERROR); assert!(msg.contains("vm-4"), "msg should embed the id: {msg}"); assert!(msg.contains(&tail), "msg should embed the log tail: {msg}"); assert!(msg.contains("capsem logs vm-4"), "msg should hint at follow-up cmd"); @@ -449,9 +450,9 @@ fn classify_boot_crash_bails_with_500_and_tail() { fn classify_provision_error_already_exists_returns_409() { let err = anyhow::anyhow!("persistent VM \"vm-5\" already exists. Use `capsem resume vm-5`."); match classify_attempt_decision(ProvisionAttemptOutcome::ProvisionError(err), "vm-5") { - AttemptDecision::BailWithError(AppError(status, _)) => { + AttemptDecision::BailWithError(err) => { assert_eq!( - status, + err.status, StatusCode::CONFLICT, "duplicate-name errors must return 409 so clients can distinguish from server failures" ); @@ -464,8 +465,9 @@ fn classify_provision_error_already_exists_returns_409() { fn classify_provision_error_other_returns_500() { let err = anyhow::anyhow!("rootfs not found at /missing/path"); match classify_attempt_decision(ProvisionAttemptOutcome::ProvisionError(err), "vm-6") { - AttemptDecision::BailWithError(AppError(status, msg)) => { - assert_eq!(status, StatusCode::INTERNAL_SERVER_ERROR); + AttemptDecision::BailWithError(err) => { + let msg = &err.body.error; + assert_eq!(err.status, StatusCode::INTERNAL_SERVER_ERROR); assert!(msg.contains("rootfs not found"), "underlying error preserved: {msg}"); } other => panic!("expected BailWithError(500), got {other:?}"), @@ -477,10 +479,10 @@ fn classify_provision_error_reports_the_whole_cause_chain() { // #289: the caller saw only "load the policy inputs", never why. let err = anyhow::anyhow!("settings.toml cannot define corp.rules").context("load the policy inputs"); match classify_attempt_decision(ProvisionAttemptOutcome::ProvisionError(err), "vm-7") { - AttemptDecision::BailWithError(AppError(status, msg)) => { - assert_eq!(status, StatusCode::INTERNAL_SERVER_ERROR); + AttemptDecision::BailWithError(err) => { + assert_eq!(err.status, StatusCode::INTERNAL_SERVER_ERROR); assert_eq!( - msg, + err.body.error, "provision failed: load the policy inputs: settings.toml cannot define corp.rules" ); } @@ -492,8 +494,8 @@ fn classify_provision_error_reports_the_whole_cause_chain() { fn classify_provision_error_finds_already_exists_below_context() { let err = anyhow::anyhow!("persistent VM \"vm-8\" already exists").context("claim the persistent name"); match classify_attempt_decision(ProvisionAttemptOutcome::ProvisionError(err), "vm-8") { - AttemptDecision::BailWithError(AppError(status, msg)) => { - assert_eq!(status, StatusCode::CONFLICT, "{msg}"); + AttemptDecision::BailWithError(err) => { + assert_eq!(err.status, StatusCode::CONFLICT, "{}", err.body.error); } other => panic!("expected BailWithError(409), got {other:?}"), } diff --git a/crates/capsem-service/src/tests/system_contracts.rs b/crates/capsem-service/src/tests/system_contracts.rs index 07e55e41c..8d53ad5cd 100644 --- a/crates/capsem-service/src/tests/system_contracts.rs +++ b/crates/capsem-service/src/tests/system_contracts.rs @@ -45,8 +45,8 @@ fn system_status_rejects_noncanonical_manifest_metadata_schema() { let error = read_manifest_metadata_status_document(&path).unwrap_err(); - assert_eq!(error.0, StatusCode::INTERNAL_SERVER_ERROR); - assert!(error.1.contains("capsem.manifest_metadata.v1")); + assert_eq!(error.status, StatusCode::INTERNAL_SERVER_ERROR); + assert!(error.body.error.contains("capsem.manifest_metadata.v1")); } #[tokio::test] diff --git a/crates/capsem-service/src/tests/update_routes.rs b/crates/capsem-service/src/tests/update_routes.rs index 08e7737ab..fbca1002e 100644 --- a/crates/capsem-service/src/tests/update_routes.rs +++ b/crates/capsem-service/src/tests/update_routes.rs @@ -297,7 +297,7 @@ fn update_runtime_rejects_invalid_manifest_without_replacing_cached_graph() { let error = reload_activated_update_runtime(&state).unwrap_err(); - assert!(error.1.contains("validate activated update manifest")); + assert!(error.body.error.contains("validate activated update manifest")); assert_eq!( state.manifest.read().unwrap().as_ref().unwrap().assets.current, "assets-1" @@ -477,11 +477,17 @@ async fn automatic_update_reports_command_failure_for_backoff() { let state = make_test_state(); let outcome = run_automatic_update_once(&state).await; + std::fs::write(&cli, "#!/bin/sh\nexit 0\n").unwrap(); + let reload_outcome = run_automatic_update_once(&state).await; + std::env::set_var("CAPSEM_CLI", dir.path().join("missing-capsem")); + let spawn_outcome = run_automatic_update_once(&state).await; match previous_cli { Some(value) => std::env::set_var("CAPSEM_CLI", value), None => std::env::remove_var("CAPSEM_CLI"), } + assert!(matches!(reload_outcome, AutomaticUpdateOutcome::Failed(_))); + assert!(matches!(spawn_outcome, AutomaticUpdateOutcome::Failed(_))); let AutomaticUpdateOutcome::Failed(error) = outcome else { panic!("expected automatic update failure"); }; diff --git a/crates/capsem-service/src/vm_files.rs b/crates/capsem-service/src/vm_files.rs index bc5175853..bfa280035 100644 --- a/crates/capsem-service/src/vm_files.rs +++ b/crates/capsem-service/src/vm_files.rs @@ -13,7 +13,7 @@ mod exec; mod file_boundary; pub(super) use file_boundary::{log_file_boundary, log_file_boundary_on_owner}; mod fork; -mod ipc_command; +pub(crate) mod ipc_command; pub(crate) use diagnostics::{handle_host_logs, handle_logs, handle_panics, handle_service_logs, handle_triage}; #[cfg(test)] pub(crate) use diagnostics::{session_db_triage, session_triage_statements}; @@ -284,7 +284,7 @@ pub(super) fn workspace_root(state: &ServiceState, id: &str) -> Result Result AppError { if is_symlink_refusal(&e) { - AppError( + AppError::new( StatusCode::FORBIDDEN, "path leaves the workspace through a symlink".into(), ) } else if e.kind() == std::io::ErrorKind::NotFound { - AppError(StatusCode::NOT_FOUND, "path not found".into()) + AppError::new(StatusCode::NOT_FOUND, "path not found".into()) } else if e.kind() == std::io::ErrorKind::InvalidInput || is_not_directory(&e) { - AppError(StatusCode::BAD_REQUEST, format!("not a workspace path: {e}")) + AppError::new(StatusCode::BAD_REQUEST, format!("not a workspace path: {e}")) } else { - AppError(StatusCode::INTERNAL_SERVER_ERROR, format!("workspace: {e}")) + AppError::new(StatusCode::INTERNAL_SERVER_ERROR, format!("workspace: {e}")) } } @@ -322,7 +322,7 @@ pub(super) fn resolve_workspace_target( let rel = StdPath::new(sanitized); let name = rel .file_name() - .ok_or_else(|| AppError(StatusCode::BAD_REQUEST, "path has no file name".into()))? + .ok_or_else(|| AppError::new(StatusCode::BAD_REQUEST, "path has no file name".into()))? .to_owned(); let parent_rel = rel.parent().unwrap_or_else(|| StdPath::new("")); let root = workspace_root(state, id)?; @@ -333,7 +333,7 @@ pub(super) fn resolve_workspace_target( } .map_err(workspace_io_error)?; if parent.entry_kind(&name).map_err(workspace_io_error)? == Some(EntryKind::Other) { - return Err(AppError( + return Err(AppError::new( StatusCode::FORBIDDEN, "path names a symlink or special file".into(), )); @@ -428,7 +428,7 @@ pub(super) async fn handle_list_files( // Directory reads are blocking I/O -- run in spawn_blocking let entries = tokio::task::spawn_blocking(move || list_dir_recursive(&target, &rel_path, 1, depth)) .await - .map_err(|e| AppError(StatusCode::INTERNAL_SERVER_ERROR, format!("list: {e}")))?; + .map_err(|e| AppError::new(StatusCode::INTERNAL_SERVER_ERROR, format!("list: {e}")))?; Ok(Json(FileListResponse { entries })) } @@ -443,7 +443,7 @@ pub(super) fn file_security_preview_bytes(data: &[u8]) -> Vec { pub(super) fn active_instance_uds_path(state: &Arc, id: &str) -> Result { let instances = state.instances.lock().unwrap(); instances.get(id).map(|i| i.uds_path.clone()).ok_or_else(|| { - AppError( + AppError::new( StatusCode::CONFLICT, "file import/export requires a running sandbox security ledger".into(), ) @@ -469,9 +469,9 @@ pub(super) async fn handle_download_file( let mut data = Vec::new(); file.take(MAX_FILE_SIZE + 1) .read_to_end(&mut data) - .map_err(|e| AppError(StatusCode::INTERNAL_SERVER_ERROR, format!("read: {e}")))?; + .map_err(|e| AppError::new(StatusCode::INTERNAL_SERVER_ERROR, format!("read: {e}")))?; if data.len() as u64 > MAX_FILE_SIZE { - return Err(AppError( + return Err(AppError::new( StatusCode::PAYLOAD_TOO_LARGE, format!("file too large (max {MAX_FILE_SIZE} bytes)"), )); @@ -486,7 +486,7 @@ pub(super) async fn handle_download_file( Ok::<_, AppError>((data, mime_str, safe_name)) }) .await - .map_err(|e| AppError(StatusCode::INTERNAL_SERVER_ERROR, format!("task: {e}")))??; + .map_err(|e| AppError::new(StatusCode::INTERNAL_SERVER_ERROR, format!("task: {e}")))??; let rewritten = log_file_boundary( &state, @@ -548,11 +548,11 @@ pub(super) async fn handle_upload_file( .open_file(&name, ContainedOpenOptions::write_create_truncate(0o644)) .map_err(workspace_io_error)?; file.write_all(&data) - .map_err(|e| AppError(StatusCode::INTERNAL_SERVER_ERROR, format!("write: {e}")))?; + .map_err(|e| AppError::new(StatusCode::INTERNAL_SERVER_ERROR, format!("write: {e}")))?; Ok::<_, AppError>(()) }) .await - .map_err(|e| AppError(StatusCode::INTERNAL_SERVER_ERROR, format!("task: {e}")))??; + .map_err(|e| AppError::new(StatusCode::INTERNAL_SERVER_ERROR, format!("task: {e}")))??; Ok(Json(UploadResponse { success: true, @@ -611,7 +611,7 @@ pub(super) fn classify_attempt_decision(outcome: ProvisionAttemptOutcome, id: &s | ProvisionAttemptOutcome::Launched { uds_path } | ProvisionAttemptOutcome::StillBootingTimedOut { uds_path } => AttemptDecision::Succeed(uds_path), ProvisionAttemptOutcome::LaunchdTransient => AttemptDecision::RetryAfterCleanup, - ProvisionAttemptOutcome::BootCrash { tail } => AttemptDecision::BailWithError(AppError( + ProvisionAttemptOutcome::BootCrash { tail } => AttemptDecision::BailWithError(AppError::new( StatusCode::INTERNAL_SERVER_ERROR, format!( "sandbox {id} failed to boot. process.log tail:\n\n{tail}\n\n\ @@ -632,7 +632,7 @@ async fn failed_process_log_tail(state: &Arc, id: &str) -> String None => "(no preserved log found)".to_string(), }) .await - .unwrap_or_else(|error| format!("(log read failed: {})", error.1)) + .unwrap_or_else(|error| format!("(log read failed: {})", error.body.error)) } pub(super) fn existing_session_names(state: &ServiceState) -> Vec { @@ -690,7 +690,7 @@ pub(super) async fn provision_attempt( Err(e) => { return ProvisionAttemptOutcome::ProvisionError(anyhow::anyhow!( "vz lifecycle lock acquire failed: {}", - e.1 + e.body.error )) } }; @@ -895,7 +895,7 @@ pub(super) async fn handle_info( return Ok(Json(info)); } - Err(AppError(StatusCode::NOT_FOUND, format!("sandbox not found: {id}"))) + Err(AppError::vm_not_found(&id)) } pub(super) async fn handle_vm_status( @@ -971,7 +971,7 @@ pub(super) async fn handle_vm_status( } } - Err(AppError(StatusCode::NOT_FOUND, format!("sandbox not found: {id}"))) + Err(AppError::vm_not_found(&id)) } pub(super) async fn vm_operation_status( @@ -1051,7 +1051,7 @@ pub(super) fn running_uds_path(state: &ServiceState, id: &str) -> Result state .off_worker(|_| triage::latest_app_log(&capsem_foundation::paths::capsem_home())) .await? - .ok_or_else(|| AppError(StatusCode::NOT_FOUND, "no app log found".into()))?, + .ok_or_else(|| AppError::new(StatusCode::NOT_FOUND, "no app log found".into()))?, HostLogSource::Service => state.run_dir.join("service.log"), HostLogSource::Mcp => state.run_dir.join("mcp.log"), HostLogSource::Gateway => state.run_dir.join("gateway.log"), @@ -346,7 +346,7 @@ pub(crate) async fn handle_host_logs( capsem_foundation::telemetry::read_log_tail(&path, max_bytes).unwrap_or_default() }) .await - .map_err(|e| AppError(StatusCode::INTERNAL_SERVER_ERROR, format!("log read failed: {e}")))?; + .map_err(|e| AppError::new(StatusCode::INTERNAL_SERVER_ERROR, format!("log read failed: {e}")))?; let text = filter_log(text, ¶ms); let json = headers.get_all(axum::http::header::ACCEPT).iter().any(|value| { @@ -405,8 +405,8 @@ pub(crate) async fn handle_service_logs(State(state): State>) .ok_or_else(|| format!("no log files in stream {}", log_path.display())) }) .await - .map_err(|e| AppError(StatusCode::INTERNAL_SERVER_ERROR, format!("log read failed: {e}")))? - .map_err(|e| AppError(StatusCode::INTERNAL_SERVER_ERROR, e))?; + .map_err(|e| AppError::new(StatusCode::INTERNAL_SERVER_ERROR, format!("log read failed: {e}")))? + .map_err(|e| AppError::new(StatusCode::INTERNAL_SERVER_ERROR, e))?; Ok(text) } diff --git a/crates/capsem-service/src/vm_files/exec.rs b/crates/capsem-service/src/vm_files/exec.rs index ba063e55b..502b5bcc5 100644 --- a/crates/capsem-service/src/vm_files/exec.rs +++ b/crates/capsem-service/src/vm_files/exec.rs @@ -15,17 +15,17 @@ pub(crate) async fn handle_exec( Json(payload): Json, ) -> Result, AppError> { let timeout_secs = - capsem_api::exec_timeout_secs(payload.timeout_secs).map_err(|e| AppError(StatusCode::BAD_REQUEST, e))?; + capsem_api::exec_timeout_secs(payload.timeout_secs).map_err(|e| AppError::new(StatusCode::BAD_REQUEST, e))?; let uds_path = running_uds_path(&state, &id)?; let target = capsem_api::exec_target( payload.target, crate::container_setup::runs_container(&state, &id).await, ) - .map_err(|e| AppError(StatusCode::BAD_REQUEST, e))?; + .map_err(|e| AppError::new(StatusCode::BAD_REQUEST, e))?; wait_for_vm_ready(&uds_path, 30, Some(&state), Some(&id)) .await - .map_err(|e| AppError(StatusCode::INTERNAL_SERVER_ERROR, e))?; + .map_err(|e| AppError::new(StatusCode::INTERNAL_SERVER_ERROR, e))?; let res = send_ipc_command( &uds_path, @@ -37,7 +37,7 @@ pub(crate) async fn handle_exec( Some(timeout_secs), ) .await - .map_err(|e| AppError(StatusCode::INTERNAL_SERVER_ERROR, e))?; + .map_err(|e| e.into_exec_app_error(false))?; match res { ProcessToService::ExecResult { @@ -52,7 +52,7 @@ pub(crate) async fn handle_exec( exit_code, truncated, })), - _ => Err(AppError( + _ => Err(AppError::new( StatusCode::INTERNAL_SERVER_ERROR, "unexpected IPC response for exec".to_string(), )), diff --git a/crates/capsem-service/src/vm_files/file_boundary.rs b/crates/capsem-service/src/vm_files/file_boundary.rs index 6ef1834a7..0effe2e61 100644 --- a/crates/capsem-service/src/vm_files/file_boundary.rs +++ b/crates/capsem-service/src/vm_files/file_boundary.rs @@ -14,7 +14,7 @@ pub(crate) async fn log_file_boundary( let uds_path = active_instance_uds_path(state, sandbox_id)?; wait_for_vm_ready(&uds_path, 30, Some(state), Some(sandbox_id)) .await - .map_err(|e| AppError(StatusCode::INTERNAL_SERVER_ERROR, e))?; + .map_err(|e| AppError::new(StatusCode::INTERNAL_SERVER_ERROR, e))?; log_file_boundary_on_owner(state, &uds_path, action, path, data_preview, size, mime_type).await } @@ -44,17 +44,17 @@ pub(crate) async fn log_file_boundary_on_owner( Some(5), ) .await - .map_err(|e| AppError(StatusCode::INTERNAL_SERVER_ERROR, e))?; + .map_err(ipc_command::IpcCommandError::into_internal_app_error)?; match res { ProcessToService::LogFileBoundaryResult { success: true, data, .. } => Ok(data), - ProcessToService::LogFileBoundaryResult { error, .. } => Err(AppError( + ProcessToService::LogFileBoundaryResult { error, .. } => Err(AppError::new( StatusCode::INTERNAL_SERVER_ERROR, error.unwrap_or_else(|| "failed to log file boundary".into()), )), - _ => Err(AppError( + _ => Err(AppError::new( StatusCode::INTERNAL_SERVER_ERROR, "unexpected IPC response for file boundary log".into(), )), diff --git a/crates/capsem-service/src/vm_files/fork.rs b/crates/capsem-service/src/vm_files/fork.rs index 2d14ca962..b2b7655c7 100644 --- a/crates/capsem-service/src/vm_files/fork.rs +++ b/crates/capsem-service/src/vm_files/fork.rs @@ -10,16 +10,16 @@ pub(crate) async fn handle_fork( let _launch = state .lifecycle .admit() - .map_err(|e| AppError(StatusCode::CONFLICT, e.to_string()))?; + .map_err(|e| AppError::new(StatusCode::CONFLICT, e.to_string()))?; let name = &payload.name; - validate_vm_name(name).map_err(|e| AppError(StatusCode::BAD_REQUEST, e.to_string()))?; - validate_vm_labels(payload.labels.as_ref()).map_err(|e| AppError(StatusCode::BAD_REQUEST, e.to_string()))?; + validate_vm_name(name).map_err(|e| AppError::new(StatusCode::BAD_REQUEST, e.to_string()))?; + validate_vm_labels(payload.labels.as_ref()).map_err(|e| AppError::new(StatusCode::BAD_REQUEST, e.to_string()))?; // Check name is not taken { let registry = state.persistent_registry.lock().unwrap(); if registry.contains(name) { - return Err(AppError( + return Err(AppError::new( StatusCode::CONFLICT, format!("sandbox '{}' already exists", name), )); @@ -43,15 +43,12 @@ pub(crate) async fn handle_fork( } else { drop(instances); let Some(p) = find_persistent_entry_by_route_id(&state, &id) else { - return Err(AppError( - StatusCode::NOT_FOUND, - format!("source sandbox not found: {}", id), - )); + return Err(AppError::vm_not_found(&id)); }; // A VM in the old shape is refused, never laundered into a fork. state .validate_persistent_entry(&p) - .map_err(|e| AppError(StatusCode::PRECONDITION_FAILED, e.to_string()))?; + .map_err(|e| AppError::new(StatusCode::PRECONDITION_FAILED, e.to_string()))?; ( p.session_dir, p.asset_pins, @@ -107,7 +104,7 @@ pub(crate) async fn handle_fork( state .off_worker(move |state| state.persistent_registry.lock().unwrap().register(entry)) .await? - .map_err(|e| AppError(StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; + .map_err(|e| AppError::new(StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; Ok(Json(ForkResponse { id: vm_id, @@ -174,7 +171,7 @@ async fn clone_guest_state( Err(error.unwrap_or_else(|| "the sandbox reported no clone size".into())) } Ok(other) => Err(format!("unexpected clone reply: {other:?}")), - Err(error) => Err(error), + Err(error) => Err(error.to_string()), }; if result.is_err() { // The owner removes what it wrote; this covers an owner that never diff --git a/crates/capsem-service/src/vm_files/ipc_command.rs b/crates/capsem-service/src/vm_files/ipc_command.rs index 9101084c8..cfbe90f07 100644 --- a/crates/capsem-service/src/vm_files/ipc_command.rs +++ b/crates/capsem-service/src/vm_files/ipc_command.rs @@ -1,31 +1,68 @@ //! One request and its correlated reply over a fresh VM-owner IPC connection. use super::*; +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) enum IpcCommandError { + Timeout { timeout_secs: u64 }, + Failed(String), +} + +impl std::fmt::Display for IpcCommandError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Timeout { timeout_secs } => write!(f, "IPC command timed out after {timeout_secs}s"), + Self::Failed(msg) => f.write_str(msg), + } + } +} + +impl From for String { + fn from(error: IpcCommandError) -> Self { + error.to_string() + } +} + +impl IpcCommandError { + pub(crate) fn into_exec_app_error(self, prefix_failed: bool) -> AppError { + match self { + Self::Timeout { timeout_secs } => AppError::exec_timeout(format!("exec failed: {self}"), timeout_secs), + Self::Failed(msg) if prefix_failed => { + AppError::new(StatusCode::INTERNAL_SERVER_ERROR, format!("exec failed: {msg}")) + } + Self::Failed(msg) => AppError::new(StatusCode::INTERNAL_SERVER_ERROR, msg), + } + } + + pub(crate) fn into_internal_app_error(self) -> AppError { + AppError::new(StatusCode::INTERNAL_SERVER_ERROR, self.to_string()) + } +} + #[tracing::instrument(skip_all, fields(cmd = ?std::mem::discriminant(&cmd), timeout_secs = ?timeout_secs))] pub(crate) async fn send_ipc_command( uds_path: &std::path::Path, cmd: ServiceToProcess, timeout_secs: Option, -) -> Result { +) -> Result { let stream = tokio::net::UnixStream::connect(uds_path) .await - .map_err(|e| format!("failed to connect to sandbox: {e}"))?; + .map_err(|e| IpcCommandError::Failed(format!("failed to connect to sandbox: {e}")))?; let std_stream = stream .into_std() - .map_err(|e| format!("failed to convert stream: {e}"))?; + .map_err(|e| IpcCommandError::Failed(format!("failed to convert stream: {e}")))?; let (std_stream, _) = capsem_foundation::ipc_handshake::negotiate_initiator_off_worker( std_stream, "capsem-service", capsem_foundation::telemetry::current_parent_traceparent(), ) .await - .map_err(|e| format!("IPC handshake failed: {e}"))?; - let (tx, rx): (Sender, Receiver) = - channel_from_std(std_stream).map_err(|e| format!("failed to create IPC channel: {e}"))?; + .map_err(|e| IpcCommandError::Failed(format!("IPC handshake failed: {e}")))?; + let (tx, rx): (Sender, Receiver) = channel_from_std(std_stream) + .map_err(|e| IpcCommandError::Failed(format!("failed to create IPC channel: {e}")))?; tx.send(cmd.clone()) .await - .map_err(|e| format!("failed to send IPC command: {e}"))?; + .map_err(|e| IpcCommandError::Failed(format!("failed to send IPC command: {e}")))?; let deadline = timeout_secs.map(|secs| tokio::time::Instant::now() + std::time::Duration::from_secs(secs)); loop { @@ -34,18 +71,18 @@ pub(crate) async fn send_ipc_command( Ok(Ok(msg)) => msg, Ok(Err(e)) => { error!(?e, "IPC receive error"); - return Err(format!("IPC connection closed: {e}")); + return Err(IpcCommandError::Failed(format!("IPC connection closed: {e}"))); } Err(_) => { - let secs = timeout_secs.unwrap_or_default(); - return Err(format!("IPC command timed out after {secs}s")); + let timeout_secs = timeout_secs.unwrap_or_default(); + return Err(IpcCommandError::Timeout { timeout_secs }); } }, None => match rx.recv().await { Ok(msg) => msg, Err(e) => { error!(?e, "IPC receive error"); - return Err(format!("IPC connection closed: {e}")); + return Err(IpcCommandError::Failed(format!("IPC connection closed: {e}"))); } }, }; @@ -62,3 +99,6 @@ pub(crate) async fn send_ipc_command( } } } + +#[cfg(test)] +mod tests; diff --git a/crates/capsem-service/src/vm_files/ipc_command/tests.rs b/crates/capsem-service/src/vm_files/ipc_command/tests.rs new file mode 100644 index 000000000..5126e843a --- /dev/null +++ b/crates/capsem-service/src/vm_files/ipc_command/tests.rs @@ -0,0 +1,88 @@ +use super::*; + +async fn accept_and_handshake(listener: &tokio::net::UnixListener) -> std::os::unix::net::UnixStream { + let (stream, _) = listener.accept().await.unwrap(); + let std_stream = stream.into_std().unwrap(); + tokio::task::spawn_blocking(move || { + let mut s = std_stream; + capsem_foundation::ipc_handshake::negotiate_responder(&mut s, "capsem-process", "").unwrap(); + s + }) + .await + .unwrap() +} + +fn peer_channel(s: std::os::unix::net::UnixStream) -> (Sender, Receiver) { + channel_from_std(s).unwrap() +} + +#[tokio::test] +async fn send_ipc_command_covers_connect_handshake_send_close_timeout_and_reply_paths() { + let failed = IpcCommandError::Failed("channel broken".to_string()); + assert_eq!( + failed.clone().into_internal_app_error().status, + StatusCode::INTERNAL_SERVER_ERROR + ); + assert_eq!( + failed.clone().into_exec_app_error(true).body.error, + "exec failed: channel broken" + ); + assert_eq!(String::from(failed), "channel broken"); + let tmp = tempfile::tempdir().unwrap(); + let missing_sock = tmp.path().join("missing.sock"); + let err = send_ipc_command(&missing_sock, ServiceToProcess::Ping, Some(1)) + .await + .unwrap_err(); + assert!(matches!(&err, IpcCommandError::Failed(msg) if msg.contains("failed to connect to sandbox"))); + + let drop_sock = tmp.path().join("drop.sock"); + let drop_listener = tokio::net::UnixListener::bind(&drop_sock).unwrap(); + tokio::spawn(async move { + let (_stream, _) = drop_listener.accept().await.unwrap(); + }); + let err = send_ipc_command(&drop_sock, ServiceToProcess::Ping, Some(1)) + .await + .unwrap_err(); + assert!(matches!(&err, IpcCommandError::Failed(msg) if msg.contains("IPC handshake failed"))); + + let sock_path = tmp.path().join("owner.sock"); + let listener = tokio::net::UnixListener::bind(&sock_path).unwrap(); + let server = tokio::spawn(async move { + let (tx, rx) = peer_channel(accept_and_handshake(&listener).await); + assert!(matches!(rx.recv().await.unwrap(), ServiceToProcess::Ping)); + tx.send(ProcessToService::Pong).await.unwrap(); + let _std_stream2 = accept_and_handshake(&listener).await; + let (_tx3, rx3) = peer_channel(accept_and_handshake(&listener).await); + let _ = rx3.recv().await; + drop((_tx3, rx3)); + let (_tx4, rx4) = peer_channel(accept_and_handshake(&listener).await); + let _ = rx4.recv().await; + drop((_tx4, rx4)); + let (_tx5, rx5) = peer_channel(accept_and_handshake(&listener).await); + let _ = rx5.recv().await; + tokio::time::sleep(std::time::Duration::from_millis(200)).await; + }); + + let ping = |t| send_ipc_command(&sock_path, ServiceToProcess::Ping, t); + let reply = ping(None).await.unwrap(); + assert!(matches!(reply, ProcessToService::Pong)); + + let oversized = ServiceToProcess::Exec { + id: 1, + command: "x".repeat(capsem_foundation::ipc_channel::MAX_IPC_FRAME_SIZE as usize + 1), + target: capsem_proto::ipc::ExecTarget::Vm, + }; + let send_err = send_ipc_command(&sock_path, oversized, Some(5)).await.unwrap_err(); + assert!(matches!(&send_err, IpcCommandError::Failed(msg) if msg.contains("failed to send IPC command"))); + + let closed_deadline_err = ping(Some(5)).await.unwrap_err(); + assert!(matches!(&closed_deadline_err, IpcCommandError::Failed(msg) if msg.contains("IPC connection closed"))); + + let closed_no_deadline_err = ping(None).await.unwrap_err(); + assert!(matches!(&closed_no_deadline_err, IpcCommandError::Failed(msg) if msg.contains("IPC connection closed"))); + + let timeout_err = ping(Some(0)).await.unwrap_err(); + assert_eq!(timeout_err, IpcCommandError::Timeout { timeout_secs: 0 }); + assert_eq!(String::from(timeout_err), "IPC command timed out after 0s"); + server.abort(); +} diff --git a/crates/capsem-service/src/vm_files/provision.rs b/crates/capsem-service/src/vm_files/provision.rs index e7a9a6c06..9a173db4c 100644 --- a/crates/capsem-service/src/vm_files/provision.rs +++ b/crates/capsem-service/src/vm_files/provision.rs @@ -7,11 +7,11 @@ pub(crate) async fn handle_provision( let _launch = state .lifecycle .admit() - .map_err(|e| AppError(StatusCode::CONFLICT, e.to_string()))?; - validate_vm_labels(payload.labels.as_ref()).map_err(|e| AppError(StatusCode::BAD_REQUEST, e.to_string()))?; + .map_err(|e| AppError::new(StatusCode::CONFLICT, e.to_string()))?; + validate_vm_labels(payload.labels.as_ref()).map_err(|e| AppError::new(StatusCode::BAD_REQUEST, e.to_string()))?; let labels = non_empty_labels(payload.labels.clone()); if let Some(reason) = state.off_worker(|state| vm_asset_block_reason(&state)).await? { - return Err(AppError(StatusCode::PRECONDITION_FAILED, reason)); + return Err(AppError::new(StatusCode::PRECONDITION_FAILED, reason)); } let existing = state.off_worker(|state| existing_session_names(&state)).await?; @@ -21,7 +21,7 @@ pub(crate) async fn handle_provision( .unwrap_or_else(|| generate_session_name(existing.iter().map(|s| s.as_str()))); let persistent = payload.persistent || payload.name.is_some() || payload.from.is_some(); if existing.iter().any(|existing| existing == &name) { - return Err(AppError( + return Err(AppError::new( StatusCode::CONFLICT, format!("persistent VM \"{}\" already exists", name), )); @@ -133,7 +133,7 @@ pub(crate) async fn handle_provision( attempts = timed_out.attempts, "provision: launchd-cleanup retries exhausted" ); - Err(AppError( + Err(AppError::new( StatusCode::INTERNAL_SERVER_ERROR, format!( "sandbox {id} could not be provisioned after {} attempts ({}). \ @@ -158,7 +158,7 @@ pub(crate) async fn finish_create( container: Option, ) -> Result { let created = complete_create(state, id, networks, container).await; - if matches!(&created, Err(error) if error.0 != StatusCode::GATEWAY_TIMEOUT) { + if matches!(&created, Err(error) if error.status != StatusCode::GATEWAY_TIMEOUT) { discard_failed_create(state, id).await; } created @@ -180,7 +180,7 @@ pub(crate) async fn complete_create( if let Some(socket) = socket { crate::credential_routes::sync_memory(state, &socket) .await - .map_err(|error| AppError(StatusCode::SERVICE_UNAVAILABLE, error))?; + .map_err(|error| AppError::new(StatusCode::SERVICE_UNAVAILABLE, error))?; } network_routes::attach_provisioned(state, id, networks).await?; if container.is_none() { @@ -203,7 +203,7 @@ async fn discard_failed_create(state: &Arc, id: &str) { // must not also lose the ledger it could not read. let session_dir = resolve_session_dir(state, id).ok(); if let Err(error) = shutdown_vm_process(state, id, ShutdownMode::Retain, None).await { - error!(vm_id = id, error = %error.1, "failed create did not shut down cleanly"); + error!(vm_id = id, error = %error.body.error, "failed create did not shut down cleanly"); } if let Some(session_dir) = session_dir { let (owner, vm_id) = (Arc::clone(state), id.to_owned()); @@ -216,7 +216,7 @@ async fn discard_failed_create(state: &Arc, id: &str) { match state.off_worker(move |state| state.forget_persistent_entry(&key)).await { Ok(Ok(())) => {} Ok(Err(error)) => error!(vm_id = id, error = %error, "failed create kept its name"), - Err(error) => error!(vm_id = id, error = %error.1, "failed create kept its name"), + Err(error) => error!(vm_id = id, error = %error.body.error, "failed create kept its name"), } } network_routes::vm_deleted(state, id).await; @@ -231,7 +231,7 @@ pub(crate) fn provision_failure(e: &anyhow::Error) -> AppError { } else { StatusCode::INTERNAL_SERVER_ERROR }; - AppError(status, format!("provision failed: {message}")) + AppError::new(status, format!("provision failed: {message}")) } #[cfg(test)] diff --git a/crates/capsem-service/src/vm_files/provision/tests.rs b/crates/capsem-service/src/vm_files/provision/tests.rs index cc3b3c533..c409e8606 100644 --- a/crates/capsem-service/src/vm_files/provision/tests.rs +++ b/crates/capsem-service/src/vm_files/provision/tests.rs @@ -86,8 +86,12 @@ async fn a_failed_container_create_discards_the_vm_and_keeps_its_ledger() { let error = finish_create(&state, "box", &[], Some(spec)).await.unwrap_err(); owner.await.unwrap(); - assert_eq!(error.0, StatusCode::INTERNAL_SERVER_ERROR, "{}", error.1); - assert!(error.1.contains("registry refused the image"), "{}", error.1); + assert_eq!(error.status, StatusCode::INTERNAL_SERVER_ERROR, "{}", error.body.error); + assert!( + error.body.error.contains("registry refused the image"), + "{}", + error.body.error + ); assert!( !state.instances.lock().unwrap().contains_key("box"), "the VM stays registered" diff --git a/crates/capsem-service/src/vm_lifecycle.rs b/crates/capsem-service/src/vm_lifecycle.rs index 7855d4056..817bd7d38 100644 --- a/crates/capsem-service/src/vm_lifecycle.rs +++ b/crates/capsem-service/src/vm_lifecycle.rs @@ -103,7 +103,7 @@ pub(super) fn resolve_session_dir(state: &ServiceState, id: &str) -> Result Ok(Some(guard)), - Ok(None) => Err(AppError( + Ok(None) => Err(AppError::new( StatusCode::SERVICE_UNAVAILABLE, "another process holds the Apple VZ save/restore lock; retry shortly".into(), )), - Err(e) => Err(AppError( + Err(e) => Err(AppError::new( StatusCode::INTERNAL_SERVER_ERROR, format!("vz host lock acquire failed: {e:#}"), )), @@ -263,7 +263,7 @@ pub(super) async fn shutdown_vm_process( .get(id) .is_some_and(|i| expected_generation.is_some_and(|expected| expected != i.generation)) { - return Err(AppError( + return Err(AppError::new( StatusCode::CONFLICT, "VM spawn generation changed; cleanup ownership refused".into(), )); @@ -289,7 +289,7 @@ pub(super) async fn shutdown_vm_process( .retirements .wait(id, generation) .await - .map_err(|error| AppError(StatusCode::INTERNAL_SERVER_ERROR, error.to_string()))?; + .map_err(|error| AppError::new(StatusCode::INTERNAL_SERVER_ERROR, error.to_string()))?; } return Ok(None); }; @@ -308,7 +308,7 @@ pub(super) async fn shutdown_vm_process( .retirements .wait(id, generation) .await - .map_err(|error| AppError(StatusCode::INTERNAL_SERVER_ERROR, error.to_string()))?; + .map_err(|error| AppError::new(StatusCode::INTERNAL_SERVER_ERROR, error.to_string()))?; return Ok(None); } state.containers.cancel_and_wait(id).await; @@ -363,7 +363,7 @@ pub(super) async fn shutdown_vm_process( .record_host_session_stopped(id, "stopped", mode.retains_state()) .await .map_err(|error| { - AppError( + AppError::new( StatusCode::INTERNAL_SERVER_ERROR, format!("host ledger stop failed for {id}: {error}"), ) @@ -376,9 +376,9 @@ pub(super) async fn shutdown_vm_process( .retirements .wait(id, generation) .await - .map_err(|error| AppError(StatusCode::INTERNAL_SERVER_ERROR, error.to_string()))?; + .map_err(|error| AppError::new(StatusCode::INTERNAL_SERVER_ERROR, error.to_string()))?; if expected_generation.is_some() && !retired { - return Err(AppError( + return Err(AppError::new( StatusCode::SERVICE_UNAVAILABLE, "original child retirement is unknown".into(), )); @@ -399,11 +399,9 @@ pub(super) async fn handle_suspend( let (uds_path, pid) = { let mut instances = state.instances.lock().unwrap(); - let i = instances - .get_mut(&id) - .ok_or_else(|| AppError(StatusCode::NOT_FOUND, format!("sandbox not found: {id}")))?; + let i = instances.get_mut(&id).ok_or_else(|| AppError::vm_not_found(&id))?; if !i.persistent { - return Err(AppError( + return Err(AppError::new( StatusCode::BAD_REQUEST, "ephemeral VMs cannot be suspended (persist first)".into(), )); @@ -414,13 +412,13 @@ pub(super) async fn handle_suspend( }; let stream = tokio::net::UnixStream::connect(&uds_path).await.map_err(|e| { - AppError( + AppError::new( StatusCode::INTERNAL_SERVER_ERROR, format!("failed to connect to VM IPC: {e}"), ) })?; let std_stream = stream.into_std().map_err(|e| { - AppError( + AppError::new( StatusCode::INTERNAL_SERVER_ERROR, format!("failed to convert stream: {e}"), ) @@ -431,9 +429,9 @@ pub(super) async fn handle_suspend( capsem_foundation::telemetry::current_parent_traceparent(), ) .await - .map_err(|e| AppError(StatusCode::INTERNAL_SERVER_ERROR, format!("IPC handshake failed: {e}")))?; + .map_err(|e| AppError::new(StatusCode::INTERNAL_SERVER_ERROR, format!("IPC handshake failed: {e}")))?; let (tx, rx) = channel_from_std::(std_stream).map_err(|e| { - AppError( + AppError::new( StatusCode::INTERNAL_SERVER_ERROR, format!("failed to create IPC channel: {e}"), ) @@ -443,7 +441,7 @@ pub(super) async fn handle_suspend( tx.send(ServiceToProcess::Suspend { checkpoint_path }) .await .map_err(|e| { - AppError( + AppError::new( StatusCode::INTERNAL_SERVER_ERROR, format!("failed to send suspend command: {e}"), ) @@ -489,7 +487,7 @@ pub(super) async fn handle_suspend( state.instances.lock().unwrap().remove(&id); let _ = std::fs::remove_file(&uds_path); let _ = std::fs::remove_file(uds_path.with_extension("ready")); - return Err(AppError(StatusCode::INTERNAL_SERVER_ERROR, error)); + return Err(AppError::new(StatusCode::INTERNAL_SERVER_ERROR, error)); } // Channel closure proves the process released IPC; prove the process also @@ -540,7 +538,7 @@ pub(super) async fn handle_stop( .off_worker(move |state| state.delete_session_dir(&session_dir)) .await? .map_err(|error| { - AppError( + AppError::new( StatusCode::INTERNAL_SERVER_ERROR, format!("ephemeral session cleanup failed: {error:#}"), ) @@ -551,7 +549,7 @@ pub(super) async fn handle_stop( persistent, })) } else { - Err(AppError(StatusCode::NOT_FOUND, format!("sandbox not found: {id}"))) + Err(AppError::vm_not_found(&id)) } } @@ -569,7 +567,7 @@ pub(super) async fn handle_delete( if let Some(entry) = find_persistent_entry_by_route_id(&state, &id) { entry.session_dir } else { - return Err(AppError(StatusCode::NOT_FOUND, format!("sandbox not found: {id}"))); + return Err(AppError::vm_not_found(&id)); } }; @@ -582,13 +580,13 @@ pub(super) async fn handle_delete( tokio::task::spawn_blocking(move || state_clone.delete_session_dir(&session_dir)) .await .map_err(|error| { - AppError( + AppError::new( StatusCode::INTERNAL_SERVER_ERROR, format!("delete session task failed: {error}"), ) })? .map_err(|error| { - AppError( + AppError::new( StatusCode::INTERNAL_SERVER_ERROR, format!("delete session state failed: {error:#}"), ) @@ -602,7 +600,7 @@ pub(super) async fn handle_delete( .off_worker(move |state| state.forget_persistent_entry(&key)) .await? .map_err(|error| { - AppError( + AppError::new( StatusCode::INTERNAL_SERVER_ERROR, format!("unregister deleted session failed: {error:#}"), ) @@ -616,7 +614,7 @@ pub(super) async fn handle_delete( pub(super) fn provision_response_for_running(state: &ServiceState, id: String) -> Result { let instances = state.instances.lock().unwrap(); let instance = instances.get(&id).ok_or_else(|| { - AppError( + AppError::new( StatusCode::INTERNAL_SERVER_ERROR, format!("provisioned VM missing from runtime registry: {id}"), ) @@ -640,16 +638,14 @@ pub(super) async fn handle_persist( Json(payload): Json, ) -> Result, AppError> { let name = &payload.name; - validate_vm_name(name).map_err(|e| AppError(StatusCode::BAD_REQUEST, e.to_string()))?; + validate_vm_name(name).map_err(|e| AppError::new(StatusCode::BAD_REQUEST, e.to_string()))?; // Find the running ephemeral instance let (live_session_dir, asset_pins, ram_mb, cpus, base_version, forked_from, env, labels) = { let instances = state.instances.lock().unwrap(); - let i = instances - .get(&id) - .ok_or_else(|| AppError(StatusCode::NOT_FOUND, format!("sandbox not found: {id}")))?; + let i = instances.get(&id).ok_or_else(|| AppError::vm_not_found(&id))?; if i.persistent { - return Err(AppError( + return Err(AppError::new( StatusCode::BAD_REQUEST, format!("VM \"{}\" is already persistent", id), )); @@ -699,7 +695,7 @@ pub(super) async fn handle_persist( let claim_state = Arc::clone(&state); tokio::task::spawn_blocking(move || claim_persistent_name(&claim_state, entry)) .await - .map_err(|e| AppError(StatusCode::INTERNAL_SERVER_ERROR, format!("persist task failed: {e}")))??; + .map_err(|e| AppError::new(StatusCode::INTERNAL_SERVER_ERROR, format!("persist task failed: {e}")))??; // Update instance info in-place; the session dir and its DB handle stay. { @@ -820,13 +816,13 @@ pub(super) async fn handle_run( Json(payload): Json, ) -> Result, AppError> { let timeout_secs = - capsem_api::exec_timeout_secs(payload.timeout_secs).map_err(|e| AppError(StatusCode::BAD_REQUEST, e))?; + capsem_api::exec_timeout_secs(payload.timeout_secs).map_err(|e| AppError::new(StatusCode::BAD_REQUEST, e))?; let _launch = state .lifecycle .admit() - .map_err(|e| AppError(StatusCode::CONFLICT, e.to_string()))?; + .map_err(|e| AppError::new(StatusCode::CONFLICT, e.to_string()))?; if let Some(reason) = state.off_worker(|state| vm_asset_block_reason(&state)).await? { - return Err(AppError(StatusCode::PRECONDITION_FAILED, reason)); + return Err(AppError::new(StatusCode::PRECONDITION_FAILED, reason)); } let id = { @@ -867,7 +863,7 @@ pub(super) async fn handle_run( }) }) .await - .map_err(|e| AppError(StatusCode::INTERNAL_SERVER_ERROR, format!("provision task: {e}")))?; + .map_err(|e| AppError::new(StatusCode::INTERNAL_SERVER_ERROR, format!("provision task: {e}")))?; provision_result.map_err(|e| provision_failure(&e))?; // 3. Wait for VM socket to appear while still holding the VZ @@ -876,7 +872,7 @@ pub(super) async fn handle_run( // sibling-service overlap this lock exists to prevent. let uds_path = state .instance_socket_path(&id) - .map_err(|e| AppError(StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; + .map_err(|e| AppError::new(StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; if let Err(e) = wait_for_vm_ready(&uds_path, 30, Some(&state), Some(&id)).await { drop(_vz_host_guard); drop(_vz_guard); @@ -890,13 +886,13 @@ pub(super) async fn handle_run( // is still flushing. let shutdown_result = shutdown_vm_process(&state, &id, ShutdownMode::Retain, None).await?; preserve_failed_run_shutdown_result(Arc::clone(&state), id.clone(), shutdown_result).await?; - return Err(AppError(StatusCode::INTERNAL_SERVER_ERROR, e)); + return Err(AppError::new(StatusCode::INTERNAL_SERVER_ERROR, e)); } } let uds_path = state .instance_socket_path(&id) - .map_err(|e| AppError(StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; + .map_err(|e| AppError::new(StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; // 2. Execute command. let job_id = state.next_job_id(); @@ -920,7 +916,7 @@ pub(super) async fn handle_run( let failed = !matches!(&exec_result, Ok(ProcessToService::ExecResult { .. })); finalize_one_shot_session(Arc::clone(&state), id.clone(), shutdown_result, failed).await?; - let response = match exec_result { + match exec_result { Ok(ProcessToService::ExecResult { stdout, stderr, @@ -933,11 +929,10 @@ pub(super) async fn handle_run( exit_code, truncated, })), - Ok(_) => Err(AppError( + Ok(_) => Err(AppError::new( StatusCode::INTERNAL_SERVER_ERROR, "unexpected IPC response".into(), )), - Err(e) => Err(AppError(StatusCode::INTERNAL_SERVER_ERROR, format!("exec failed: {e}"))), - }; - response + Err(e) => Err(e.into_exec_app_error(true)), + } } diff --git a/crates/capsem-service/src/vm_lifecycle/resume.rs b/crates/capsem-service/src/vm_lifecycle/resume.rs index 9f69e8cef..c45c208df 100644 --- a/crates/capsem-service/src/vm_lifecycle/resume.rs +++ b/crates/capsem-service/src/vm_lifecycle/resume.rs @@ -10,7 +10,7 @@ pub(crate) async fn handle_resume( let _launch = state .lifecycle .admit() - .map_err(|e| AppError(StatusCode::CONFLICT, e.to_string()))?; + .map_err(|e| AppError::new(StatusCode::CONFLICT, e.to_string()))?; // See handle_suspend: same lock, same reason. Restore happens in the // freshly spawned capsem-process's boot, so the lock must bridge the // spawn and the readiness sentinel for a sibling save_state not to @@ -30,7 +30,7 @@ pub(crate) async fn handle_resume( Ok(resumed_id) => { let uds_path = state .instance_socket_path(&resumed_id) - .map_err(|e| AppError(StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; + .map_err(|e| AppError::new(StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; if let Err(e) = wait_for_vm_ready(&uds_path, 30, Some(&state), Some(&resumed_id)).await { error!(id, error = %e, "resume ready-wait failed"); if attempted_checkpoint { @@ -50,12 +50,12 @@ pub(crate) async fn handle_resume( Ok(cold_id) => { let cold_uds_path = state .instance_socket_path(&cold_id) - .map_err(|e| AppError(StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; + .map_err(|e| AppError::new(StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; if let Err(cold_e) = wait_for_vm_ready(&cold_uds_path, 30, Some(&state), Some(&cold_id)).await { error!(id, "cold resume fallback failed after warm restore failure: {cold_e}"); - return Err(AppError( + return Err(AppError::new( StatusCode::INTERNAL_SERVER_ERROR, format!( "resume failed: warm restore failed ({e}); cold fallback failed ({cold_e})" @@ -74,14 +74,14 @@ pub(crate) async fn handle_resume( id, "cold resume fallback spawn failed after warm restore failure: {cold_e}" ); - return Err(AppError( + return Err(AppError::new( StatusCode::INTERNAL_SERVER_ERROR, format!("resume failed: warm restore failed ({e}); cold fallback failed ({cold_e})"), )); } } } - return Err(AppError( + return Err(AppError::new( StatusCode::INTERNAL_SERVER_ERROR, format!("resume failed: {e}"), )); @@ -95,7 +95,11 @@ pub(crate) async fn handle_resume( } Err(e) => { error!(id, error = %e, "resume failed"); - Err(AppError(StatusCode::NOT_FOUND, format!("resume failed: {e}"))) + let mut err = AppError::new(StatusCode::NOT_FOUND, format!("resume failed: {e}")); + if find_persistent_entry_by_route_id(&state, &id).is_none() { + err = err.with_code(ErrorCode::VmNotFound).with_vm_id(&id); + } + Err(err) } } } diff --git a/crates/capsem-service/src/vm_lifecycle/session_dirs.rs b/crates/capsem-service/src/vm_lifecycle/session_dirs.rs index b1408f83b..4c9d63eef 100644 --- a/crates/capsem-service/src/vm_lifecycle/session_dirs.rs +++ b/crates/capsem-service/src/vm_lifecycle/session_dirs.rs @@ -14,14 +14,14 @@ use super::*; pub(super) fn claim_persistent_name(state: &ServiceState, entry: PersistentVmEntry) -> Result<(), AppError> { let registry = state.persistent_registry.lock().unwrap(); if registry.contains(&entry.name) { - return Err(AppError( + return Err(AppError::new( StatusCode::CONFLICT, format!("persistent VM \"{}\" already exists", entry.name), )); } registry .register(entry) - .map_err(|error| AppError(StatusCode::INTERNAL_SERVER_ERROR, error.to_string())) + .map_err(|error| AppError::new(StatusCode::INTERNAL_SERVER_ERROR, error.to_string())) } /// Move a persisted session that still lives where it ran to diff --git a/crates/capsem-service/src/vm_lifecycle/transcript.rs b/crates/capsem-service/src/vm_lifecycle/transcript.rs index cd7f2ee70..eddddb818 100644 --- a/crates/capsem-service/src/vm_lifecycle/transcript.rs +++ b/crates/capsem-service/src/vm_lifecycle/transcript.rs @@ -30,13 +30,13 @@ pub(crate) async fn handle_history_transcript( }) .await .map_err(|e| { - AppError( + AppError::new( StatusCode::INTERNAL_SERVER_ERROR, format!("transcript read task failed: {e}"), ) })? .map_err(|e| { - AppError( + AppError::new( StatusCode::INTERNAL_SERVER_ERROR, format!("failed to read pty.log: {e}"), ) diff --git a/sdk/python/README.md b/sdk/python/README.md index fa6e3399f..bb9d5210a 100644 --- a/sdk/python/README.md +++ b/sdk/python/README.md @@ -40,15 +40,16 @@ python3 build_system/scripts/ci/run-bounded-command.py --timeout-seconds 90 -- \ Focused source tests do not need this package build. Clean-install checks use the prewarmed offline dependency cache and remove their runtime environments. -An async client for the Capsem HTTP gateway. It takes an explicit gateway URL -and bearer token; it does not discover services, open local service sockets, -or run host commands. +An async client for the Capsem HTTP gateway. Pass an explicit gateway URL and +bearer token, or omit either to resolve them via `discover_gateway()` from +`CAPSEM_GATEWAY_URL`, `CAPSEM_GATEWAY_TOKEN`, and `/gateway.{port,token}` +(`CAPSEM_RUN_DIR`, `CAPSEM_HOME/run`, or `~/.capsem/run`). ```python from capsem import Hypervisor, VM from capsem.models import HostLogSource, TimelineLayer -async with Hypervisor("http://127.0.0.1:19222", token, timeout=120) as hv: +async with Hypervisor.connect(timeout=120) as hv: overview = await hv.info() # health, versions, assets, updates network = await hv.networks.create("private") vm = await hv.create( @@ -104,17 +105,23 @@ from `capsem`. Omitted or `None` target keeps the service default: workload when present, otherwise VM. A refused workload target is returned as an error; the SDK does not switch targets or replay the command. -Objects and enums live in `capsem.models`. `HttpError` exposes the gateway's -HTTP `status` and response `body`; invalid typed responses raise Pydantic -`ValidationError`. HTTP `timeout` is the client deadline, while `exec`'s -`timeout_secs` is the command deadline sent to the gateway. Choose an HTTP -deadline long enough for the command. Mutations are never automatically retried. -Create, `start`, and `resume` use at least 230 seconds for workload readiness -and the gateway budget, retaining a larger configured client deadline. -Cancelling their coroutine cancels the request without replaying the mutation. -Printing an execution result prints its decoded stdout. `stdout_bytes` and -`stderr_bytes` preserve exact bytes regardless of whether the wire value uses -UTF-8 or base64; the exit code and typed wire fields remain available. +Objects and enums live in `capsem.models`. `CapsemError` is the base SDK +exception. `HttpError` exposes the gateway's HTTP `status`, response `body`, +parsed `response` (`ErrorResponse | None`), and `code` (`ErrorCode | None`); +`CapsemTimeoutError` (`CreateTimeoutError`, `ExecTimeoutError`) and +`VmNotFoundError` provide structured fields (`vm_id`, `vm_name`, `vm`, +`deadline_secs`, `command`, `timeout_secs`, `status`, `body`) while remaining +subclasses of `TimeoutError`/`LookupError`. Invalid typed +responses raise Pydantic `ValidationError`. HTTP `timeout` is the client +deadline, while `exec`'s `timeout_secs` is the command deadline sent to the +gateway. Choose an HTTP deadline long enough for the command. Mutations are +never automatically retried. Create, `start`, and `resume` use at least 230 +seconds for workload readiness and the gateway budget, retaining a larger +configured client deadline. Cancelling their coroutine cancels the request +without replaying the mutation. Printing an execution result prints its decoded +stdout. `stdout_bytes` and `stderr_bytes` preserve exact bytes regardless of +whether the wire value uses UTF-8 or base64; the exit code and typed wire +fields remain available. A VM selected by name resolves once, then retains its canonical ID. Handles returned by `create` and `fork` share their parent's connection. Close the diff --git a/sdk/python/capsem/__init__.py b/sdk/python/capsem/__init__.py index 458836d19..4693fda72 100644 --- a/sdk/python/capsem/__init__.py +++ b/sdk/python/capsem/__init__.py @@ -6,7 +6,17 @@ from ._mcp import McpServer as McpServer from ._mcp import McpTools as McpTools from ._ports import Port as Port +from ._transport import CapsemError as CapsemError from ._transport import HttpError as HttpError +from .discovery import DEFAULT_GATEWAY_PORT as DEFAULT_GATEWAY_PORT +from .discovery import DEFAULT_GATEWAY_URL as DEFAULT_GATEWAY_URL +from .discovery import GatewayEndpoint as GatewayEndpoint +from .discovery import capsem_run_dir as capsem_run_dir +from .discovery import discover_gateway as discover_gateway +from .errors import CapsemTimeoutError as CapsemTimeoutError +from .errors import CreateTimeoutError as CreateTimeoutError +from .errors import ExecTimeoutError as ExecTimeoutError +from .errors import VmNotFoundError as VmNotFoundError from .execution import ExecResult as ExecResult from .hypervisor import Hypervisor as Hypervisor from .registry import Registry as Registry diff --git a/sdk/python/capsem/_client.py b/sdk/python/capsem/_client.py index e080a2768..2ffb54789 100644 --- a/sdk/python/capsem/_client.py +++ b/sdk/python/capsem/_client.py @@ -2,13 +2,19 @@ from __future__ import annotations +from pathlib import Path from typing import Self from ._transport import Transport +from .discovery import discover_gateway class Client: - def __init__(self, url: str, token: str, *, timeout: float = 30) -> None: + def __init__(self, url: str | None = None, token: str | None = None, *, + run_dir: str | Path | None = None, timeout: float = 30) -> None: + if url is None or token is None: + endpoint = discover_gateway(url=url, token=token, run_dir=run_dir) + url, token = endpoint.url, endpoint.token self.__transport = Transport(url, token, timeout=timeout) self._owns_connection = True self._closed = False diff --git a/sdk/python/capsem/_resources.py b/sdk/python/capsem/_resources.py index af20ad3ed..a80db8ee3 100644 --- a/sdk/python/capsem/_resources.py +++ b/sdk/python/capsem/_resources.py @@ -22,26 +22,31 @@ class Files(Resource): relative to the workspace root, even when it is absolute.""" async def read(self, path: str, *, exact: bool = False) -> bytes: - return await api.download_vm_file( - self._vm._transport, id=await self._vm._resolve(), path=path, exact=exact or None, - ) + vm_id = await self._vm._resolve() + return await self._vm._call(vm_id, api.download_vm_file( + self._vm._transport, id=vm_id, path=path, exact=exact or None, + )) async def write(self, path: str, data: bytes, *, exact: bool = False) -> models.UploadResponse: - return await api.upload_vm_file( - self._vm._transport, id=await self._vm._resolve(), path=path, exact=exact or None, body=data, - ) + vm_id = await self._vm._resolve() + return await self._vm._call(vm_id, api.upload_vm_file( + self._vm._transport, id=vm_id, path=path, exact=exact or None, body=data, + )) async def list(self, path: str = "", *, depth: int | None = None, exact: bool = False) -> models.FileListResponse: """An empty `path` lists the workspace root.""" - return await api.list_vm_files( - self._vm._transport, id=await self._vm._resolve(), + vm_id = await self._vm._resolve() + return await self._vm._call(vm_id, api.list_vm_files( + self._vm._transport, id=vm_id, path=path or None, depth=depth, exact=exact or None, - ) + )) class Stats(Resource): async def summary(self) -> models.VmStatsSummaryResponse: - return await api.get_vm_stats_summary(self._vm._transport, id=await self._vm._resolve()) + vm_id = await self._vm._resolve() + return await self._vm._call(vm_id, api.get_vm_stats_summary(self._vm._transport, id=vm_id)) async def details(self) -> models.VmStatsDetailResponse: - return await api.get_vm_stats_detail(self._vm._transport, id=await self._vm._resolve()) + vm_id = await self._vm._resolve() + return await self._vm._call(vm_id, api.get_vm_stats_detail(self._vm._transport, id=vm_id)) diff --git a/sdk/python/capsem/_transport.py b/sdk/python/capsem/_transport.py index 3c531bc46..1eacd3bde 100644 --- a/sdk/python/capsem/_transport.py +++ b/sdk/python/capsem/_transport.py @@ -5,6 +5,7 @@ import io import math from collections.abc import Mapping, Sequence +from contextlib import suppress from enum import StrEnum from typing import Self from urllib.parse import quote, urlencode, urlsplit @@ -13,6 +14,8 @@ from pydantic import BaseModel, TypeAdapter from yarl import URL +from .models.error_code import ErrorCode +from .models.error_response import ErrorResponse from .models.model_base import JsonValue QueryValue = str | int | bool | Sequence[str | int | bool] | None @@ -31,14 +34,35 @@ class MediaType(StrEnum): GZIP = "application/gzip" -class HttpError(Exception): +class CapsemError(Exception): + """Base exception for Capsem SDK errors.""" + + +class HttpError(CapsemError): """The gateway returned an unsuccessful HTTP status.""" + status: int + body: str + response: ErrorResponse | None + code: ErrorCode | None + def __init__(self, status: int, body: str) -> None: self.status = status self.body = body + self.response = None + with suppress(ValueError): + self.response = ErrorResponse.model_validate_json(body) + self.code = self.response.code if self.response is not None else None super().__init__(f"HTTP {status}: {body}") + def __reduce__(self) -> tuple[type[Self], tuple[object, ...], dict[str, object]]: + return (type(self), (self.status, self.body), self.__dict__.copy()) + + def __setstate__(self, state: dict[str, object] | None, /) -> None: + if state is not None: + self.__dict__.update(state) + self.args = (f"HTTP {self.status}: {self.body}",) + def _query_value(value: str | int | bool) -> str: return str(value).lower() if isinstance(value, bool) else str(value) @@ -66,6 +90,9 @@ def timeout(self) -> float: """The default per-request deadline, in seconds.""" return self._timeout + def __getstate__(self) -> dict[str, object]: + return {**self.__dict__, "_session": None} + async def __aenter__(self) -> Self: return self diff --git a/sdk/python/capsem/discovery.py b/sdk/python/capsem/discovery.py new file mode 100644 index 000000000..3afe66e6e --- /dev/null +++ b/sdk/python/capsem/discovery.py @@ -0,0 +1,88 @@ +"""Host gateway URL and bearer token discovery from environment and `~/.capsem/run`.""" + +from __future__ import annotations + +import os +from dataclasses import dataclass +from pathlib import Path + +DEFAULT_GATEWAY_PORT = 19222 +DEFAULT_GATEWAY_URL = f"http://127.0.0.1:{DEFAULT_GATEWAY_PORT}" + + +@dataclass(frozen=True, slots=True) +class GatewayEndpoint: + """Resolved HTTP gateway base URL and bearer token.""" + + url: str + token: str + run_dir: Path | None = None + + +def capsem_run_dir(run_dir: str | Path | None = None) -> Path: + """Resolve the Capsem runtime directory matching `capsem_foundation::paths::capsem_run_dir`. + + Priority: + 1. Explicit `run_dir` argument (when provided and non-empty). + 2. `CAPSEM_RUN_DIR` environment variable. + 3. `CAPSEM_HOME/run` when `CAPSEM_HOME` is set. + 4. `~/.capsem/run`. + """ + if run_dir is not None: + raw = str(run_dir).strip() + if not raw: + raise ValueError("run_dir must be a nonempty path") + return Path(raw).expanduser() + env_run = os.environ.get("CAPSEM_RUN_DIR", "").strip() + if env_run: + return Path(env_run).expanduser() + env_home = os.environ.get("CAPSEM_HOME", "").strip() + if env_home: + return Path(env_home).expanduser() / "run" + return Path.home() / ".capsem" / "run" + + +def _resolve_gateway_url(url: str | None, resolved_run_dir: Path) -> str: + if url is not None: + return url + env_url = os.environ.get("CAPSEM_GATEWAY_URL", "").strip() + if env_url: + return env_url.rstrip("/") + port_file = resolved_run_dir / "gateway.port" + if port_file.is_file(): + raw_port = port_file.read_text(encoding="utf-8").strip() + if raw_port: + try: + port = int(raw_port) + except ValueError as error: + raise ValueError(f"invalid port in {port_file}") from error + if not 1 <= port <= 65535: + raise ValueError(f"invalid port in {port_file}") + return f"http://127.0.0.1:{port}" + return DEFAULT_GATEWAY_URL + + +def _resolve_gateway_token(token: str | None, resolved_run_dir: Path) -> str: + if token is not None: + return token + env_token = os.environ.get("CAPSEM_GATEWAY_TOKEN", "").strip() + if env_token: + return env_token + token_file = resolved_run_dir / "gateway.token" + if token_file.is_file(): + file_token = token_file.read_text(encoding="utf-8").strip() + if file_token: + return file_token + raise RuntimeError(f"gateway bearer token not found in CAPSEM_GATEWAY_TOKEN or {token_file}") + + +def discover_gateway( + *, url: str | None = None, token: str | None = None, run_dir: str | Path | None = None, +) -> GatewayEndpoint: + """Resolve the local gateway URL and bearer token from arguments, environment, or `run_dir`.""" + resolved_run_dir = capsem_run_dir(run_dir) + return GatewayEndpoint( + url=_resolve_gateway_url(url, resolved_run_dir), + token=_resolve_gateway_token(token, resolved_run_dir), + run_dir=resolved_run_dir, + ) diff --git a/sdk/python/capsem/errors.py b/sdk/python/capsem/errors.py new file mode 100644 index 000000000..c81000ffd --- /dev/null +++ b/sdk/python/capsem/errors.py @@ -0,0 +1,168 @@ +"""Structured exceptions and error mapping for the Capsem gateway SDK.""" + +from __future__ import annotations + +from collections.abc import Awaitable, Callable +from dataclasses import dataclass +from typing import TYPE_CHECKING, Self + +from . import models +from ._transport import CapsemError, HttpError, Transport +from .execution import CREATE_READY_SECS, EXEC_TIMEOUT_CEILING_SECS, ExecResult + +if TYPE_CHECKING: + from .vm import VM + +__all__ = [ + "CapsemError", "CapsemTimeoutError", "CreateTimeoutError", + "ExecTimeoutError", "HttpError", "VmNotFoundError", +] + + +class CapsemTimeoutError(TimeoutError, CapsemError): + """Base exception for Capsem SDK operation timeouts.""" + + status: int | None + body: str | None + vm_id: str | None + vm_name: str | None + vm: VM | None + command: str | None + timeout_secs: int | None + deadline_secs: float | None + + def __init__( + self, message: str, *, vm_id: str | None = None, vm_name: str | None = None, + vm: VM | None = None, command: str | None = None, timeout_secs: int | None = None, + deadline_secs: float | None = None, status: int | None = None, body: str | None = None, + ) -> None: + self.vm_id = vm_id + self.vm_name = vm_name + self.vm = vm + self.command = command + self.timeout_secs = timeout_secs + self.deadline_secs = deadline_secs + self.status = status + self.body = body + TimeoutError.__init__(self, message) + + +class CreateTimeoutError(CapsemTimeoutError): + """Raised when `Hypervisor.create` crosses the HTTP 504 or client deadline.""" + + +class ExecTimeoutError(CapsemTimeoutError): + """Raised when `VM.exec` or `Hypervisor.run` times out on the service or client.""" + + +class VmNotFoundError(HttpError, LookupError): + """Raised when a VM handle cannot be resolved by name or ID (HTTP 404).""" + + vm_id: str | None + vm_name: str | None + + def __init__(self, body: str, *, vm_id: str | None = None, vm_name: str | None = None, status: int = 404) -> None: + self.vm_id = vm_id + self.vm_name = vm_name + HttpError.__init__(self, status, body) + + def __reduce__(self) -> tuple[type[Self], tuple[object, ...], dict[str, object]]: + return (type(self), (self.body,), self.__dict__.copy()) + + +@dataclass(frozen=True) +class _ErrorContext: + vm_id: str | None = None + vm_name: str | None = None + bind_vm: Callable[[str | None], VM | None] | None = None + command: str | None = None + timeout_secs: int | None = None + deadline_secs: float | None = None + + +_ERROR_DISPATCH: dict[models.ErrorCode, Callable[[HttpError, _ErrorContext], CapsemError]] = { + models.ErrorCode.VM_NOT_FOUND: lambda error, ctx: VmNotFoundError( + error.body, + vm_id=error.response.vm_id if error.response and error.response.vm_id else ctx.vm_id, + vm_name=ctx.vm_name, status=error.status, + ), + models.ErrorCode.CREATE_TIMEOUT: lambda error, ctx: CreateTimeoutError( + f"HTTP {error.status}: {error.body}", + vm_id=error.response.vm_id if error.response else None, + vm_name=ctx.vm_name, + vm=ctx.bind_vm(error.response.vm_id if error.response else None) if ctx.bind_vm is not None else None, + deadline_secs=ctx.deadline_secs, status=error.status, body=error.body, + ), + models.ErrorCode.EXEC_TIMEOUT: lambda error, ctx: ExecTimeoutError( + f"HTTP {error.status}: {error.body}", + vm_id=ctx.vm_id, command=ctx.command, + timeout_secs=( + error.response.timeout_secs + if error.response and error.response.timeout_secs is not None + else ctx.timeout_secs + ), + status=error.status, body=error.body, + ), +} + + +def _map_http_error( + error: HttpError, ctx: _ErrorContext, *, fallback_code: models.ErrorCode | None = None, +) -> CapsemError | None: + code = error.code or fallback_code + if code is None: + return None + builder = _ERROR_DISPATCH.get(code) + return builder(error, ctx) if builder is not None else None + + +async def _create_vm_with_timeout( + op: Awaitable[models.ProvisionResponse], transport: Transport, *, + bind_vm: Callable[..., VM], name: str, container: bool, request_timeout: float, +) -> VM: + vm_name = name or None + + def _bind(vm_id: str | None) -> VM | None: + if vm_id is None and vm_name is None: + return None + return bind_vm(transport, id=vm_id, name=vm_name, container=container) + + ctx = _ErrorContext(vm_name=vm_name, bind_vm=_bind, deadline_secs=float(CREATE_READY_SECS)) + try: + response = await op + except HttpError as error: + fallback = models.ErrorCode.CREATE_TIMEOUT if error.status == 504 else None + mapped = _map_http_error(error, ctx, fallback_code=fallback) + if mapped is not None: + raise mapped from error + raise + except TimeoutError as error: + raise CreateTimeoutError( + str(error) or "create timed out", + vm_id=None, vm_name=vm_name, vm=_bind(None), deadline_secs=float(request_timeout), + ) from error + return bind_vm(transport, id=response.id, name=response.name, container=container) + + +async def _exec_with_timeout( + op: Awaitable[models.ExecResponse], *, command: str, timeout_secs: int | None, + vm_id: str | None = None, vm_name: str | None = None, +) -> ExecResult: + effective_secs = ( + EXEC_TIMEOUT_CEILING_SECS if timeout_secs is None else min(timeout_secs, EXEC_TIMEOUT_CEILING_SECS) + ) + ctx = _ErrorContext(vm_id=vm_id, vm_name=vm_name, command=command, timeout_secs=effective_secs) + try: + response = await op + except HttpError as error: + fallback = models.ErrorCode.EXEC_TIMEOUT if error.status in (408, 504) else None + mapped = _map_http_error(error, ctx, fallback_code=fallback) + if mapped is not None: + raise mapped from error + raise + except TimeoutError as error: + raise ExecTimeoutError( + str(error) or "command timed out", + vm_id=vm_id, command=command, timeout_secs=effective_secs, + ) from error + return ExecResult.from_wire(response) diff --git a/sdk/python/capsem/execution.py b/sdk/python/capsem/execution.py index cd2ddc5e6..492ea6bd4 100644 --- a/sdk/python/capsem/execution.py +++ b/sdk/python/capsem/execution.py @@ -13,6 +13,8 @@ #: How long the service waits for a container workload to become ready #: before it answers a create; the image pull happens inside this window. CREATE_READY_SECS = 110 +#: Largest request body and workspace file the HTTP API accepts, in bytes. +MAX_REQUEST_BODY_BYTES = 10 * 1024 * 1024 def command_deadline(default: float, timeout_secs: int | None) -> float: diff --git a/sdk/python/capsem/hypervisor.py b/sdk/python/capsem/hypervisor.py index 7b83ef711..822ce6dd7 100644 --- a/sdk/python/capsem/hypervisor.py +++ b/sdk/python/capsem/hypervisor.py @@ -3,6 +3,8 @@ from __future__ import annotations from collections.abc import Sequence +from pathlib import Path +from typing import Self from . import _operations as api from . import models @@ -12,6 +14,7 @@ from ._images import Images from ._mcp import Mcp from ._networks import Networks +from .errors import _create_vm_with_timeout, _exec_with_timeout from .execution import ( CREATE_READY_SECS, GATEWAY_REQUEST_BUDGET_SECS, @@ -21,6 +24,8 @@ from .registry import Registry from .vm import VM +__all__ = ["Hypervisor"] + def _memory_mb(memory: int | None) -> int | None: if memory is None: @@ -31,14 +36,21 @@ def _memory_mb(memory: int | None) -> int | None: class Hypervisor(Client): - def __init__(self, url: str, token: str, *, timeout: float = 30) -> None: - super().__init__(url, token, timeout=timeout) + def __init__(self, url: str | None = None, token: str | None = None, *, + run_dir: str | Path | None = None, timeout: float = 30) -> None: + super().__init__(url, token, run_dir=run_dir, timeout=timeout) self.networks = Networks(self._transport) self.mcp = Mcp(self._transport) self.debug = Debug(self._transport) self.images = Images(self._transport) self.credentials = Credentials(self._transport) + @classmethod + def connect(cls, url: str | None = None, token: str | None = None, *, + run_dir: str | Path | None = None, timeout: float = 30) -> Self: + """Create a `Hypervisor` client using gateway discovery when `url` or `token` is omitted.""" + return cls(url, token, run_dir=run_dir, timeout=timeout) + async def info(self) -> models.HypervisorInfo: return await api.get_hypervisor_info(self._transport) @@ -99,11 +111,12 @@ async def create(self, *, name: str = "", cpus: int | None = None, request.container = wire # The service answers only once a container workload is ready, so # wait at least as long as it does before giving up on the create. - response = await api.create_vm( - self._transport, body=request, - request_timeout=max(self._transport.timeout, CREATE_READY_SECS + GATEWAY_REQUEST_BUDGET_SECS), + request_timeout = max(self._transport.timeout, CREATE_READY_SECS + GATEWAY_REQUEST_BUDGET_SECS) + return await _create_vm_with_timeout( + api.create_vm(self._transport, body=request, request_timeout=request_timeout), + self._transport, bind_vm=VM._bind, name=name, + container=image is not None, request_timeout=request_timeout, ) - return VM._bind(self._transport, id=response.id, name=response.name, container=image is not None) async def log(self, source: models.HostLogSource = models.HostLogSource.SERVICE, *, grep: str | None = None, tail: int | None = None, @@ -113,11 +126,13 @@ async def log(self, source: models.HostLogSource = models.HostLogSource.SERVICE, async def run(self, command: str, *, timeout_secs: int | None = None, cpus: int | None = None, memory: int | None = None, env: dict[str, str] | None = None) -> ExecResult: - response = await api.run_vm(self._transport, body=models.RunRequest( + return await _exec_with_timeout( + api.run_vm(self._transport, body=models.RunRequest( + command=command, timeout_secs=timeout_secs, + cpus=cpus, ram_mb=_memory_mb(memory), env=env, + ), request_timeout=command_deadline(self._transport.timeout, timeout_secs)), command=command, timeout_secs=timeout_secs, - cpus=cpus, ram_mb=_memory_mb(memory), env=env, - ), request_timeout=command_deadline(self._transport.timeout, timeout_secs)) - return ExecResult.from_wire(response) + ) async def purge(self, *, all: bool = False) -> models.PurgeResponse: return await api.purge_vms(self._transport, body=models.PurgeRequest(all=all)) diff --git a/sdk/python/capsem/models/__init__.py b/sdk/python/capsem/models/__init__.py index ef9dc05ff..8802e6bfd 100644 --- a/sdk/python/capsem/models/__init__.py +++ b/sdk/python/capsem/models/__init__.py @@ -34,6 +34,7 @@ from .credential_outcome import CredentialOutcome as CredentialOutcome from .credential_storage import CredentialStorage as CredentialStorage from .dns_event import DnsEvent as DnsEvent +from .error_code import ErrorCode as ErrorCode from .error_event import ErrorEvent as ErrorEvent from .error_response import ErrorResponse as ErrorResponse from .event_bodies_response import EventBodiesResponse as EventBodiesResponse diff --git a/sdk/python/capsem/models/error_code.py b/sdk/python/capsem/models/error_code.py new file mode 100644 index 000000000..19358ef9d --- /dev/null +++ b/sdk/python/capsem/models/error_code.py @@ -0,0 +1,11 @@ +"""Generated from Capsem OpenAPI. Do not edit.""" + +from __future__ import annotations + +from enum import StrEnum + + +class ErrorCode(StrEnum): + VM_NOT_FOUND = 'vm_not_found' + CREATE_TIMEOUT = 'create_timeout' + EXEC_TIMEOUT = 'exec_timeout' diff --git a/sdk/python/capsem/models/error_response.py b/sdk/python/capsem/models/error_response.py index 5fe3fa30e..3d9f5130c 100644 --- a/sdk/python/capsem/models/error_response.py +++ b/sdk/python/capsem/models/error_response.py @@ -2,10 +2,16 @@ from __future__ import annotations -from pydantic import StrictStr +from typing import Annotated +from pydantic import Field, StrictInt, StrictStr + +from .error_code import ErrorCode from .model_base import Model class ErrorResponse(Model): + code: ErrorCode | None = None error: StrictStr + timeout_secs: Annotated[StrictInt, Field(ge=0)] | None = None + vm_id: StrictStr | None = None diff --git a/sdk/python/capsem/vm.py b/sdk/python/capsem/vm.py index cb7b12766..297e02c28 100644 --- a/sdk/python/capsem/vm.py +++ b/sdk/python/capsem/vm.py @@ -2,7 +2,9 @@ from __future__ import annotations -from collections.abc import Sequence +from collections.abc import Awaitable, Sequence +from pathlib import Path +from typing import TypeVar from pydantic import StrictStr, TypeAdapter @@ -14,13 +16,17 @@ from ._ports import Ports from ._resources import Files, Stats from ._transport import HttpError, Transport +from .errors import VmNotFoundError, _ErrorContext, _exec_with_timeout, _map_http_error from .execution import CREATE_READY_SECS, ExecResult, command_deadline +_T = TypeVar("_T") + class VM(Client): - def __init__(self, url: str, token: str, *, name: str | None = None, - id: str | None = None, timeout: float = 30) -> None: - super().__init__(url, token, timeout=timeout) + def __init__(self, url: str | None = None, token: str | None = None, *, + name: str | None = None, id: str | None = None, + run_dir: str | Path | None = None, timeout: float = 30) -> None: + super().__init__(url, token, run_dir=run_dir, timeout=timeout) self._select(name=name, id=id) def _select(self, *, name: str | None, id: str | None) -> None: @@ -38,10 +44,16 @@ def _select(self, *, name: str | None, id: str | None) -> None: self.ports = Ports(self) @classmethod - def _bind(cls, transport: Transport, *, id: str, name: str | None = None, + def _bind(cls, transport: Transport, *, id: str | None = None, name: str | None = None, container: bool | None = None) -> VM: + """Bind a handle to `transport` by `id`, or lazily by `name` when `id` is unknown. + + A handle bound with `id=None` resolves `name` via `GET /vms/list` on its + first operation; if the name is reused before then, resolution binds to + whichever sandbox currently holds `name`. + """ vm = cls._from_transport(transport) - vm._select(name=None, id=id) + vm._select(name=None if id else name, id=id) vm._name = name vm._has_container = container return vm @@ -59,11 +71,22 @@ async def _resolve(self) -> str: if self._id is None: response = await api.list_vms(self._transport) matches = [vm for vm in response.sandboxes if vm.name == self._name] + if not matches: + raise VmNotFoundError(f"expected one VM named {self._name!r}, found 0", vm_name=self._name) if len(matches) != 1: raise LookupError(f"expected one VM named {self._name!r}, found {len(matches)}") self._id = matches[0].id return self._id + async def _call(self, vm_id: str, op: Awaitable[_T]) -> _T: + try: + return await op + except HttpError as error: + mapped = _map_http_error(error, _ErrorContext(vm_id=vm_id, vm_name=self._name)) + if mapped is not None: + raise mapped from error + raise + async def _port_target(self) -> models.ExposureTarget: if self._has_container is None: try: @@ -77,7 +100,8 @@ async def _port_target(self) -> models.ExposureTarget: return models.ExposureTarget.CONTAINER if self._has_container else models.ExposureTarget.VM async def info(self) -> models.SandboxInfo: - return await api.get_vm_info(self._transport, id=await self._resolve()) + vm_id = await self._resolve() + return await self._call(vm_id, api.get_vm_info(self._transport, id=vm_id)) async def exec(self, command: str, *, timeout_secs: int | None = None, target: models.ExecTarget | None = None) -> ExecResult: @@ -85,38 +109,53 @@ async def exec(self, command: str, *, timeout_secs: int | None = None, "command": command, "timeout_secs": timeout_secs, **({"target": target} if target is not None else {}), }) - response = await api.exec_vm( - self._transport, id=await self._resolve(), - body=body, - request_timeout=command_deadline(self._transport.timeout, timeout_secs), + vm_id = await self._resolve() + return await _exec_with_timeout( + api.exec_vm( + self._transport, id=vm_id, + body=body, + request_timeout=command_deadline(self._transport.timeout, timeout_secs), + ), + command=command, timeout_secs=timeout_secs, vm_id=vm_id, vm_name=self._name, ) - return ExecResult.from_wire(response) async def start(self) -> models.ProvisionResponse: - return await api.start_vm( - self._transport, id=await self._resolve(), - request_timeout=command_deadline(self._transport.timeout, CREATE_READY_SECS), + vm_id = await self._resolve() + return await self._call( + vm_id, + api.start_vm( + self._transport, id=vm_id, + request_timeout=command_deadline(self._transport.timeout, CREATE_READY_SECS), + ), ) async def persist(self, name: str) -> models.PersistResponse: - return await api.persist_vm( - self._transport, id=await self._resolve(), body=models.PersistRequest(name=name), - ) + vm_id = await self._resolve() + return await self._call(vm_id, api.persist_vm( + self._transport, id=vm_id, body=models.PersistRequest(name=name), + )) async def stop(self) -> models.StopResponse: - return await api.stop_vm(self._transport, id=await self._resolve()) + vm_id = await self._resolve() + return await self._call(vm_id, api.stop_vm(self._transport, id=vm_id)) async def pause(self) -> models.VmActionResponse: - return await api.pause_vm(self._transport, id=await self._resolve()) + vm_id = await self._resolve() + return await self._call(vm_id, api.pause_vm(self._transport, id=vm_id)) async def resume(self) -> models.ProvisionResponse: - return await api.resume_vm( - self._transport, id=await self._resolve(), - request_timeout=command_deadline(self._transport.timeout, CREATE_READY_SECS), + vm_id = await self._resolve() + return await self._call( + vm_id, + api.resume_vm( + self._transport, id=vm_id, + request_timeout=command_deadline(self._transport.timeout, CREATE_READY_SECS), + ), ) async def delete(self) -> models.VmActionResponse: - return await api.delete_vm(self._transport, id=await self._resolve()) + vm_id = await self._resolve() + return await self._call(vm_id, api.delete_vm(self._transport, id=vm_id)) async def fork( self, @@ -132,21 +171,35 @@ async def fork( description: Optional description for the forked VM. labels: Optional labels overriding the source VM's labels (`{}` clears). """ - response = await api.fork_vm(self._transport, id=await self._resolve(), body=models.ForkRequest( + vm_id = await self._resolve() + response = await self._call(vm_id, api.fork_vm(self._transport, id=vm_id, body=models.ForkRequest( name=name, description=description, labels=labels, - )) + ))) return VM._bind( self._transport, id=response.id, name=response.name, container=self._has_container, ) async def log(self, *, grep: str | None = None, tail: int | None = None, max_bytes: int | None = None) -> models.LogsResponse: - return await api.get_vm_logs(self._transport, id=await self._resolve(), grep=grep, tail=tail, max_bytes=max_bytes) + vm_id = await self._resolve() + return await self._call( + vm_id, api.get_vm_logs(self._transport, id=vm_id, grep=grep, tail=tail, max_bytes=max_bytes), + ) async def history(self, *, limit: int | None = None, offset: int | None = None, search: str | None = None, layer: models.HistoryLayerFilter | None = None) -> models.HistoryResponse: - return await api.get_vm_history(self._transport, id=await self._resolve(), limit=limit, offset=offset, search=search, layer=layer) + vm_id = await self._resolve() + return await self._call( + vm_id, api.get_vm_history(self._transport, id=vm_id, limit=limit, offset=offset, search=search, layer=layer), + ) async def timeline(self, *, trace_id: str | None = None, since: str | None = None, limit: int | None = None, layers: Sequence[models.TimelineLayer] | None = None) -> models.TimelineResponse: - return await api.get_vm_timeline(self._transport, id=await self._resolve(), trace_id=trace_id, since=since, limit=limit, layers=list(layers) if layers is not None else None) + vm_id = await self._resolve() + return await self._call( + vm_id, + api.get_vm_timeline( + self._transport, id=vm_id, trace_id=trace_id, since=since, limit=limit, + layers=list(layers) if layers is not None else None, + ), + ) diff --git a/sdk/python/tests/facade_gateway.py b/sdk/python/tests/facade_gateway.py index 08ef66c8b..c63788a3d 100644 --- a/sdk/python/tests/facade_gateway.py +++ b/sdk/python/tests/facade_gateway.py @@ -27,6 +27,8 @@ class GatewayState: container_states: list[str] = field(default_factory=lambda: ["running"]) preview_session_status: int | None = None delays: dict[str, float] = field(default_factory=dict) + create_error: tuple[int, str] | None = None + sandboxes: dict[str, str] = field(default_factory=dict) created_vms: dict[str, tuple[str, bool]] = field(default_factory=dict) sandbox_labels: dict[str, dict[str, str] | None] = field(default_factory=dict) @@ -48,22 +50,29 @@ async def handle(request: web.Request) -> web.Response: if request.path in state.delays: await asyncio.sleep(state.delays[request.path]) if request.path == "/vms/list": - return web.json_response( - { - "sandboxes": [ - response_model( - "SandboxInfo", - id=f"vm-{index}", - name=name, - labels=state.sandbox_labels.get(f"vm-{index}"), - ) - for index, name in enumerate(state.names) - ] - } - ) + listed = [ + response_model("SandboxInfo", id=vm_id, name=name, labels=state.sandbox_labels.get(vm_id)) + for vm_id, name in state.sandboxes.items() + ] + [ + response_model( + "SandboxInfo", + id=f"vm-{index}", + name=name, + labels=state.sandbox_labels.get(f"vm-{index}"), + ) + for index, name in enumerate(state.names) + ] + return web.json_response({"sandboxes": listed}) if request.path == "/vms/create": payload = json.loads(body) vm_name = payload["name"] or "temporary" + if state.create_error is not None: + status, message = state.create_error + if status == 504: + err_doc = json.loads(message) + if err_doc.get("code") == "create_timeout" and isinstance(err_doc.get("vm_id"), str): + state.sandboxes[err_doc["vm_id"]] = vm_name + return web.Response(status=status, text=message, content_type="application/json") persistent = bool(payload.get("persistent", bool(payload.get("name") or payload.get("from")))) state.created_vms["created-id"] = (vm_name, persistent) state.sandbox_labels["created-id"] = payload.get("labels") or None @@ -75,6 +84,13 @@ async def handle(request: web.Request) -> web.Response: persistent=persistent, ) ) + if request.method == "DELETE" and request.path.endswith("/delete"): + vm_id = request.path.removeprefix("/vms/").removesuffix("/delete") + state.sandboxes.pop(vm_id, None) + if vm_id.startswith("vm-") and vm_id[3:].isdigit(): + index = int(vm_id[3:]) + if 0 <= index < len(state.names): + state.names.pop(index) if request.path.endswith("/info") and request.path.split("/")[2] in state.created_vms: vm_id = request.path.split("/")[2] vm_name, persistent = state.created_vms[vm_id] diff --git a/sdk/python/tests/test_contract.py b/sdk/python/tests/test_contract.py index 4a39d9b3c..45b9c004f 100644 --- a/sdk/python/tests/test_contract.py +++ b/sdk/python/tests/test_contract.py @@ -12,6 +12,12 @@ import pytest from capsem import _operations, models from capsem._transport import Transport +from capsem.execution import ( + CREATE_READY_SECS, + EXEC_TIMEOUT_CEILING_SECS, + GATEWAY_REQUEST_BUDGET_SECS, + MAX_REQUEST_BODY_BYTES, +) from pydantic import TypeAdapter, ValidationError from .test_transport import gateway @@ -21,6 +27,13 @@ OPERATIONS = [operation for methods in SPEC["paths"].values() for operation in methods.values()] +def test_execution_mirrors_gateway_and_service_limits() -> None: + assert EXEC_TIMEOUT_CEILING_SECS == 60 * 60 + assert GATEWAY_REQUEST_BUDGET_SECS == 120 + assert CREATE_READY_SECS == 110 + assert MAX_REQUEST_BODY_BYTES == 10 * 1024 * 1024 + + def sample(schema: dict[str, Any]) -> object: if not (schema.keys() - {"description"}): return {"nested": [True, None, {"number": 3}]} diff --git a/sdk/python/tests/test_create.py b/sdk/python/tests/test_create.py new file mode 100644 index 000000000..1ea0e8b90 --- /dev/null +++ b/sdk/python/tests/test_create.py @@ -0,0 +1,85 @@ +"""Structured VM handle recovery on HTTP 504 and client-side create timeouts.""" + +from __future__ import annotations + +import asyncio +import json +from typing import Any + +import pytest +from capsem import CreateTimeoutError, HttpError, Hypervisor, models + +from .facade_gateway import gateway + + +def test_create_504_surfaces_vm_id_and_deletes_in_flight_sandbox() -> None: + async def run() -> None: + async with gateway() as (url, state), Hypervisor(url, "token") as hv: + state.names = [] + body = {"error": "workload for VM vm-504 did not become ready", "code": "create_timeout", "vm_id": "vm-504"} + state.create_error = (504, json.dumps(body)) + with pytest.raises(CreateTimeoutError) as caught: + await hv.create(name="box", image="docker://busybox:latest") + err = caught.value + assert isinstance(err, TimeoutError) + assert (err.status, err.vm_id, err.vm_name) == (504, "vm-504", "box") + assert err.vm is not None and (err.vm.id, err.vm.name) == ("vm-504", "box") + assert [vm.id for vm in (await hv.list()).sandboxes] == ["vm-504"] + assert (await err.vm.container.status()).state is models.ContainerState.RUNNING + await err.vm.delete() + assert (await hv.list()).sandboxes == [] + + asyncio.run(run()) + + +def test_create_504_without_structured_vm_id_late_binds_by_name() -> None: + async def run() -> None: + async with gateway() as (url, state), Hypervisor(url, "token") as hv: + state.names = ["named"] + state.create_error = (504, json.dumps({"error": "upstream proxy timed out"})) + with pytest.raises(CreateTimeoutError) as named_err: + await hv.create(name="named") + assert (named_err.value.vm_id, named_err.value.vm_name) == (None, "named") + assert named_err.value.vm is not None and named_err.value.vm.id is None + await named_err.value.vm.delete() + assert named_err.value.vm.id == "vm-0" + + with pytest.raises(CreateTimeoutError) as unnamed_err: + await hv.create() + assert (unnamed_err.value.vm_id, unnamed_err.value.vm_name, unnamed_err.value.vm) == (None, None, None) + + state.create_error = (409, json.dumps({"error": 'persistent VM "named" exists'})) + with pytest.raises(HttpError) as conflict_err: + await hv.create(name="named") + assert not isinstance(conflict_err.value, CreateTimeoutError) + assert conflict_err.value.status == 409 + + asyncio.run(run()) + + +def test_create_client_timeout_late_binds_named_vm(monkeypatch: pytest.MonkeyPatch) -> None: + async def run() -> None: + async with gateway() as (url, state), Hypervisor(url, "token") as hv: + request = hv._transport.request + + async def timeout_on_create(method: Any, path: str, **kwargs: Any) -> bytes: + if path == "/vms/create": + raise TimeoutError("request timed out") + return await request(method, path, **kwargs) + + monkeypatch.setattr(hv._transport, "request", timeout_on_create) + state.names = [] + with pytest.raises(CreateTimeoutError) as late_err: + await hv.create(name="late") + assert not isinstance(late_err.value, HttpError) + assert (late_err.value.status, late_err.value.vm_id, late_err.value.vm_name) == (None, None, "late") + assert late_err.value.vm is not None and late_err.value.vm.id is None + state.names = ["late"] + await late_err.value.vm.delete() + assert late_err.value.vm.id == "vm-0" and (await hv.list()).sandboxes == [] + + with pytest.raises(CreateTimeoutError) as unnamed_err: + await hv.create() + assert unnamed_err.value.vm is None + + asyncio.run(run()) diff --git a/sdk/python/tests/test_helpers.py b/sdk/python/tests/test_helpers.py new file mode 100644 index 000000000..8dad07646 --- /dev/null +++ b/sdk/python/tests/test_helpers.py @@ -0,0 +1,226 @@ +"""Tests for gateway discovery and structured SDK errors.""" + +from __future__ import annotations + +import asyncio +import json +import pickle +from pathlib import Path +from typing import Any + +import pytest +from capsem import ( + DEFAULT_GATEWAY_URL, + VM, + CapsemError, + CapsemTimeoutError, + CreateTimeoutError, + ExecTimeoutError, + GatewayEndpoint, + HttpError, + Hypervisor, + VmNotFoundError, + capsem_run_dir, + discover_gateway, + models, +) + +from .facade_gateway import gateway + + +def test_capsem_run_dir_priority(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + monkeypatch.delenv("CAPSEM_RUN_DIR", raising=False) + monkeypatch.delenv("CAPSEM_HOME", raising=False) + monkeypatch.setenv("HOME", str(tmp_path)) + assert capsem_run_dir() == tmp_path / ".capsem" / "run" + monkeypatch.setenv("CAPSEM_HOME", str(tmp_path / "custom-home")) + assert capsem_run_dir() == tmp_path / "custom-home" / "run" + monkeypatch.setenv("CAPSEM_RUN_DIR", str(tmp_path / "explicit-env-run")) + assert capsem_run_dir() == tmp_path / "explicit-env-run" + assert capsem_run_dir(tmp_path / "arg-run") == tmp_path / "arg-run" + with pytest.raises(ValueError, match="run_dir"): + capsem_run_dir(" ") + + +def test_discover_gateway_from_files_and_env(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + for var in ("CAPSEM_GATEWAY_URL", "CAPSEM_GATEWAY_TOKEN", "CAPSEM_RUN_DIR", "CAPSEM_HOME"): + monkeypatch.delenv(var, raising=False) + with pytest.raises(RuntimeError, match="gateway bearer token not found"): + discover_gateway(run_dir=tmp_path) + (tmp_path / "gateway.token").write_text("file-token\n", encoding="utf-8") + assert discover_gateway(run_dir=tmp_path) == GatewayEndpoint( + url=DEFAULT_GATEWAY_URL, token="file-token", run_dir=tmp_path, + ) + (tmp_path / "gateway.port").write_text(" 19444\n", encoding="utf-8") + assert discover_gateway(run_dir=tmp_path).url == "http://127.0.0.1:19444" + for bad_port in ("not-a-port", "0", "70000"): + (tmp_path / "gateway.port").write_text(f"{bad_port}\n", encoding="utf-8") + with pytest.raises(ValueError, match="invalid port"): + discover_gateway(run_dir=tmp_path) + monkeypatch.setenv("CAPSEM_GATEWAY_URL", "http://127.0.0.1:19666/") + monkeypatch.setenv("CAPSEM_GATEWAY_TOKEN", "env-token") + ep_env = discover_gateway(run_dir=tmp_path) + assert (ep_env.url, ep_env.token) == ("http://127.0.0.1:19666", "env-token") + ep_explicit = discover_gateway(url="http://127.0.0.1:19777", token="arg-token", run_dir=tmp_path) + assert (ep_explicit.url, ep_explicit.token) == ("http://127.0.0.1:19777", "arg-token") + + +def test_hypervisor_connect_and_vm_use_discovered_gateway( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path, +) -> None: + async def run() -> None: + async with gateway() as (url, _): + port = url.rsplit(":", 1)[1] + monkeypatch.delenv("CAPSEM_GATEWAY_URL", raising=False) + monkeypatch.delenv("CAPSEM_GATEWAY_TOKEN", raising=False) + (tmp_path / "gateway.port").write_text(f"{port}\n", encoding="utf-8") + (tmp_path / "gateway.token").write_text("token\n", encoding="utf-8") + async with Hypervisor.connect(run_dir=tmp_path) as hv, VM(run_dir=tmp_path, id="vm-0") as vm: + assert isinstance(await hv.info(), models.HypervisorInfo) + assert isinstance(await vm.info(), models.SandboxInfo) + + asyncio.run(run()) + + +def test_vm_not_found_error_on_name_resolution_and_404_routes(monkeypatch: pytest.MonkeyPatch) -> None: + async def run() -> None: + async with gateway() as (url, state), Hypervisor(url, "token") as hv: + state.names = [] + with pytest.raises(VmNotFoundError) as by_name_err: + await hv.vm(name="gone").info() + err = by_name_err.value + assert isinstance(err, (HttpError, LookupError, CapsemError)) + assert (err.status, err.vm_name, err.vm_id) == (404, "gone", None) + + state.names = ["dup", "dup"] + with pytest.raises(LookupError, match="found 2") as dup_err: + await hv.vm(name="dup").info() + assert not isinstance(dup_err.value, VmNotFoundError) + + vm = hv.vm(id="missing-id") + with pytest.raises(HttpError) as file_err: + await vm.files.read("/missing") + assert not isinstance(file_err.value, VmNotFoundError) + + request = hv._transport.request + + async def non_vm_404(method: Any, path: str, **kwargs: Any) -> bytes: + if path.endswith(("/logs", "/exec")): + raise HttpError(404, json.dumps({"error": "no app log found"})) + return await request(method, path, **kwargs) + + monkeypatch.setattr(hv._transport, "request", non_vm_404) + for non_vm_call in (lambda: vm.log(), lambda: vm.exec("true")): + with pytest.raises(HttpError) as plain_404: + await non_vm_call() + assert not isinstance(plain_404.value, VmNotFoundError) + + async def sandbox_404(method: Any, path: str, **kwargs: Any) -> bytes: + params = kwargs.get("path_parameters") or {} + if params.get("id") == "missing-id": + body = {"error": "not found", "code": "vm_not_found", "vm_id": "missing-id"} + raise HttpError(404, json.dumps(body)) + return await request(method, path, **kwargs) + + monkeypatch.setattr(hv._transport, "request", sandbox_404) + calls = ( + lambda: vm.info(), lambda: vm.exec("true"), lambda: vm.delete(), + lambda: vm.fork("child"), lambda: vm.log(), lambda: vm.stats.summary(), + lambda: vm.files.read("/work/a"), lambda: vm.files.write("/work/a", b"x"), lambda: vm.files.list(), + ) + for call in calls: + with pytest.raises(VmNotFoundError) as route_err: + await call() + assert (route_err.value.vm_id, route_err.value.status) == ("missing-id", 404) + assert route_err.value.code is models.ErrorCode.VM_NOT_FOUND + + asyncio.run(run()) + + +def test_exec_timeout_error_on_http_and_client_deadlines(monkeypatch: pytest.MonkeyPatch) -> None: + async def run() -> None: + async with gateway() as (url, _), Hypervisor(url, "token") as hv: + vm = hv.vm(id="vm-0") + + async def ipc_timeout(method: Any, path: str, **_kwargs: Any) -> bytes: + body = {"error": "timed out", "code": "exec_timeout", "timeout_secs": 17} + raise HttpError(504, json.dumps(body)) + + monkeypatch.setattr(hv._transport, "request", ipc_timeout) + with pytest.raises(ExecTimeoutError) as exec_504_coded: + await vm.exec("sleep 20", timeout_secs=20) + assert isinstance(exec_504_coded.value, (TimeoutError, CapsemTimeoutError)) + assert (exec_504_coded.value.status, exec_504_coded.value.vm_id) == (504, "vm-0") + assert (exec_504_coded.value.command, exec_504_coded.value.timeout_secs) == ("sleep 20", 17) + + with pytest.raises(ExecTimeoutError) as run_504_coded: + await hv.run("sleep 20", timeout_secs=20) + assert (run_504_coded.value.status, run_504_coded.value.timeout_secs) == (504, 17) + + async def gateway_504(method: Any, path: str, **_kwargs: Any) -> bytes: + raise HttpError(504, "gateway timeout") + + monkeypatch.setattr(hv._transport, "request", gateway_504) + with pytest.raises(ExecTimeoutError) as exec_504: + await vm.exec("sleep 9", timeout_secs=9) + assert (exec_504.value.status, exec_504.value.timeout_secs) == (504, 9) + + async def non_timeout_500(method: Any, path: str, **_kwargs: Any) -> bytes: + raise HttpError(500, json.dumps({"error": "IPC command failed without code"})) + + monkeypatch.setattr(hv._transport, "request", non_timeout_500) + for fn in (lambda: vm.exec("broken"), lambda: hv.run("broken")): + with pytest.raises(HttpError) as plain_500: + await fn() + assert not isinstance(plain_500.value, ExecTimeoutError) + + async def client_timeout(method: Any, path: str, **_kwargs: Any) -> bytes: + raise TimeoutError("client deadline exceeded") + + monkeypatch.setattr(hv._transport, "request", client_timeout) + with pytest.raises(ExecTimeoutError) as exec_client: + await vm.exec("sleep 11", timeout_secs=11) + assert not isinstance(exec_client.value, HttpError) + assert (exec_client.value.status, exec_client.value.timeout_secs) == (None, 11) + + with pytest.raises(ExecTimeoutError) as run_client: + await hv.run("sleep 12", timeout_secs=12) + assert (run_client.value.status, run_client.value.timeout_secs) == (None, 12) + + asyncio.run(run()) + + +def test_exception_pickle_roundtrip_and_parsed_error_code() -> None: + raw_body = json.dumps({"error": "not found", "code": "vm_not_found", "vm_id": "vm-9"}) + http_err = HttpError(404, raw_body) + assert http_err.code is models.ErrorCode.VM_NOT_FOUND + assert http_err.response is not None and http_err.response.vm_id == "vm-9" + for _ in range(2): + http_err = pickle.loads(pickle.dumps(http_err)) + assert (http_err.status, http_err.body, http_err.code) == (404, raw_body, models.ErrorCode.VM_NOT_FOUND) + assert str(http_err) == f"HTTP 404: {raw_body}" + + vm_err = VmNotFoundError(raw_body, vm_id="vm-9", vm_name="box", status=404) + for _ in range(2): + vm_err = pickle.loads(pickle.dumps(vm_err)) + assert (vm_err.status, vm_err.body, vm_err.vm_id, vm_err.vm_name) == (404, raw_body, "vm-9", "box") + assert str(vm_err) == f"HTTP 404: {raw_body}" + + create_err = CreateTimeoutError( + "HTTP 504: timeout", vm_id="vm-1", vm_name="box", deadline_secs=180.0, status=504, body="timeout", + ) + create_rt = pickle.loads(pickle.dumps(create_err)) + assert isinstance(create_rt, CapsemTimeoutError) + assert (create_rt.vm_id, create_rt.vm_name, create_rt.deadline_secs, create_rt.status) == ( + "vm-1", "box", 180.0, 504, + ) + assert str(create_rt) == "HTTP 504: timeout" + + exec_err = ExecTimeoutError( + "HTTP 504: exec timed out", vm_id="vm-2", command="sleep 30", + timeout_secs=30, status=504, body="exec timed out", + ) + exec_rt = pickle.loads(pickle.dumps(exec_err)) + assert isinstance(exec_rt, CapsemTimeoutError) + assert (exec_rt.vm_id, exec_rt.command, exec_rt.timeout_secs, exec_rt.status) == ("vm-2", "sleep 30", 30, 504) + assert str(exec_rt) == "HTTP 504: exec timed out" diff --git a/sdk/rust/src/operations/tests.rs b/sdk/rust/src/operations/tests.rs index 8f0a1ab66..80e81aa52 100644 --- a/sdk/rust/src/operations/tests.rs +++ b/sdk/rust/src/operations/tests.rs @@ -82,6 +82,7 @@ operation!(get_vm_status, "getVmStatus", GetVmStatusParams); operation!(get_vm_container, "getVmContainer", GetVmContainerParams); operation!(list_images, "listImages", ListImagesParams); operation!(pull_image, "pullImage", PullImageParams); +operation!(inject_credential, "injectCredential", InjectCredentialParams); operation!(list_vm_exposures, "listVmExposures", ListVmExposuresParams); operation!(create_vm_exposure, "createVmExposure", CreateVmExposureParams); operation!(delete_vm_exposure, "deleteVmExposure", DeleteVmExposureParams); diff --git a/sdk/specification/openapi.json b/sdk/specification/openapi.json index 69e670e5f..617aac583 100644 --- a/sdk/specification/openapi.json +++ b/sdk/specification/openapi.json @@ -3579,6 +3579,14 @@ } } }, + "ErrorCode": { + "type": "string", + "enum": [ + "vm_not_found", + "create_timeout", + "exec_timeout" + ] + }, "ErrorEvent": { "type": "object", "required": [ @@ -3614,8 +3622,32 @@ "error" ], "properties": { + "code": { + "oneOf": [ + { + "type": "null" + }, + { + "$ref": "#/components/schemas/ErrorCode" + } + ] + }, "error": { "type": "string" + }, + "timeout_secs": { + "type": [ + "integer", + "null" + ], + "format": "int64", + "minimum": 0 + }, + "vm_id": { + "type": [ + "string", + "null" + ] } } }, diff --git a/sdk/typescript/src/execution.ts b/sdk/typescript/src/execution.ts index d4b6de905..39ca1bd96 100644 --- a/sdk/typescript/src/execution.ts +++ b/sdk/typescript/src/execution.ts @@ -7,11 +7,16 @@ export function decodeExecOutput(output: ExecOutput): Uint8Array { } /** The service's ceiling for one exec or run, which is also its default. */ -const EXEC_TIMEOUT_CEILING_SECS = 60 * 60; +export const EXEC_TIMEOUT_CEILING_SECS = 60 * 60; /** The gateway's budget for readiness, boot and teardown around a command. */ -const GATEWAY_REQUEST_BUDGET_SECS = 120; -/** The service's workload readiness window, including its image pull. */ -const CREATE_READY_SECS = 110; +export const GATEWAY_REQUEST_BUDGET_SECS = 120; +/** + * How long the service waits for a container workload to become ready + * before it answers a create; the image pull happens inside this window. + */ +export const CREATE_READY_SECS = 110; +/** Largest request body and workspace file the HTTP API accepts, in bytes. */ +export const MAX_REQUEST_BODY_BYTES = 10 * 1024 * 1024; /** Creation answers only after workload readiness, so allow its full window. */ export function createDeadlineMs(fallbackMs: number): number { diff --git a/sdk/typescript/src/models/ErrorCode.ts b/sdk/typescript/src/models/ErrorCode.ts new file mode 100644 index 000000000..c26d85749 --- /dev/null +++ b/sdk/typescript/src/models/ErrorCode.ts @@ -0,0 +1,9 @@ +// Generated from Capsem OpenAPI. Do not edit. + + + +export enum ErrorCode { + VM_NOT_FOUND = "vm_not_found", + CREATE_TIMEOUT = "create_timeout", + EXEC_TIMEOUT = "exec_timeout", +} diff --git a/sdk/typescript/src/models/ErrorResponse.ts b/sdk/typescript/src/models/ErrorResponse.ts index 7efbc7834..3ea89f213 100644 --- a/sdk/typescript/src/models/ErrorResponse.ts +++ b/sdk/typescript/src/models/ErrorResponse.ts @@ -1,7 +1,10 @@ // Generated from Capsem OpenAPI. Do not edit. - +import type { ErrorCode } from "./ErrorCode.js"; export interface ErrorResponse { + "code"?: null | ErrorCode; "error": string; + "timeout_secs"?: number | null; + "vm_id"?: string | null; } diff --git a/sdk/typescript/src/models/index.ts b/sdk/typescript/src/models/index.ts index e6050fa16..a0a54ca38 100644 --- a/sdk/typescript/src/models/index.ts +++ b/sdk/typescript/src/models/index.ts @@ -26,6 +26,7 @@ export type { CredentialInjectResponse } from "./CredentialInjectResponse.js"; export { CredentialOutcome } from "./CredentialOutcome.js"; export { CredentialStorage } from "./CredentialStorage.js"; export type { DnsEvent } from "./DnsEvent.js"; +export { ErrorCode } from "./ErrorCode.js"; export type { ErrorEvent } from "./ErrorEvent.js"; export type { ErrorResponse } from "./ErrorResponse.js"; export type { EventBodiesResponse } from "./EventBodiesResponse.js"; diff --git a/sdk/typescript/src/validation/ErrorCode.ts b/sdk/typescript/src/validation/ErrorCode.ts new file mode 100644 index 000000000..16e9e855c --- /dev/null +++ b/sdk/typescript/src/validation/ErrorCode.ts @@ -0,0 +1,6 @@ +// Generated from Capsem OpenAPI. Do not edit. + +import {z} from "zod"; +import {ErrorCode} from "../models/ErrorCode.js"; + +export const ErrorCodeSchema: z.ZodType = z.enum(ErrorCode); diff --git a/sdk/typescript/src/validation/ErrorResponse.ts b/sdk/typescript/src/validation/ErrorResponse.ts index 2767abe1d..9498fd71b 100644 --- a/sdk/typescript/src/validation/ErrorResponse.ts +++ b/sdk/typescript/src/validation/ErrorResponse.ts @@ -2,7 +2,11 @@ import {z} from "zod"; import type {ErrorResponse} from "../models/ErrorResponse.js"; +import {ErrorCodeSchema} from "./ErrorCode.js"; export const ErrorResponseSchema: z.ZodType = z.object({ + "code": z.union([z.null(), z.lazy(() => ErrorCodeSchema)]).exactOptional(), "error": z.string(), + "timeout_secs": z.int().min(0).nullable().exactOptional(), + "vm_id": z.string().nullable().exactOptional(), }); diff --git a/sdk/typescript/src/validation/index.ts b/sdk/typescript/src/validation/index.ts index 787566225..637cf83e5 100644 --- a/sdk/typescript/src/validation/index.ts +++ b/sdk/typescript/src/validation/index.ts @@ -26,6 +26,7 @@ export {CredentialInjectResponseSchema} from "./CredentialInjectResponse.js"; export {CredentialOutcomeSchema} from "./CredentialOutcome.js"; export {CredentialStorageSchema} from "./CredentialStorage.js"; export {DnsEventSchema} from "./DnsEvent.js"; +export {ErrorCodeSchema} from "./ErrorCode.js"; export {ErrorEventSchema} from "./ErrorEvent.js"; export {ErrorResponseSchema} from "./ErrorResponse.js"; export {EventBodiesResponseSchema} from "./EventBodiesResponse.js"; diff --git a/sdk/typescript/tests/deadlines.test.ts b/sdk/typescript/tests/deadlines.test.ts index 0bafa3324..76c725009 100644 --- a/sdk/typescript/tests/deadlines.test.ts +++ b/sdk/typescript/tests/deadlines.test.ts @@ -1,5 +1,12 @@ import {expect, it} from 'vitest'; -import {commandDeadlineMs, createDeadlineMs} from '../src/execution.js'; +import { + CREATE_READY_SECS, + EXEC_TIMEOUT_CEILING_SECS, + GATEWAY_REQUEST_BUDGET_SECS, + MAX_REQUEST_BODY_BYTES, + commandDeadlineMs, + createDeadlineMs, +} from '../src/execution.js'; import {Hypervisor, VM} from '../src/index.js'; import {FacadeGateway} from './facade-gateway.js'; import {gateway} from './gateway.js'; @@ -13,6 +20,10 @@ it.each([ [5_000_000, 10, 5_000_000], ])('deadline for default %i ms and timeout_secs %s is %i ms', (fallback, timeoutSecs, expected) => { expect(commandDeadlineMs(fallback, timeoutSecs)).toBe(expected); + expect(EXEC_TIMEOUT_CEILING_SECS).toBe(3600); + expect(GATEWAY_REQUEST_BUDGET_SECS).toBe(120); + expect(CREATE_READY_SECS).toBe(110); + expect(MAX_REQUEST_BODY_BYTES).toBe(10 * 1024 * 1024); }); it.each([[30_000, 230_000], [500_000, 500_000]])('create deadline preserves readiness and a larger default: %i', (fallback, expected) => { diff --git a/tests/citadel/test_exec_deadline_constants.py b/tests/citadel/test_exec_deadline_constants.py index c48795a65..5fca91159 100644 --- a/tests/citadel/test_exec_deadline_constants.py +++ b/tests/citadel/test_exec_deadline_constants.py @@ -15,8 +15,20 @@ #: Where the service and the gateway define what a client has to wait for. SERVICE_CEILING = ("crates/capsem-api/src/execution.rs", r"MAX_EXEC_TIMEOUT_SECS: u64 = (.+?);") -GATEWAY_BUDGET = ("crates/capsem-gateway/src/proxy.rs", r"REQUEST_TIMEOUT: Duration = Duration::from_secs\((\d+)\)") -#: Each client's copy of those two numbers, in seconds. +GATEWAY_BUDGET = ( + "crates/capsem-gateway/src/proxy.rs", + r"REQUEST_TIMEOUT: Duration = Duration::from_secs\((\d+)\)", +) +SERVICE_CREATE_READY = ( + "crates/capsem-service/src/container_setup.rs", + r"CREATE_READY_TIMEOUT: std::time::Duration = std::time::Duration::from_secs\((.+?)\)", +) +PROVISION_CREATE_WAIT = ( + "crates/capsem-service/src/vm_files/provision.rs", + r"container_setup::require_ready\(state, id\)", +) +API_BODY_LIMIT = ("crates/capsem-api/src/files.rs", r"MAX_REQUEST_BODY_BYTES: usize = (.+?);") +#: Each client's copy of those numbers, in seconds or bytes. CLIENT_CEILINGS = ( ("sdk/python/capsem/execution.py", r"EXEC_TIMEOUT_CEILING_SECS = (.+)"), ("sdk/typescript/src/execution.ts", r"EXEC_TIMEOUT_CEILING_SECS = (.+?);"), @@ -27,21 +39,30 @@ ("sdk/typescript/src/execution.ts", r"GATEWAY_REQUEST_BUDGET_SECS = (.+?);"), ("sdk/rust/src/client.rs", r"GATEWAY_REQUEST_BUDGET: Duration = Duration::from_secs\((.+?)\)"), ) +CLIENT_CREATE_READY = ( + ("sdk/python/capsem/execution.py", r"CREATE_READY_SECS = (.+)"), + ("sdk/typescript/src/execution.ts", r"CREATE_READY_SECS = (.+?);"), +) +CLIENT_BODY_LIMITS = ( + ("sdk/python/capsem/execution.py", r"MAX_REQUEST_BODY_BYTES = (.+)"), + ("sdk/typescript/src/execution.ts", r"MAX_REQUEST_BODY_BYTES = (.+?);"), +) #: The desktop UI waits at least as long as the gateway does. WEB_BUDGET = ("web/app/src/lib/gateway-sdk.ts", r"GATEWAY_REQUEST_BUDGET_MS = (.+?);") DEADLINE_RATIONALE = """\ -Exec and run answer only when the command finishes. - -The service bounds that by the request's timeout_secs, defaulting to -MAX_EXEC_TIMEOUT_SECS, and the gateway waits that ceiling plus its ordinary -REQUEST_TIMEOUT. Clients whose only deadline was their 30 s transport default -aborted long commands that kept running server-side, and a retrying agent -started a second build. - -Every SDK now stretches its per-call deadline to the same two numbers, so the -numbers must be the same numbers. Change the service or gateway value and this -guard names every client that still holds the old one. +Exec, run, container create and file transfers share contract limits with the +service and gateway. + +The service bounds exec/run by the request's timeout_secs, defaulting to +MAX_EXEC_TIMEOUT_SECS, waits CREATE_READY_TIMEOUT for a container workload on +create, and caps HTTP request/file bodies at MAX_REQUEST_BODY_BYTES; the +gateway adds REQUEST_TIMEOUT. Clients whose only deadline was their 30 s +transport default or a magic chunk size drifted from the server contract. + +Every SDK mirrors the same numbers, so the numbers must stay equal. Change the +service, API or gateway value and this guard names every client that still +holds the old one. """ @@ -67,13 +88,40 @@ def constant(source: tuple[str, str]) -> int: def test_every_client_waits_for_the_service_exec_ceiling() -> None: expected = constant(SERVICE_CEILING) drifted = {path: constant((path, pattern)) for path, pattern in CLIENT_CEILINGS} - assert set(drifted.values()) == {expected}, f"service ceiling is {expected}s: {drifted}.\n\n{DEADLINE_RATIONALE}" + assert set(drifted.values()) == {expected}, ( + f"service ceiling is {expected}s: {drifted}.\n\n{DEADLINE_RATIONALE}" + ) def test_every_client_adds_the_gateway_request_budget() -> None: expected = constant(GATEWAY_BUDGET) drifted = {path: constant((path, pattern)) for path, pattern in CLIENT_BUDGETS} - assert set(drifted.values()) == {expected}, f"gateway budget is {expected}s: {drifted}.\n\n{DEADLINE_RATIONALE}" + assert set(drifted.values()) == {expected}, ( + f"gateway budget is {expected}s: {drifted}.\n\n{DEADLINE_RATIONALE}" + ) + + +def test_every_client_waits_for_the_service_create_ready_timeout() -> None: + provision_path, provision_pattern = PROVISION_CREATE_WAIT + assert re.search(provision_pattern, (PROJECT_ROOT / provision_path).read_text()), ( + f"{provision_path} no longer calls {provision_pattern!r}.\n\n{DEADLINE_RATIONALE}" + ) + expected = constant(SERVICE_CREATE_READY) + drifted = {path: constant((path, pattern)) for path, pattern in CLIENT_CREATE_READY} + assert set(drifted.values()) == {expected}, ( + f"service create-ready timeout is {expected}s: {drifted}.\n\n{DEADLINE_RATIONALE}" + ) + + +def test_every_client_mirrors_the_api_request_body_limit() -> None: + expected = constant(API_BODY_LIMIT) + drifted = {path: constant((path, pattern)) for path, pattern in CLIENT_BODY_LIMITS} + assert set(drifted.values()) == {expected}, ( + f"API request body limit is {expected} bytes: {drifted}.\n\n{DEADLINE_RATIONALE}" + ) + assert "pub use capsem_api as models;" in (PROJECT_ROOT / "sdk/rust/src/lib.rs").read_text(), ( + DEADLINE_RATIONALE + ) def test_the_desktop_ui_waits_at_least_as_long_as_the_gateway() -> None: