From 08eb260cc959afe5766789a414fc37073e417c6f Mon Sep 17 00:00:00 2001 From: Soham Das Date: Wed, 4 Nov 2026 12:12:21 -0800 Subject: [PATCH 1/2] [WSLC] Set operation and nativeCode on container lifecycle errors Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docs/backends/wslc/wslc-state-aware.md | 8 +- .../architecture/container-lifecycle.md | 8 +- .../backends/wslc/common/container_steps.rs | 175 ++++++---- .../src/backends/wslc/common/daemon_client.rs | 98 +++--- .../backends/wslc/common/daemon_protocol.rs | 76 ++++- src/mxc-sdk/src/backends/wslc/common/error.rs | 68 ++++ src/mxc-sdk/src/backends/wslc/common/image.rs | 77 +++-- .../src/backends/wslc/common/state_aware.rs | 110 +++++- .../wslc/common/wsl_container_runner.rs | 2 +- .../src/bin/wslc_daemon/control_server.rs | 66 +++- .../src/bin/wslc_daemon/session_manager.rs | 323 +++++++++++++++--- tests/scripts/run_wslc_state_aware_tests.ps1 | 4 + 12 files changed, 792 insertions(+), 223 deletions(-) diff --git a/docs/backends/wslc/wslc-state-aware.md b/docs/backends/wslc/wslc-state-aware.md index 224041e4c0..d57ed5c4e1 100644 --- a/docs/backends/wslc/wslc-state-aware.md +++ b/docs/backends/wslc/wslc-state-aware.md @@ -703,6 +703,10 @@ reported, so a phase failure reaches the SDK with the same code that create-and-run execution returns for the same host condition. A caller branches on the code without matching the message. +When a WSLc SDK call fails, including in an exec that has already started, the +error envelope also carries the SDK function name in `operation`, and its HRESULT +in `nativeCode` when the call returned one. + `ErrKind` is additive: a kind a client does not recognize decodes to `ErrKind::Unknown` and maps to `backend_error`, so a daemon from a newer install degrades to the previous behavior instead of failing the decode. Adding @@ -727,8 +731,8 @@ success for a request it only partly honoured — a sandbox that is quietly not what was asked for. A refusal the caller can see is better. Upgrading an install that may have a live daemon therefore means stopping the -old one first. `PROTOCOL_VERSION` is **9** as of the addition of exec stdin -(the `stdin` flag on `exec` and the `exec_stdin` request). +old one first. `PROTOCOL_VERSION` is **10** as of the addition of optional WSLc +SDK failure detail to lifecycle error responses and exec error frames. ## Daemon idle-timeout env overrides diff --git a/docs/development/architecture/container-lifecycle.md b/docs/development/architecture/container-lifecycle.md index 1cc78f6f18..5aad09dd8c 100644 --- a/docs/development/architecture/container-lifecycle.md +++ b/docs/development/architecture/container-lifecycle.md @@ -685,9 +685,11 @@ The three optional named fields carry structured failure detail. `operation` and | `nativeCode` | The underlying platform status as a string. An HRESULT such as `0x80070490` on Windows; the field is platform-neutral, so another backend can carry an errno or equivalent. | | `remediation` | An actionable "how to fix it" hint, when the failure has one. | -**Availability.** These fields are currently populated only by **IsolationSession -state-aware** operations. Other backends may adopt them — treat all three as optional on -every backend, and branch program logic on `code` first. +**Availability.** IsolationSession container lifecycle operations may populate all +three fields. WSLc container lifecycle operations populate `operation` for failures +reported by the WSLc SDK and `nativeCode` when the SDK supplies an HRESULT. Other +backends may adopt them — treat all three as optional on every backend, and branch +program logic on `code` first. **Stability.** Unlike `code`, which is a closed and versioned enum, the *values* of `operation` and `nativeCode` are **best-effort diagnostics and may change without a schema version bump**. They are derived from the underlying platform API — for IsolationSession, from the projected WinRT class and method names — which MXC does not own and cannot version. Consumers should aggregate on them for telemetry and log them for diagnosis, but branch program logic on `code`, and should not treat a particular `operation` value as a guarantee. (MXC's own end-to-end tests do pin exact values; that is deliberate — they verify MXC's mapping, and move with it in the same change.) diff --git a/src/mxc-sdk/src/backends/wslc/common/container_steps.rs b/src/mxc-sdk/src/backends/wslc/common/container_steps.rs index 7cc10cda50..fdf8784ade 100644 --- a/src/mxc-sdk/src/backends/wslc/common/container_steps.rs +++ b/src/mxc-sdk/src/backends/wslc/common/container_steps.rs @@ -46,7 +46,7 @@ use crate::mxc_common::string_util::{to_wide, CoTaskMemPWSTR}; use crate::wslc_common::daemon_protocol::ExecTerminal; -use crate::wslc_common::error::WslcError; +use crate::wslc_common::error::{sdk_api_failure, WslcError, WslcStepError}; use crate::wslc_common::policy_mapping::VolumeMount; use crate::wslc_common::process_env::{self, EnvScope}; use crate::wslc_common::sdk_init; @@ -156,6 +156,21 @@ pub(crate) fn sdk_error(context: &str, hr: HRESULT, sdk_msg: &str) -> ScriptResp WslcError::sdk(context, hr, sdk_msg).into_response() } +pub(crate) fn sdk_step_error(operation: &str, hr: HRESULT, sdk_msg: &str) -> WslcStepError { + sdk_step_error_with_context(operation, &format!("{operation} failed"), hr, sdk_msg) +} + +pub(crate) fn sdk_step_error_with_context( + operation: &str, + context: &str, + hr: HRESULT, + sdk_msg: &str, +) -> WslcStepError { + WslcError::sdk(context, hr, sdk_msg) + .into_step_error() + .with_api_failure(sdk_api_failure(operation, hr)) +} + /// NUL-terminate `value` for a C string the WSLc SDK will read, rejecting an /// interior NUL instead of letting the SDK silently observe a truncated value. /// @@ -369,7 +384,7 @@ pub(crate) unsafe fn set_command_line_and_env( scope: EnvScope, env: &[String], script_code: &str, -) -> Result { +) -> Result { let argv_cstrings = process_env::argv_words(scope, env, script_code) .iter() .map(|word| cstr_bytes("command", word)) @@ -377,7 +392,7 @@ pub(crate) unsafe fn set_command_line_and_env( let argv: Vec = argv_cstrings.iter().map(|w| w.as_ptr() as PCSTR).collect(); let hr = sdk.WslcSetProcessSettingsCmdLine(settings, argv.as_ptr(), argv.len()); if hr != S_OK { - return Err(sdk_error("WslcSetProcessSettingsCmdLine failed", hr, "")); + return Err(sdk_step_error("WslcSetProcessSettingsCmdLine", hr, "")); } let env_cstrings: Vec> = process_env::sdk_entries(scope, env) @@ -390,11 +405,7 @@ pub(crate) unsafe fn set_command_line_and_env( let hr = sdk.WslcSetProcessSettingsEnvVariables(settings, env_ptrs.as_ptr(), env_ptrs.len()); if hr != S_OK { - return Err(sdk_error( - "WslcSetProcessSettingsEnvVariables failed", - hr, - "", - )); + return Err(sdk_step_error("WslcSetProcessSettingsEnvVariables", hr, "")); } } @@ -479,7 +490,7 @@ impl ProcessSettings { scope: EnvScope, working_directory: &str, sink: Option, - ) -> Result { + ) -> Result { Self::build_inner(sdk, script_code, env, scope, working_directory, true, sink) } @@ -495,7 +506,7 @@ impl ProcessSettings { env: &[String], scope: EnvScope, working_directory: &str, - ) -> Result { + ) -> Result { Self::build_inner(sdk, script_code, env, scope, working_directory, false, None) } @@ -507,11 +518,11 @@ impl ProcessSettings { working_directory: &str, register_callbacks: bool, sink: Option, - ) -> Result { + ) -> Result { let mut raw = std::mem::zeroed::(); let hr = sdk.WslcInitProcessSettings(&mut raw); if hr != S_OK { - return Err(sdk_error("WslcInitProcessSettings failed", hr, "")); + return Err(sdk_step_error("WslcInitProcessSettings", hr, "")); } let io_ctx = Arc::new(IoContext { @@ -540,7 +551,7 @@ impl ProcessSettings { if hr != S_OK { // The SDK never took ownership; reclaim the reference now. drop(Arc::from_raw(io_ctx_raw as *const IoContext)); - return Err(sdk_error("WslcSetProcessSettingsCallbacks failed", hr, "")); + return Err(sdk_step_error("WslcSetProcessSettingsCallbacks", hr, "")); } sdk_io_ref = SdkIoRef(Some(io_ctx_raw as *const IoContext)); } @@ -556,13 +567,13 @@ impl ProcessSettings { return Err(WslcError::Rejected(format!( "working_directory must be an absolute in-container path (got {working_directory:?})" )) - .into_response()); + .into()); } let c = cstr_bytes("working_directory", working_directory)?; let hr = sdk.WslcSetProcessSettingsWorkingDirectory(&mut raw, c.as_ptr() as PCSTR); if hr != S_OK { - return Err(sdk_error( - "WslcSetProcessSettingsWorkingDirectory failed", + return Err(sdk_step_error( + "WslcSetProcessSettingsWorkingDirectory", hr, "", )); @@ -639,12 +650,12 @@ impl ContainerSettings { net_mode: WslcContainerNetworkingMode, flags: WslcContainerFlags, logger: &mut Logger, - ) -> Result { + ) -> Result { let image_cstr = cstr_bytes("image", image)?; let mut raw = std::mem::zeroed::(); let hr = sdk.WslcInitContainerSettings(image_cstr.as_ptr() as PCSTR, &mut raw); if hr != S_OK { - return Err(sdk_error("WslcInitContainerSettings failed", hr, "")); + return Err(sdk_step_error("WslcInitContainerSettings", hr, "")); } // Port mappings (host<->container). Empty list = no forwarding. The @@ -672,8 +683,8 @@ impl ContainerSettings { port_vec.len() as u32, ); if hr != S_OK { - return Err(sdk_error( - "WslcSetContainerSettingsPortMappings failed", + return Err(sdk_step_error( + "WslcSetContainerSettingsPortMappings", hr, "", )); @@ -714,7 +725,7 @@ impl ContainerSettings { volumes.len() as u32, ); if hr != S_OK { - return Err(sdk_error("WslcSetContainerSettingsVolumes failed", hr, "")); + return Err(sdk_step_error("WslcSetContainerSettingsVolumes", hr, "")); } let _ = writeln!( logger, @@ -725,8 +736,8 @@ impl ContainerSettings { let hr = sdk.WslcSetContainerSettingsNetworkingMode(&mut raw, net_mode); if hr != S_OK { - return Err(sdk_error( - "WslcSetContainerSettingsNetworkingMode failed", + return Err(sdk_step_error( + "WslcSetContainerSettingsNetworkingMode", hr, "", )); @@ -735,7 +746,7 @@ impl ContainerSettings { let hr = sdk.WslcSetContainerSettingsFlags(&mut raw, flags); if hr != S_OK { - return Err(sdk_error("WslcSetContainerSettingsFlags failed", hr, "")); + return Err(sdk_step_error("WslcSetContainerSettingsFlags", hr, "")); } Ok(ContainerSettings { @@ -789,16 +800,16 @@ pub const KEEPALIVE_SCRIPT: &str = "while true; do sleep 86400; done"; /// COM must already be initialised on the calling thread and the SDK DLL must be /// resolvable. The returned [`WslcSdk`] holds raw function pointers; keep it /// alive for the duration of all SDK use. -pub unsafe fn load_sdk_checked(logger: &mut Logger) -> Result { - let sdk = WslcSdk::load().map_err(|e| WslcError::Unavailable(e).into_response())?; +pub unsafe fn load_sdk_checked(logger: &mut Logger) -> Result { + let sdk = WslcSdk::load().map_err(|e| WslcError::Unavailable(e).into_step_error())?; let mut missing = WslcComponentFlags::WSLC_COMPONENT_FLAG_NONE; let hr = sdk.WslcGetMissingComponents(&mut missing); if hr != S_OK { - return Err(sdk_error("WslcGetMissingComponents failed", hr, "")); + return Err(sdk_step_error("WslcGetMissingComponents", hr, "")); } if missing.any_missing() { - return Err(WslcError::Unavailable(sdk_init::prerequisite_error(missing)).into_response()); + return Err(WslcError::Unavailable(sdk_init::prerequisite_error(missing)).into()); } let _ = writeln!(logger, "[WSLC][daemon] Runtime check passed"); Ok(sdk) @@ -806,15 +817,14 @@ pub unsafe fn load_sdk_checked(logger: &mut Logger) -> Result, + pub response: WslcStepError, pub store_may_be_held: bool, } -impl From for SessionCreateFailure { - fn from(response: ScriptResponse) -> Self { +impl From for SessionCreateFailure { + fn from(response: WslcStepError) -> Self { Self { - response: Box::new(response), + response, store_may_be_held: false, } } @@ -841,7 +851,7 @@ pub unsafe fn create_daemon_session( let mut settings = std::mem::zeroed::(); let hr = sdk.WslcInitSessionSettings(name_wide.as_ptr(), storage_wide.as_ptr(), &mut settings); if hr != S_OK { - return Err(sdk_error("WslcInitSessionSettings failed", hr, "").into()); + return Err(sdk_step_error("WslcInitSessionSettings", hr, "").into()); } apply_session_sizing(sdk, &mut settings, sizing)?; @@ -852,7 +862,7 @@ pub unsafe fn create_daemon_session( let created_in = started.elapsed(); if hr != S_OK { let msg = err_msg.to_string_lossy(); - return Err(sdk_error("WslcCreateSession failed", hr, &msg).into()); + return Err(sdk_step_error("WslcCreateSession", hr, &msg).into()); } let guard = WslcSessionGuard::from_raw( @@ -890,7 +900,7 @@ pub unsafe fn create_daemon_session( } }; return Err(SessionCreateFailure { - response: Box::new(response), + response, store_may_be_held, }); } @@ -909,13 +919,13 @@ unsafe fn apply_session_sizing( sdk: &WslcSdk, settings: &mut WslcSessionSettings, sizing: SessionSizing, -) -> Result<(), ScriptResponse> { +) -> Result<(), WslcStepError> { check_sizing_against_host(sizing)?; if let Some(cpu) = sizing.cpu_count { let hr = sdk.WslcSetSessionSettingsCpuCount(settings, cpu); if hr != S_OK { - return Err(sdk_error("WslcSetSessionSettingsCpuCount failed", hr, "")); + return Err(sdk_step_error("WslcSetSessionSettingsCpuCount", hr, "")); } } @@ -925,17 +935,17 @@ unsafe fn apply_session_sizing( "memory_mb {memory_mb} exceeds the {} MB the WSLc SDK accepts", u32::MAX )) - .into_response() + .into_step_error() })?; let hr = sdk.WslcSetSessionSettingsMemory(settings, memory_mb); if hr != S_OK { - return Err(sdk_error("WslcSetSessionSettingsMemory failed", hr, "")); + return Err(sdk_step_error("WslcSetSessionSettingsMemory", hr, "")); } } let hr = sdk.WslcSetSessionSettingsTimeout(settings, SESSION_BOOT_TIMEOUT_MS); if hr != S_OK { - return Err(sdk_error("WslcSetSessionSettingsTimeout failed", hr, "")); + return Err(sdk_step_error("WslcSetSessionSettingsTimeout", hr, "")); } if sizing.gpu { @@ -944,11 +954,7 @@ unsafe fn apply_session_sizing( WslcSessionFeatureFlags::WSLC_SESSION_FEATURE_FLAG_ENABLE_GPU, ); if hr != S_OK { - return Err(sdk_error( - "WslcSetSessionSettingsFeatureFlags failed", - hr, - "", - )); + return Err(sdk_step_error("WslcSetSessionSettingsFeatureFlags", hr, "")); } } @@ -963,11 +969,11 @@ pub fn release_daemon_session( session: WslcSessionGuard, session_name: &str, logger: &mut Logger, -) -> Result<(), ScriptResponse> { +) -> Result<(), WslcStepError> { let hr = session.release_checked(); if hr != S_OK { - return Err(sdk_error( - "WslcTerminateSession failed", + return Err(sdk_step_error( + "WslcTerminateSession", hr, &format!( "the WSL2 utility VM for session '{session_name}' may still be running, so its \ @@ -995,7 +1001,7 @@ pub unsafe fn validate_store_access( sdk: &WslcSdk, session: WslcSession, storage_path: &str, -) -> Result<(), ScriptResponse> { +) -> Result<(), WslcStepError> { list_stored_images(sdk, session) .map(|_| ()) .map_err(|hr| store_access_failure(hr, storage_path)) @@ -1003,9 +1009,10 @@ pub unsafe fn validate_store_access( /// Render a failed store probe, telling a sharing violation apart from any /// other reason the store could not be read. -fn store_access_failure(hr: HRESULT, storage_path: &str) -> ScriptResponse { +fn store_access_failure(hr: HRESULT, storage_path: &str) -> WslcStepError { if hr == E_SHARING_VIOLATION { - sdk_error( + sdk_step_error_with_context( + "WslcListSessionImages", "WSLc store unavailable", hr, &format!( @@ -1016,7 +1023,8 @@ fn store_access_failure(hr: HRESULT, storage_path: &str) -> ScriptResponse { ), ) } else { - sdk_error( + sdk_step_error_with_context( + "WslcListSessionImages", "WslcListSessionImages failed", hr, &format!("could not read the image store at {storage_path}"), @@ -1098,7 +1106,7 @@ pub unsafe fn create_daemon_container( gpu: bool, keepalive: &mut ProcessSettings, logger: &mut Logger, -) -> Result { +) -> Result { let mut container_settings = ContainerSettings::build( sdk, image, @@ -1112,8 +1120,8 @@ pub unsafe fn create_daemon_container( let hr = sdk.WslcSetContainerSettingsInitProcess(container_settings.raw_mut(), keepalive.raw_mut()); if hr != S_OK { - return Err(sdk_error( - "WslcSetContainerSettingsInitProcess failed", + return Err(sdk_step_error( + "WslcSetContainerSettingsInitProcess", hr, "", )); @@ -1129,7 +1137,7 @@ pub unsafe fn create_daemon_container( ); if hr != S_OK { let msg = err_msg.to_string_lossy(); - return Err(sdk_error("WslcCreateContainer failed", hr, &msg)); + return Err(sdk_step_error("WslcCreateContainer", hr, &msg)); } let _ = writeln!(logger, "[WSLC][daemon] Container created"); @@ -1149,7 +1157,7 @@ pub unsafe fn start_daemon_container( sdk: &WslcSdk, container: WslcContainer, logger: &mut Logger, -) -> Result<(), ScriptResponse> { +) -> Result<(), WslcStepError> { let mut err_msg = CoTaskMemPWSTR::null(); let hr = sdk.WslcStartContainer( container, @@ -1158,7 +1166,7 @@ pub unsafe fn start_daemon_container( ); if hr != S_OK { let msg = err_msg.to_string_lossy(); - return Err(sdk_error("WslcStartContainer failed", hr, &msg)); + return Err(sdk_step_error("WslcStartContainer", hr, &msg)); } let _ = writeln!(logger, "[WSLC][daemon] Container started"); Ok(()) @@ -1323,7 +1331,7 @@ pub struct ExecOutcome { pub stderr: Vec, /// Specific post-process-creation failure that made termination /// unobservable. The daemon uses this context when quarantining. - pub post_launch_error: Option, + pub post_launch_error: Option, /// The process's stdin, still open because its termination was not /// confirmed. @@ -1356,7 +1364,7 @@ pub unsafe fn exec_in_container( sink: Option, stdin: Option, logger: &mut Logger, -) -> Result { +) -> Result { // `process_settings` owns every buffer the SDK reads at // `WslcCreateContainerProcess` time plus the I/O-capture context; it is held // as a stationary local until after the process exits below. @@ -1368,7 +1376,7 @@ pub unsafe fn exec_in_container( WslcProcessFlags::WSLC_PROCESS_FLAG_STDIN, ); if hr != S_OK { - return Err(sdk_error("WslcSetProcessSettingsFlags failed", hr, "")); + return Err(sdk_step_error("WslcSetProcessSettingsFlags", hr, "")); } } @@ -1382,7 +1390,7 @@ pub unsafe fn exec_in_container( ); if hr != S_OK { let msg = err_msg.to_string_lossy(); - return Err(sdk_error("WslcCreateContainerProcess failed", hr, &msg)); + return Err(sdk_step_error("WslcCreateContainerProcess", hr, &msg)); } let process_guard = WslcProcessGuard::from_raw(process, sdk.release_process_fn()); // The process now owns the SDK's IoContext reference; `exit_callback` will @@ -1420,7 +1428,7 @@ pub unsafe fn exec_in_container( completion: ProcessCompletion::TerminationUnconfirmed, stdout: Vec::new(), stderr: Vec::new(), - post_launch_error: Some(sdk_error("WslcGetProcessExitEvent failed", hr, "")), + post_launch_error: Some(sdk_step_error("WslcGetProcessExitEvent", hr, "")), held_stdin: Some(stdin_guard), }); } @@ -1500,7 +1508,7 @@ pub unsafe fn exec_in_container( let hr = sdk.WslcGetProcessExitCode(process_guard.as_raw(), &mut exit_code); if hr != S_OK { if natural_exit_confirmed(exit_wait, confirmed) { - return Err(sdk_error("WslcGetProcessExitCode failed", hr, "")); + return Err(sdk_step_error("WslcGetProcessExitCode", hr, "")); } let _ = writeln!( logger, @@ -1625,7 +1633,7 @@ unsafe fn attach_stdin( sdk: &WslcSdk, process: WslcProcess, consumer: StdinConsumer, -) -> Result, ScriptResponse> { +) -> Result, WslcStepError> { use std::os::windows::io::FromRawHandle; let mut handle: HANDLE = ptr::null_mut(); @@ -1635,13 +1643,18 @@ unsafe fn attach_stdin( &mut handle, ); if hr != S_OK || handle.is_null() { - return Err(sdk_error("WslcGetProcessIOHandle(stdin) failed", hr, "")); + return Err(sdk_step_error_with_context( + "WslcGetProcessIOHandle", + "WslcGetProcessIOHandle(stdin) failed", + hr, + "", + )); } // SAFETY: the SDK transfers ownership of the returned handle to the caller. let stdin = std::fs::File::from_raw_handle(handle); consumer(stdin).map_err(|error| { - WslcError::Runtime(format!("could not start forwarding stdin: {error}")).into_response() + WslcError::Runtime(format!("could not start forwarding stdin: {error}")).into_step_error() }) } @@ -1654,7 +1667,7 @@ pub unsafe fn stop_daemon_container( sdk: &WslcSdk, container: WslcContainer, logger: &mut Logger, -) -> Result<(), ScriptResponse> { +) -> Result<(), WslcStepError> { let mut err_msg = CoTaskMemPWSTR::null(); let hr = sdk.WslcStopContainer( container, @@ -1664,7 +1677,7 @@ pub unsafe fn stop_daemon_container( ); if hr != S_OK { let msg = err_msg.to_string_lossy(); - return Err(sdk_error("WslcStopContainer failed", hr, &msg)); + return Err(sdk_step_error("WslcStopContainer", hr, &msg)); } let _ = writeln!(logger, "[WSLC][daemon] Container stopped"); Ok(()) @@ -1678,7 +1691,7 @@ pub unsafe fn delete_daemon_container( sdk: &WslcSdk, container: WslcContainer, logger: &mut Logger, -) -> Result<(), ScriptResponse> { +) -> Result<(), WslcStepError> { let mut err_msg = CoTaskMemPWSTR::null(); let hr = sdk.WslcDeleteContainer( container, @@ -1687,7 +1700,7 @@ pub unsafe fn delete_daemon_container( ); if hr != S_OK { let msg = err_msg.to_string_lossy(); - return Err(sdk_error("WslcDeleteContainer failed", hr, &msg)); + return Err(sdk_step_error("WslcDeleteContainer", hr, &msg)); } let _ = writeln!(logger, "[WSLC][daemon] Container deleted"); Ok(()) @@ -1828,6 +1841,24 @@ mod tests { assert!(resp.error_message.contains(r"C:\slots\slot-0")); } + #[test] + fn a_failed_sdk_call_keeps_its_message_and_names_the_call() { + let error = sdk_step_error( + "WslcStartContainer", + 0x8007_2740_u32 as HRESULT, + "Only one usage of each socket address is normally permitted.", + ); + + assert_eq!( + error.error_message, + "WslcStartContainer failed: Only one usage of each socket address is normally \ + permitted. (HRESULT 0x80072740)" + ); + let failure = error.api_failure.as_deref().expect("api failure"); + assert_eq!(failure.operation, "WslcStartContainer"); + assert_eq!(failure.native_code.as_deref(), Some("0x80072740")); + } + #[test] fn a_store_failure_is_attributed_to_bring_up() { assert_eq!( diff --git a/src/mxc-sdk/src/backends/wslc/common/daemon_client.rs b/src/mxc-sdk/src/backends/wslc/common/daemon_client.rs index 90ad254cec..9397a7cc0d 100644 --- a/src/mxc-sdk/src/backends/wslc/common/daemon_client.rs +++ b/src/mxc-sdk/src/backends/wslc/common/daemon_client.rs @@ -40,10 +40,10 @@ use serde::Serialize; use crate::wslc_common::container_steps::OutStream; use crate::wslc_common::daemon_protocol::{ - encode_frame, CancelExecConfig, DaemonRequest, DaemonResponse, DeprovisionConfig, - DeprovisionSessionConfig, ErrKind, ExecConfig, ExecStdinConfig, ExecTerminal, ProvisionConfig, - ProvisionSessionConfig, StartConfig, StopConfig, StreamFrame, MAX_FRAME_SIZE, - MAX_STDIN_CHUNK_BYTES, PROTOCOL_VERSION, + encode_frame, CancelExecConfig, DaemonApiFailure, DaemonRequest, DaemonResponse, + DeprovisionConfig, DeprovisionSessionConfig, ErrKind, ExecConfig, ExecStdinConfig, + ExecTerminal, ProvisionConfig, ProvisionSessionConfig, StartConfig, StopConfig, StreamFrame, + MAX_FRAME_SIZE, MAX_STDIN_CHUNK_BYTES, PROTOCOL_VERSION, }; use crate::wslc_common::daemon_record::{live_daemon, DaemonRecord, TransitionLock}; @@ -190,11 +190,15 @@ impl AdmittedExec { StreamFrame::Exit { code } => DaemonExecOutcome::Exited(code), StreamFrame::TimedOut => DaemonExecOutcome::TimedOut, StreamFrame::Cancelled => DaemonExecOutcome::Cancelled, - StreamFrame::Error { message } => { - return Err(DaemonError::transport(format!( - "exec failed: {message}{}", - truncation_suffix(truncated) - ))) + StreamFrame::Error { + message, + api_failure, + } => { + return Err(DaemonError::Daemon { + kind: ErrKind::Backend, + message: format!("exec failed: {message}{}", truncation_suffix(truncated)), + api_failure, + }) } StreamFrame::Stdin { .. } => { return Err(DaemonError::transport( @@ -225,8 +229,12 @@ pub(crate) fn truncation_suffix(truncated: bool) -> &'static str { /// which map to a generic backend error. #[derive(Debug)] pub enum DaemonError { - /// The daemon returned a structured [`DaemonResponse::Err`]. - Daemon { kind: ErrKind, message: String }, + /// The daemon returned a structured [`DaemonResponse::Err`] or [`StreamFrame::Error`]. + Daemon { + kind: ErrKind, + message: String, + api_failure: Option, + }, /// A transport / protocol failure (connect, framing, or an unexpected /// reply shape). Transport(anyhow::Error), @@ -241,7 +249,9 @@ impl DaemonError { impl std::fmt::Display for DaemonError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - Self::Daemon { kind, message } => write!(f, "daemon error [{kind:?}]: {message}"), + Self::Daemon { kind, message, .. } => { + write!(f, "daemon error [{kind:?}]: {message}") + } Self::Transport(e) => write!(f, "{e}"), } } @@ -337,20 +347,14 @@ impl DaemonClient { pub fn ping(&self) -> DaemonResult<()> { match self.call(&DaemonRequest::Ping)? { DaemonResponse::Pong => Ok(()), - DaemonResponse::Err { kind, message } => Err(DaemonError::Daemon { kind, message }), - other => Err(DaemonError::transport(format!( - "unexpected reply to ping: {other:?}" - ))), + other => Err(reply_error("unexpected reply to ping", other)), } } pub fn provision_session(&self, config: ProvisionSessionConfig) -> DaemonResult { match self.call(&DaemonRequest::ProvisionSession(config))? { DaemonResponse::SessionProvisioned { session_ref } => Ok(session_ref), - DaemonResponse::Err { kind, message } => Err(DaemonError::Daemon { kind, message }), - other => Err(DaemonError::transport(format!( - "unexpected reply to provision_session: {other:?}" - ))), + other => Err(reply_error("unexpected reply to provision_session", other)), } } @@ -362,10 +366,7 @@ impl DaemonClient { pub fn provision(&self, config: ProvisionConfig) -> DaemonResult { match self.call(&DaemonRequest::Provision(config))? { DaemonResponse::Provisioned { sandbox_id } => Ok(sandbox_id), - DaemonResponse::Err { kind, message } => Err(DaemonError::Daemon { kind, message }), - other => Err(DaemonError::transport(format!( - "unexpected reply to provision: {other:?}" - ))), + other => Err(reply_error("unexpected reply to provision", other)), } } @@ -467,14 +468,7 @@ impl DaemonClient { match response { DaemonResponse::Ok => {} - DaemonResponse::Err { kind, message } => { - return Err(DaemonError::Daemon { kind, message }) - } - other => { - return Err(DaemonError::transport(format!( - "unexpected exec admission reply: {other:?}" - ))) - } + other => return Err(reply_error("unexpected exec admission reply", other)), } Ok(AdmittedExec { pipe }) @@ -751,10 +745,22 @@ fn daemon_exe_path() -> Result { fn expect_ok(response: DaemonResponse) -> DaemonResult<()> { match response { DaemonResponse::Ok => Ok(()), - DaemonResponse::Err { kind, message } => Err(DaemonError::Daemon { kind, message }), - other => Err(DaemonError::transport(format!( - "unexpected reply (expected ok): {other:?}" - ))), + other => Err(reply_error("unexpected reply (expected ok)", other)), + } +} + +fn reply_error(unexpected: &str, response: DaemonResponse) -> DaemonError { + match response { + DaemonResponse::Err { + kind, + message, + api_failure, + } => DaemonError::Daemon { + kind, + message, + api_failure, + }, + other => DaemonError::transport(format!("{unexpected}: {other:?}")), } } @@ -1027,19 +1033,30 @@ mod tests { let err = expect_ok(DaemonResponse::Err { kind: ErrKind::NotStarted, message: "sandbox not started".to_string(), + api_failure: Some(DaemonApiFailure { + operation: "WslcStartContainer".to_string(), + native_code: Some("0x80072740".to_string()), + }), }) .unwrap_err(); - assert!(matches!( - err, + match &err { DaemonError::Daemon { kind: ErrKind::NotStarted, + api_failure: Some(api_failure), .. + } => { + assert_eq!(api_failure.operation, "WslcStartContainer"); + assert_eq!(api_failure.native_code.as_deref(), Some("0x80072740")); } - )); + other => panic!("unexpected error: {other:?}"), + } assert!(err.to_string().contains("NotStarted")); assert!(err.to_string().contains("sandbox not started")); - assert!(expect_ok(DaemonResponse::Pong).is_err()); + assert_eq!( + expect_ok(DaemonResponse::Pong).unwrap_err().to_string(), + "unexpected reply (expected ok): Pong" + ); } struct ScriptedInput(F); @@ -1131,6 +1148,7 @@ mod tests { let err = DaemonError::Daemon { kind: ErrKind::Busy, message: "single-flight slot held".to_string(), + api_failure: None, }; let text = err.to_string(); assert!(text.contains("Busy")); diff --git a/src/mxc-sdk/src/backends/wslc/common/daemon_protocol.rs b/src/mxc-sdk/src/backends/wslc/common/daemon_protocol.rs index 0a2a051517..c790e01c21 100644 --- a/src/mxc-sdk/src/backends/wslc/common/daemon_protocol.rs +++ b/src/mxc-sdk/src/backends/wslc/common/daemon_protocol.rs @@ -29,6 +29,7 @@ use serde::de::DeserializeOwned; use serde::ser::Error as _; use serde::{Deserialize, Deserializer, Serialize}; +use crate::mxc_common::mxc_error::ApiFailure; use crate::wslc_common::process_env::EnvScope; /// Upper bound on a single decoded frame (16 MiB). Guards the decoder against a @@ -53,7 +54,7 @@ pub const MAX_EXEC_ID_BYTES: usize = 128; /// /// Changing this strands whatever a running daemon is holding, since the SDK /// offers no way to re-attach to a live session from another process. -pub const PROTOCOL_VERSION: u32 = 9; +pub const PROTOCOL_VERSION: u32 = 10; /// Largest payload one [`StreamFrame::Stdin`] may carry. pub const MAX_STDIN_CHUNK_BYTES: usize = 64 * 1024; @@ -315,7 +316,12 @@ pub enum DaemonResponse { Pong, /// The request failed or was refused. `kind` is a stable machine-readable /// token (see [`ErrKind`]); `message` is human-readable detail. - Err { kind: ErrKind, message: String }, + Err { + kind: ErrKind, + message: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + api_failure: Option, + }, } /// Stable, machine-readable classification of a [`DaemonResponse::Err`], so the @@ -355,6 +361,31 @@ pub enum ErrKind { Unknown, } +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct DaemonApiFailure { + pub operation: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub native_code: Option, +} + +impl From<&ApiFailure> for DaemonApiFailure { + fn from(failure: &ApiFailure) -> Self { + Self { + operation: failure.operation.clone(), + native_code: failure.native_code.clone(), + } + } +} + +impl From for ApiFailure { + fn from(failure: DaemonApiFailure) -> Self { + Self { + operation: failure.operation, + native_code: failure.native_code, + } + } +} + // --------------------------------------------------------------------------- // Exec data-phase stream frames // --------------------------------------------------------------------------- @@ -402,7 +433,11 @@ pub enum StreamFrame { /// Daemon→client: live output was dropped; the terminal frame still follows. Truncated, /// Daemon→client: terminal frame; the exec failed before or during the run. - Error { message: String }, + Error { + message: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + api_failure: Option, + }, } /// serde adapter that (de)serializes a `Vec` as a base64 string rather than @@ -616,8 +651,18 @@ mod tests { roundtrip(DaemonResponse::Err { kind, message: "detail".to_string(), + api_failure: None, }); } + + roundtrip(DaemonResponse::Err { + kind: ErrKind::Backend, + message: "start failed".to_string(), + api_failure: Some(DaemonApiFailure { + operation: "WslcStartContainer".to_string(), + native_code: Some("0x80072740".to_string()), + }), + }); } #[test] @@ -703,6 +748,7 @@ mod tests { DaemonResponse::Err { kind: ErrKind::Unknown, message: "detail".to_string(), + api_failure: None, } ); } @@ -762,9 +808,33 @@ mod tests { roundtrip(StreamFrame::Truncated); roundtrip(StreamFrame::Error { message: "spawn failed".to_string(), + api_failure: Some(DaemonApiFailure { + operation: "WslcCreateContainerProcess".to_string(), + native_code: Some("0x80004005".to_string()), + }), }); } + #[test] + fn an_api_failure_travels_under_snake_case_keys() { + let encoded = serde_json::to_value(StreamFrame::Error { + message: "spawn failed".to_string(), + api_failure: Some(DaemonApiFailure { + operation: "WslcCreateContainerProcess".to_string(), + native_code: Some("0x80004005".to_string()), + }), + }) + .expect("encode"); + + assert_eq!( + encoded["api_failure"], + serde_json::json!({ + "operation": "WslcCreateContainerProcess", + "native_code": "0x80004005" + }) + ); + } + #[test] fn stream_frame_bytes_roundtrip_including_non_utf8() { // Arbitrary bytes (incl. non-UTF8, NUL, 0xFF) must survive the base64 diff --git a/src/mxc-sdk/src/backends/wslc/common/error.rs b/src/mxc-sdk/src/backends/wslc/common/error.rs index 4e284b6762..d64e92c3c3 100644 --- a/src/mxc-sdk/src/backends/wslc/common/error.rs +++ b/src/mxc-sdk/src/backends/wslc/common/error.rs @@ -17,9 +17,48 @@ use std::fmt; use crate::mxc_common::models::{FailurePhase, ScriptResponse}; +use crate::mxc_common::mxc_error::ApiFailure; use crate::wslc_common::wslc_bindings::HRESULT; +#[derive(Debug)] +pub struct WslcStepError { + pub failure_phase: FailurePhase, + pub error_message: String, + pub api_failure: Option>, +} + +impl WslcStepError { + pub fn with_api_failure(mut self, failure: ApiFailure) -> Self { + self.api_failure = Some(Box::new(failure)); + self + } + + pub fn into_response(self) -> ScriptResponse { + ScriptResponse::failed(self.failure_phase, &self.error_message) + } +} + +impl From for WslcStepError { + fn from(response: ScriptResponse) -> Self { + Self { + failure_phase: response.failure_phase, + error_message: response.error_message, + api_failure: None, + } + } +} + +impl From for ScriptResponse { + fn from(error: WslcStepError) -> Self { + error.into_response() + } +} + +pub(crate) fn sdk_api_failure(operation: &str, hr: HRESULT) -> ApiFailure { + ApiFailure::new(operation).with_native_code(format!("{:#010x}", hr as u32)) +} + /// A WSLc backend failure, tagged with the phase it is attributable to. #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) enum WslcError { @@ -66,6 +105,14 @@ impl WslcError { } } + pub(crate) fn into_step_error(self) -> WslcStepError { + WslcStepError { + failure_phase: self.failure_phase(), + error_message: self.to_string(), + api_failure: None, + } + } + /// Render as a [`ScriptResponse`] carrying the message verbatim. pub(crate) fn into_response(self) -> ScriptResponse { ScriptResponse::failed(self.failure_phase(), &self.to_string()) @@ -101,6 +148,12 @@ impl From for ScriptResponse { } } +impl From for WslcStepError { + fn from(err: WslcError) -> Self { + err.into_step_error() + } +} + #[cfg(test)] mod tests { use super::*; @@ -158,6 +211,21 @@ mod tests { assert_eq!(typed.failure_phase, FailurePhase::LaunchFailed); } + #[test] + fn a_step_error_becomes_the_response_create_and_run_returns() { + let hr = 0x8007_2740_u32 as HRESULT; + let error = WslcError::sdk("WslcStartContainer failed", hr, "collision"); + let create_and_run = error.clone().into_response(); + let step = error + .into_step_error() + .with_api_failure(sdk_api_failure("WslcStartContainer", hr)); + + assert_eq!( + serde_json::to_value(step.into_response()).unwrap(), + serde_json::to_value(create_and_run).unwrap() + ); + } + /// A WSLc rejection has to cost the caller the same exit code a rejection /// from any other backend does. #[test] diff --git a/src/mxc-sdk/src/backends/wslc/common/image.rs b/src/mxc-sdk/src/backends/wslc/common/image.rs index ba4ba4fe07..f6ebfafeda 100644 --- a/src/mxc-sdk/src/backends/wslc/common/image.rs +++ b/src/mxc-sdk/src/backends/wslc/common/image.rs @@ -10,13 +10,14 @@ use std::sync::Arc; use std::time::{Duration, Instant}; use crate::mxc_common::logger::Logger; -use crate::mxc_common::models::ScriptResponse; +use crate::mxc_common::mxc_error::ApiFailure; use crate::mxc_common::string_util::{to_wide, CoTaskMemPWSTR}; use crate::wslc_common::container_steps::{ - self, cstr_bytes, sdk_error, validate_store_access, StoredImage, + self, cstr_bytes, sdk_step_error, sdk_step_error_with_context, validate_store_access, + StoredImage, }; -use crate::wslc_common::error::WslcError; +use crate::wslc_common::error::{sdk_api_failure, WslcError, WslcStepError}; use crate::wslc_common::registry_policy; use crate::wslc_common::sdk_init; use crate::wslc_common::store_slots; @@ -153,7 +154,7 @@ pub unsafe fn pull_image( store: &StoreHint, log_prefix: &str, logger: &mut Logger, -) -> Result<(), ScriptResponse> { +) -> Result<(), WslcStepError> { let budget = pull_budget(); let _ = writeln!( logger, @@ -263,9 +264,9 @@ pub unsafe fn start_pull( store: &StoreHint, log_prefix: &str, on_done: F, -) -> Result<(), ScriptResponse> +) -> Result<(), WslcStepError> where - F: FnOnce(Result<(), ScriptResponse>) + Send + 'static, + F: FnOnce(Result<(), WslcStepError>) + Send + 'static, { // SAFETY: forwarded unchanged; this shares `start_pull_counted`'s contract. unsafe { start_pull_counted(sdk, session, image, store, log_prefix, None, on_done) } @@ -292,9 +293,9 @@ pub unsafe fn start_pull_counted( log_prefix: &str, counter: Option>, on_done: F, -) -> Result<(), ScriptResponse> +) -> Result<(), WslcStepError> where - F: FnOnce(Result<(), ScriptResponse>) + Send + 'static, + F: FnOnce(Result<(), WslcStepError>) + Send + 'static, { let job = PullJob { sdk: sdk as *const WslcSdk, @@ -344,7 +345,7 @@ where return Err(WslcError::Host(format!( "could not start a thread to pull WSLC image '{image}': {e}" )) - .into_response()); + .into_step_error()); } Ok(()) } @@ -369,13 +370,14 @@ pub fn wait_for_pulls_in_flight(budget: Duration) -> bool { struct PullApartment; impl PullApartment { - fn enter() -> Result { + fn enter() -> Result { use windows::Win32::System::Com::{CoInitializeEx, COINIT_MULTITHREADED}; // SAFETY: called once on a freshly spawned thread; `Drop` below pairs it. let hr = unsafe { CoInitializeEx(None, COINIT_MULTITHREADED) }; if hr.is_err() { - return Err(sdk_error( + return Err(sdk_step_error_with_context( + "CoInitializeEx", "could not join the COM apartment for a WSLC pull", hr.0, "", @@ -395,7 +397,7 @@ impl Drop for PullApartment { } /// Report a pull the caller stopped waiting for. -pub fn pull_deadline_expired(image: &str, budget: Duration) -> ScriptResponse { +pub fn pull_deadline_expired(image: &str, budget: Duration) -> WslcStepError { WslcError::Host(format!( "WSLC image '{}' did not finish pulling within {}s. The transfer was \ abandoned and this sandbox was not created. Retry, raise {} (a \ @@ -406,7 +408,8 @@ pub fn pull_deadline_expired(image: &str, budget: Duration) -> ScriptResponse { PULL_TIMEOUT_ENV, crate::wslc_common::daemon_client::CALL_TIMEOUT_ENV, )) - .into_response() + .into_step_error() + .with_api_failure(ApiFailure::new("WslcPullSessionImage")) } /// Classify a `WslcPullSessionImage` failure and render it for the caller. @@ -416,7 +419,7 @@ fn pull_failure( hr: HRESULT, sdk_msg: &str, budget: Duration, -) -> ScriptResponse { +) -> WslcStepError { let sanitized = sanitize_sdk_message(sdk_msg); let detail = if sanitized.is_empty() { format!("HRESULT 0x{:08X}", hr as u32) @@ -464,7 +467,8 @@ fn pull_failure( store.warm_instruction(), )), } - .into_response() + .into_step_error() + .with_api_failure(sdk_api_failure("WslcPullSessionImage", hr)) } /// Longest run of registry-supplied text kept in a user-facing message. @@ -606,14 +610,13 @@ fn detect_tar_format(path: &str) -> std::io::Result { /// /// Supports both rootfs tars (`docker export`) and Docker image archives /// (`docker save`). The format is auto-detected via `detect_tar_format`. -/// Returns `Ok(())` on success or `Err(ScriptResponse)` on failure. pub(crate) unsafe fn import_image_from_tar( sdk: &WslcSdk, session: WslcSession, image_name: &str, tar_path: &str, logger: &mut Logger, -) -> Result<(), ScriptResponse> { +) -> Result<(), WslcStepError> { let path = std::path::Path::new(tar_path); if !path.exists() { return Err(WslcError::Rejected(format!( @@ -621,7 +624,7 @@ pub(crate) unsafe fn import_image_from_tar( (via 'docker export') or Docker image archive (via 'docker save').", tar_path )) - .into_response()); + .into_step_error()); } // Resolve to absolute path, following symlinks. Fall back to the @@ -636,7 +639,7 @@ pub(crate) unsafe fn import_image_from_tar( "Failed to read tar file '{}': {}", tar_path, e )) - .into_response()); + .into_step_error()); } }; let wide_path: Vec = to_wide(&tar_path); @@ -661,7 +664,8 @@ pub(crate) unsafe fn import_image_from_tar( ); if hr != S_OK { let msg = err_msg.to_string_lossy(); - return Err(sdk_error( + return Err(sdk_step_error_with_context( + "WslcLoadSessionImageFromFile", &format!("Failed to load Docker image archive from '{}'", tar_path), hr, &msg, @@ -699,7 +703,8 @@ pub(crate) unsafe fn import_image_from_tar( ); if hr != S_OK { let msg = err_msg.to_string_lossy(); - return Err(sdk_error( + return Err(sdk_step_error_with_context( + "WslcImportSessionImageFromFile", &format!("Failed to import image '{}' from tar", image_name), hr, &msg, @@ -717,7 +722,7 @@ pub(crate) unsafe fn import_image_from_tar( (via 'docker export') or a Docker image archive (via 'docker save').", tar_path )) - .into_response()); + .into_step_error()); } } @@ -762,7 +767,7 @@ pub unsafe fn begin_resolve( registry: RegistryAccess, log_prefix: &str, logger: &mut Logger, -) -> Result { +) -> Result { let cached = list_cached_images(sdk, session)?; let matched = cached.iter().find(|entry| entry.satisfies(image)); let policy = registry_policy::get_policy(); @@ -803,13 +808,13 @@ pub unsafe fn begin_resolve( } ImageAction::Pull => Ok(ImageStep::Pull), ImageAction::RefuseByPolicy => { - Err(WslcError::Rejected(policy.refusal(image)).into_response()) + Err(WslcError::Rejected(policy.refusal(image)).into_step_error()) } ImageAction::RefuseNoEgress => Err(no_egress_refusal(image, store)), } } -fn no_egress_refusal(image: &str, store: &StoreHint) -> ScriptResponse { +fn no_egress_refusal(image: &str, store: &StoreHint) -> WslcStepError { if let StoreHint::DaemonSlot(index) = store { return WslcError::Rejected(format!( "WSLC isolated provision refused: daemon image store slot {index} at '{}' is cold \ @@ -819,7 +824,7 @@ fn no_egress_refusal(image: &str, store: &StoreHint) -> ScriptResponse { store.store_path(), store.warm_instruction(), )) - .into_response(); + .into_step_error(); } WslcError::Rejected(format!( "WSLC image '{}' is not cached, and this sandbox declares no egress. \ @@ -832,7 +837,7 @@ fn no_egress_refusal(image: &str, store: &StoreHint) -> ScriptResponse { image, store.warm_instruction(), )) - .into_response() + .into_step_error() } /// Record which content a completed pull produced. @@ -849,7 +854,7 @@ pub unsafe fn report_pulled_digest( image: &str, log_prefix: &str, logger: &mut Logger, -) -> Result<(), ScriptResponse> { +) -> Result<(), WslcStepError> { if let Some(entry) = list_cached_images(sdk, session)? .iter() .find(|entry| entry.satisfies(image)) @@ -886,7 +891,7 @@ pub unsafe fn resolve_image( registry: RegistryAccess, log_prefix: &str, logger: &mut Logger, -) -> Result<(), ScriptResponse> { +) -> Result<(), WslcStepError> { match begin_resolve( sdk, session, @@ -953,9 +958,9 @@ fn digest_of(reference: &str) -> Option<&str> { unsafe fn list_cached_images( sdk: &WslcSdk, session: WslcSession, -) -> Result, ScriptResponse> { +) -> Result, WslcStepError> { container_steps::list_stored_images(sdk, session) - .map_err(|hr| sdk_error("WslcListSessionImages failed", hr, "")) + .map_err(|hr| sdk_step_error("WslcListSessionImages", hr, "")) } /// How `resolve_image` satisfies the request once the cache has been consulted. @@ -1203,7 +1208,7 @@ mod tests { const E_FAIL: HRESULT = -2147467259; /// `pull_failure` at the production budget, which most cases do not care about. - fn failure(image: &str, store: &StoreHint, hr: HRESULT, sdk_msg: &str) -> ScriptResponse { + fn failure(image: &str, store: &StoreHint, hr: HRESULT, sdk_msg: &str) -> WslcStepError { pull_failure(image, store, hr, sdk_msg, PULL_TIMEOUT) } @@ -1377,6 +1382,9 @@ mod tests { "a state-aware operator needs the coupled deadline: {}", resp.error_message ); + let api_failure = resp.api_failure.as_deref().expect("pull operation"); + assert_eq!(api_failure.operation, "WslcPullSessionImage"); + assert!(api_failure.native_code.is_none()); } #[test] @@ -1539,6 +1547,9 @@ mod tests { "denied", ); assert_eq!(resp.failure_phase, FailurePhase::Rejected); + let api_failure = resp.api_failure.as_deref().expect("pull operation"); + assert_eq!(api_failure.operation, "WslcPullSessionImage"); + assert_eq!(api_failure.native_code.as_deref(), Some("0x80040601")); } #[test] @@ -1720,7 +1731,7 @@ mod tests { #[test] fn an_isolated_provision_names_the_selected_cold_store_and_how_to_warm_it() { let refusal = no_egress_refusal("alpine:latest", &StoreHint::DaemonSlot(1)); - let message = refusal.error_message; + let message = &refusal.error_message; assert_eq!( refusal.failure_phase, crate::mxc_common::models::FailurePhase::Rejected diff --git a/src/mxc-sdk/src/backends/wslc/common/state_aware.rs b/src/mxc-sdk/src/backends/wslc/common/state_aware.rs index f30d0616ba..1176ebbbc1 100644 --- a/src/mxc-sdk/src/backends/wslc/common/state_aware.rs +++ b/src/mxc-sdk/src/backends/wslc/common/state_aware.rs @@ -765,17 +765,27 @@ fn validate_session_sizing(config: Option<&WslcProvisionSessionConfig>) -> Resul /// Map a typed [`DaemonError`] onto the matching wire-format [`MxcError`] code. fn map_daemon_error(err: DaemonError) -> MxcError { match err { - DaemonError::Daemon { kind, message } => match kind { - ErrKind::NotProvisioned => MxcError::not_provisioned(message), - ErrKind::NotStarted => MxcError::not_started(message), - ErrKind::Unavailable => MxcError::backend_unavailable(message), - ErrKind::Rejected => MxcError::policy_validation(message), - ErrKind::Busy - | ErrKind::NotReady - | ErrKind::Protocol - | ErrKind::Backend - | ErrKind::Unknown => MxcError::backend_error(message), - }, + DaemonError::Daemon { + kind, + message, + api_failure, + } => { + let error = match kind { + ErrKind::NotProvisioned => MxcError::not_provisioned(message), + ErrKind::NotStarted => MxcError::not_started(message), + ErrKind::Unavailable => MxcError::backend_unavailable(message), + ErrKind::Rejected => MxcError::policy_validation(message), + ErrKind::Busy + | ErrKind::NotReady + | ErrKind::Protocol + | ErrKind::Backend + | ErrKind::Unknown => MxcError::backend_error(message), + }; + match api_failure { + Some(api_failure) => error.with_api_failure(api_failure.into()), + None => error, + } + } DaemonError::Transport(e) => MxcError::backend_error(format!("{e:#}")), } } @@ -939,7 +949,9 @@ fn split_env(env: &[String]) -> Vec<(String, String)> { mod stdin_relay_tests { use super::*; use crate::wslc_common::daemon_client::test_daemon::TestDaemon; - use crate::wslc_common::daemon_protocol::{DaemonRequest, DaemonResponse, StreamFrame}; + use crate::wslc_common::daemon_protocol::{ + DaemonApiFailure, DaemonRequest, DaemonResponse, StreamFrame, + }; use std::os::windows::io::BorrowedHandle; use std::time::Duration; @@ -1021,6 +1033,59 @@ mod stdin_relay_tests { assert_eq!(running.join().unwrap().unwrap(), ExecOutcome::Exited(0)); } + fn process_launch_failure() -> StreamFrame { + StreamFrame::Error { + message: "process launch failed".to_string(), + api_failure: Some(DaemonApiFailure { + operation: "WslcCreateContainerProcess".to_string(), + native_code: Some("0x80004005".to_string()), + }), + } + } + + fn assert_process_launch_failure(error: &MxcError) { + assert_eq!( + error.code, + crate::mxc_common::mxc_error::MxcErrorCode::BackendError + ); + assert_eq!(error.message, "exec failed: process launch failed"); + assert_eq!(error.operation(), Some("WslcCreateContainerProcess")); + assert_eq!(error.native_code(), Some("0x80004005")); + } + + #[test] + fn a_relayed_exec_preserves_structured_daemon_failure() { + let daemon = TestDaemon::start(); + let client = daemon.client.clone(); + let running = std::thread::spawn(move || { + relay_exec(client, config(), None::).unwrap_err() + }); + + let mut exec = daemon.next(); + exec.reply(&DaemonResponse::Ok); + exec.reply(&process_launch_failure()); + + assert_process_launch_failure(&running.join().unwrap()); + } + + #[test] + fn a_piped_exec_preserves_structured_daemon_failure() { + let daemon = TestDaemon::start(); + let handle = exec_piped( + daemon.client.clone(), + config(), + "exec-1".to_string(), + "run-1".to_string(), + false, + ) + .unwrap(); + let mut exec = daemon.next(); + exec.reply(&DaemonResponse::Ok); + exec.reply(&process_launch_failure()); + + assert_process_launch_failure(&within("the waiter", handle.waiter).unwrap_err()); + } + #[test] fn a_piped_exec_without_stdin_gives_the_command_end_of_file() { let daemon = TestDaemon::start(); @@ -1995,6 +2060,7 @@ mod tests { let err = map_daemon_error(DaemonError::Daemon { kind: ErrKind::NotProvisioned, message: "unknown sandbox".to_string(), + api_failure: None, }); assert_eq!( err.code, @@ -2007,6 +2073,7 @@ mod tests { let err = map_daemon_error(DaemonError::Daemon { kind: ErrKind::NotStarted, message: "not started".to_string(), + api_failure: None, }); assert_eq!( err.code, @@ -2019,6 +2086,7 @@ mod tests { let err = map_daemon_error(DaemonError::Daemon { kind: ErrKind::Busy, message: "busy".to_string(), + api_failure: None, }); assert_eq!( err.code, @@ -2031,6 +2099,7 @@ mod tests { let err = map_daemon_error(DaemonError::Daemon { kind: ErrKind::Unavailable, message: "WSLc components are missing".to_string(), + api_failure: None, }); assert_eq!( err.code, @@ -2043,6 +2112,7 @@ mod tests { let err = map_daemon_error(DaemonError::Daemon { kind: ErrKind::Rejected, message: "network.allowLocalNetwork=true is not supported".to_string(), + api_failure: None, }); assert_eq!( err.code, @@ -2055,10 +2125,26 @@ mod tests { let err = map_daemon_error(DaemonError::Daemon { kind: ErrKind::Unknown, message: "from a newer daemon".to_string(), + api_failure: None, }); assert_eq!( err.code, crate::mxc_common::mxc_error::MxcErrorCode::BackendError ); } + + #[test] + fn map_daemon_error_preserves_api_failure() { + let err = map_daemon_error(DaemonError::Daemon { + kind: ErrKind::Backend, + message: "WslcStartContainer failed with HRESULT 0x80072740: collision".to_string(), + api_failure: Some(crate::wslc_common::daemon_protocol::DaemonApiFailure { + operation: "WslcStartContainer".to_string(), + native_code: Some("0x80072740".to_string()), + }), + }); + + assert_eq!(err.operation(), Some("WslcStartContainer")); + assert_eq!(err.native_code(), Some("0x80072740")); + } } diff --git a/src/mxc-sdk/src/backends/wslc/common/wsl_container_runner.rs b/src/mxc-sdk/src/backends/wslc/common/wsl_container_runner.rs index b154f44742..79287cb6f3 100644 --- a/src/mxc-sdk/src/backends/wslc/common/wsl_container_runner.rs +++ b/src/mxc-sdk/src/backends/wslc/common/wsl_container_runner.rs @@ -972,7 +972,7 @@ impl WSLContainerRunner { if !image::wait_for_pulls_in_flight(PULL_DRAIN_TIMEOUT) { std::mem::forget(session_guard); } - return Err(e); + return Err(e.into_response()); } // -- Process settings -- diff --git a/src/mxc-sdk/src/bin/wslc_daemon/control_server.rs b/src/mxc-sdk/src/bin/wslc_daemon/control_server.rs index 81cc5cafd5..793a6addad 100644 --- a/src/mxc-sdk/src/bin/wslc_daemon/control_server.rs +++ b/src/mxc-sdk/src/bin/wslc_daemon/control_server.rs @@ -42,8 +42,9 @@ use windows::Win32::System::Threading::{GetCurrentProcess, OpenProcessToken}; use mxc_sdk::wslc_common::container_steps::OutStream; use mxc_sdk::wslc_common::daemon_protocol::{ - encode_frame, DaemonRequest, DaemonResponse, DeprovisionConfig, DeprovisionSessionConfig, - ExecTerminal, StreamFrame, MAX_EXEC_ID_BYTES, MAX_FRAME_SIZE, MAX_STDIN_CHUNK_BYTES, + encode_frame, DaemonApiFailure, DaemonRequest, DaemonResponse, DeprovisionConfig, + DeprovisionSessionConfig, ExecTerminal, StreamFrame, MAX_EXEC_ID_BYTES, MAX_FRAME_SIZE, + MAX_STDIN_CHUNK_BYTES, }; use crate::session_manager::{ExecSlotGuard, ExecStream, SessionHandle, StdinRelay, WorkerError}; @@ -575,6 +576,7 @@ where &DaemonResponse::Err { kind: mxc_sdk::wslc_common::daemon_protocol::ErrKind::Busy, message: "WSLc daemon client capacity is exhausted".to_string(), + api_failure: None, }, ) .await?; @@ -629,6 +631,7 @@ where &DaemonResponse::Err { kind: mxc_sdk::wslc_common::daemon_protocol::ErrKind::Busy, message: "WSLc daemon exec capacity is exhausted".to_string(), + api_failure: None, }, ) .await?; @@ -905,9 +908,11 @@ fn exit_terminal( Ok(Ok(ExecTerminal::Cancelled)) => StreamFrame::Cancelled, Ok(Err(e)) => StreamFrame::Error { message: e.to_string(), + api_failure: e.api_failure().map(DaemonApiFailure::from), }, Err(_) => StreamFrame::Error { message: "WSLc worker dropped the exec reply channel".to_string(), + api_failure: None, }, } } @@ -925,6 +930,7 @@ fn worker_err_response(e: WorkerError) -> DaemonResponse { DaemonResponse::Err { kind: e.kind(), message: e.to_string(), + api_failure: e.api_failure().map(DaemonApiFailure::from), } } @@ -979,6 +985,49 @@ mod tests { ) } + fn sdk_worker_error() -> WorkerError { + WorkerError::Step { + kind: ErrKind::Backend, + error: anyhow::anyhow!("start failed"), + api_failure: Some(Box::new( + mxc_sdk::mxc_common::mxc_error::ApiFailure::new("WslcStartContainer") + .with_native_code("0x80072740"), + )), + } + } + + #[test] + fn worker_error_response_carries_api_failure() { + match worker_err_response(sdk_worker_error()) { + DaemonResponse::Err { + kind, + message, + api_failure: Some(api_failure), + } => { + assert_eq!(kind, ErrKind::Backend); + assert_eq!(message, "start failed"); + assert_eq!(api_failure.operation, "WslcStartContainer"); + assert_eq!(api_failure.native_code.as_deref(), Some("0x80072740")); + } + other => panic!("unexpected response: {other:?}"), + } + } + + #[test] + fn exec_error_frame_carries_api_failure() { + match exit_terminal(Ok(Err(sdk_worker_error()))) { + StreamFrame::Error { + message, + api_failure: Some(api_failure), + } => { + assert_eq!(message, "start failed"); + assert_eq!(api_failure.operation, "WslcStartContainer"); + assert_eq!(api_failure.native_code.as_deref(), Some("0x80072740")); + } + other => panic!("unexpected frame: {other:?}"), + } + } + /// A streaming exec's output memory is its live-output queue alone, so the /// cap is what holds the daemon's worst case down. #[test] @@ -1068,6 +1117,7 @@ mod tests { DaemonResponse::Err { kind: ErrKind::Busy, message: "WSLc daemon client capacity is exhausted".to_string(), + api_failure: None, } ); session.shutdown().await.unwrap(); @@ -1106,6 +1156,7 @@ mod tests { DaemonResponse::Err { kind: ErrKind::Busy, message: "WSLc daemon exec capacity is exhausted".to_string(), + api_failure: None, } ); session.shutdown().await.unwrap(); @@ -1416,6 +1467,7 @@ mod tests { DaemonResponse::Err { kind: ErrKind::Busy, message: "WSLc daemon client capacity is exhausted".to_string(), + api_failure: None, } ); clients.shutdown().await; @@ -1702,6 +1754,7 @@ mod tests { let resp = DaemonResponse::Err { kind: ErrKind::Backend, message: "provision failed".to_string(), + api_failure: None, }; let outcome = deliver_minted_reply(&mut BrokenPipe, &resp, |id| { @@ -2272,7 +2325,7 @@ mod tests { let resp: DaemonResponse = read_frame(&mut client).await.unwrap(); match resp { - DaemonResponse::Err { kind, message } => { + DaemonResponse::Err { kind, message, .. } => { assert_eq!(kind, ErrKind::NotProvisioned); assert!(message.contains("wslc:nope"), "message was {message:?}"); } @@ -2299,6 +2352,7 @@ mod tests { DaemonResponse::Err { kind: ErrKind::NotStarted, message: "sandbox wslc:cold is not started".to_string(), + api_failure: None, } ); } @@ -2320,7 +2374,11 @@ mod tests { assert_eq!(admit, DaemonResponse::Ok); let terminal: StreamFrame = read_frame(&mut client).await.unwrap(); match terminal { - StreamFrame::Error { message } => { + StreamFrame::Error { + message, + api_failure, + } => { + assert!(api_failure.is_none()); assert!( message.contains("dropped the exec reply channel"), "message was {message:?}" diff --git a/src/mxc-sdk/src/bin/wslc_daemon/session_manager.rs b/src/mxc-sdk/src/bin/wslc_daemon/session_manager.rs index 807f853387..e071de9b59 100644 --- a/src/mxc-sdk/src/bin/wslc_daemon/session_manager.rs +++ b/src/mxc-sdk/src/bin/wslc_daemon/session_manager.rs @@ -49,7 +49,10 @@ use tokio::sync::mpsc::error::TrySendError; use tokio::sync::{mpsc, oneshot}; use mxc_sdk::mxc_common::logger::{Logger, Mode}; -use mxc_sdk::mxc_common::models::{FailurePhase, ScriptResponse}; +use mxc_sdk::mxc_common::models::FailurePhase; +#[cfg(test)] +use mxc_sdk::mxc_common::models::ScriptResponse; +use mxc_sdk::mxc_common::mxc_error::ApiFailure; use mxc_sdk::wslc_common::container_steps::{ self, OutStream, OutputSink, ProcessSettings, SessionSizing, StdinConsumer, }; @@ -57,6 +60,7 @@ use mxc_sdk::wslc_common::daemon_protocol::{ DeprovisionConfig, DeprovisionSessionConfig, ErrKind, ExecConfig, ExecTerminal, NetworkMode, PortMapping, ProvisionConfig, ProvisionSessionConfig, StartConfig, StopConfig, }; +use mxc_sdk::wslc_common::error::WslcStepError; use mxc_sdk::wslc_common::image; use mxc_sdk::wslc_common::policy_mapping; use mxc_sdk::wslc_common::process_env::EnvScope; @@ -154,14 +158,22 @@ fn wait_for_in_flight(in_flight: &AtomicUsize, deadline: Instant) -> bool { true } -/// Convert a step helper's `failure_phase` into a typed failure, since the wire -/// carries only a message and an [`ErrKind`]. -fn sr_err(resp: ScriptResponse) -> WorkerError { - let err = anyhow::anyhow!(resp.error_message); - match resp.failure_phase { - FailurePhase::BackendUnavailable => WorkerError::Unavailable(err), - FailurePhase::Rejected => WorkerError::Rejected(err), - _ => WorkerError::Backend(err), +/// Convert a step helper's `failure_phase` into a typed failure. +fn sr_err(resp: impl Into) -> WorkerError { + let WslcStepError { + failure_phase, + error_message, + api_failure, + } = resp.into(); + let kind = match failure_phase { + FailurePhase::BackendUnavailable => ErrKind::Unavailable, + FailurePhase::Rejected => ErrKind::Rejected, + _ => ErrKind::Backend, + }; + WorkerError::Step { + kind, + error: anyhow::anyhow!(error_message), + api_failure, } } @@ -182,14 +194,17 @@ pub enum WorkerError { /// An exec already holds this container's single-flight slot. Busy(String), - /// The host cannot run WSLc at all. - Unavailable(anyhow::Error), - /// The request cannot be honored as written. Rejected(anyhow::Error), - /// A backend/SDK-level failure, or an internal worker/channel fault. + /// A backend-level failure, or an internal worker/channel fault. Backend(anyhow::Error), + + Step { + kind: ErrKind, + error: anyhow::Error, + api_failure: Option>, + }, } impl WorkerError { @@ -200,9 +215,27 @@ impl WorkerError { WorkerError::UnknownSession(_) => ErrKind::NotProvisioned, WorkerError::NotStarted(_) => ErrKind::NotStarted, WorkerError::Busy(_) => ErrKind::Busy, - WorkerError::Unavailable(_) => ErrKind::Unavailable, WorkerError::Rejected(_) => ErrKind::Rejected, WorkerError::Backend(_) => ErrKind::Backend, + WorkerError::Step { kind, .. } => *kind, + } + } + + pub fn api_failure(&self) -> Option<&ApiFailure> { + match self { + WorkerError::Step { api_failure, .. } => api_failure.as_deref(), + _ => None, + } + } + + fn backend_with_api_failure( + error: anyhow::Error, + api_failure: Option>, + ) -> Self { + Self::Step { + kind: ErrKind::Backend, + error, + api_failure, } } } @@ -214,9 +247,8 @@ impl std::fmt::Display for WorkerError { WorkerError::UnknownSession(id) => write!(f, "unknown session {id}"), WorkerError::NotStarted(id) => write!(f, "sandbox {id} is not started"), WorkerError::Busy(id) => write!(f, "sandbox {id} already has an exec in flight"), - WorkerError::Unavailable(e) | WorkerError::Rejected(e) | WorkerError::Backend(e) => { - write!(f, "{e:#}") - } + WorkerError::Rejected(e) | WorkerError::Backend(e) => write!(f, "{e:#}"), + WorkerError::Step { error, .. } => write!(f, "{error:#}"), } } } @@ -265,7 +297,7 @@ pub enum WorkerCommand { /// Report a parked provision's pull, from the pull thread or its deadline. PullFinished { token: u64, - outcome: Result<(), ScriptResponse>, + outcome: Result<(), WslcStepError>, }, /// Report an off-worker exec, from its run thread. ExecFinished { @@ -322,7 +354,11 @@ pub enum ExecReport { Finished(Result), /// The detail, and the process's stdin if it is still open. - Unconfirmed(String, Option), + Unconfirmed { + detail: String, + api_failure: Option>, + held_stdin: Option, + }, } /// Everything an off-worker exec needs, in a form that can cross a thread. @@ -391,9 +427,12 @@ fn start_exec( // A panic leaves the process's fate unknown, which is what // `Unconfirmed` already means. - let report = std::panic::catch_unwind(run).unwrap_or_else(|_| { - ExecReport::Unconfirmed("the exec thread panicked".to_string(), None) - }); + let report = + std::panic::catch_unwind(run).unwrap_or_else(|_| ExecReport::Unconfirmed { + detail: "the exec thread panicked".to_string(), + api_failure: None, + held_stdin: None, + }); let _ = worker.send(WorkerCommand::ExecFinished { sandbox_id, report }); }); @@ -476,13 +515,20 @@ fn run_exec(job: ExecJob, logger: &mut Logger) -> ExecReport { }; match outcome.completion { - container_steps::ProcessCompletion::TerminationUnconfirmed => ExecReport::Unconfirmed( - outcome - .post_launch_error - .map(|error| error.error_message) - .unwrap_or_else(|| "process termination could not be confirmed".to_string()), - outcome.held_stdin, - ), + container_steps::ProcessCompletion::TerminationUnconfirmed => { + let (detail, api_failure) = match outcome.post_launch_error { + Some(error) => (error.error_message, error.api_failure), + None => ( + "process termination could not be confirmed".to_string(), + None, + ), + }; + ExecReport::Unconfirmed { + detail, + api_failure, + held_stdin: outcome.held_stdin, + } + } container_steps::ProcessCompletion::Confirmed(terminal) => { match stdin_failure .lock() @@ -1157,7 +1203,7 @@ fn start_session_boot( } .map(BootedSession) .map_err(|failure| BootFailure { - error: sr_err(*failure.response), + error: sr_err(failure.response), store_may_be_held: failure.store_may_be_held, }) } @@ -1692,14 +1738,19 @@ impl Worker { } let message = e.to_string(); + let api_failure = e.api_failure().cloned().map(Box::new); if let Some(reply) = reply { let _ = reply.send(Err(e)); } for (_, parked_reply) in parked { - let _ = parked_reply.send(Err(WorkerError::Backend(anyhow::anyhow!( + let error = anyhow::anyhow!( "the WSLc session this provision was waiting for could not be \ created: {message}" - )))); + ); + let _ = parked_reply.send(Err(WorkerError::backend_with_api_failure( + error, + api_failure.clone(), + ))); } return; } @@ -2232,7 +2283,7 @@ impl Worker { /// Resume a parked provision once its pull reported, or its deadline did. /// /// An unknown token is the loser of that race, and has nothing left to do. - fn finish_provision(&mut self, token: u64, outcome: Result<(), ScriptResponse>) { + fn finish_provision(&mut self, token: u64, outcome: Result<(), WslcStepError>) { let Some(PendingProvision { config, reply, @@ -2512,10 +2563,15 @@ impl Worker { let outcome = match report { ExecReport::Finished(outcome) => outcome, - ExecReport::Unconfirmed(detail, held_stdin) => Err(self.quarantine( + ExecReport::Unconfirmed { + detail, + api_failure, + held_stdin, + } => Err(self.quarantine( sandbox_id, in_flight.container, &detail, + api_failure, in_flight.slot, held_stdin, )), @@ -2539,6 +2595,7 @@ impl Worker { sandbox_id: &str, container: WslcContainer, detail: &str, + api_failure: Option>, slot: Option, held_stdin: Option, ) -> WorkerError { @@ -2555,38 +2612,55 @@ impl Worker { (slot, Some(held_stdin)) => Some(Arc::new((slot, held_stdin)) as ExecSlotGuard), }; + self.finish_quarantine(sandbox_id, detail, api_failure, slot, delete_result) + } + + fn finish_quarantine( + &mut self, + sandbox_id: &str, + detail: &str, + api_failure: Option>, + slot: Option, + delete_result: Option>, + ) -> WorkerError { match delete_result { Some(Ok(())) => { // Deleting the container ends anything still running inside it, // so this exec stops counting against capacity. drop(slot); self.containers.remove(sandbox_id); - WorkerError::Backend(anyhow::anyhow!( + let error = anyhow::anyhow!( "exec on sandbox {sandbox_id} could not be confirmed terminated ({detail}); \ the container was quarantined and deleted" - )) + ); + WorkerError::backend_with_api_failure(error, api_failure) } Some(Err(delete_error)) => { if let Some(entry) = self.containers.get_mut(sandbox_id) { entry.quarantined = true; entry.exec_slot = slot; } - WorkerError::Backend(anyhow::anyhow!( + let error = anyhow::anyhow!( "exec on sandbox {sandbox_id} could not be confirmed terminated ({detail}); \ quarantine deletion failed and the sandbox remains blocked until \ deprovision succeeds: {}", delete_error.error_message - )) + ); + WorkerError::backend_with_api_failure( + error, + api_failure.or(delete_error.api_failure), + ) } None => { if let Some(entry) = self.containers.get_mut(sandbox_id) { entry.quarantined = true; entry.exec_slot = slot; } - WorkerError::Backend(anyhow::anyhow!( + let error = anyhow::anyhow!( "exec on sandbox {sandbox_id} could not be confirmed terminated ({detail}); \ the sandbox remains quarantined because no active WSLc SDK is available" - )) + ); + WorkerError::backend_with_api_failure(error, api_failure) } } } @@ -3307,13 +3381,13 @@ mod tests { ); } - /// The pull and its deadline race, and both report. - #[test] - fn only_the_first_report_of_a_pull_answers_the_client() { - let mut worker = Worker::new(); - let (reply, mut rx) = oneshot::channel(); + fn park_provision( + worker: &mut Worker, + token: u64, + ) -> oneshot::Receiver> { + let (reply, rx) = oneshot::channel(); worker.pending.insert( - 3, + token, PendingProvision { config: ProvisionConfig { image: "alpine:latest".to_string(), @@ -3328,6 +3402,14 @@ mod tests { _retire_deadline: std::sync::mpsc::channel().0, }, ); + rx + } + + /// The pull and its deadline race, and both report. + #[test] + fn only_the_first_report_of_a_pull_answers_the_client() { + let mut worker = Worker::new(); + let mut rx = park_provision(&mut worker, 3); worker.finish_provision( 3, @@ -3340,6 +3422,37 @@ mod tests { assert_eq!(worker.live_container_count(), 0); } + #[test] + fn a_failed_pull_answers_with_the_call_that_failed() { + let mut worker = Worker::new(); + let mut timed_out = park_provision(&mut worker, 1); + let mut refused = park_provision(&mut worker, 2); + let image_not_found = + ApiFailure::new("WslcPullSessionImage").with_native_code("0x80040601"); + + worker.finish_provision( + 1, + Err(image::pull_deadline_expired("alpine:latest", DRAIN_TIMEOUT)), + ); + worker.finish_provision( + 2, + Err(WslcStepError { + failure_phase: FailurePhase::Rejected, + error_message: "WSLC image 'nope:1' could not be pulled".to_string(), + api_failure: Some(Box::new(image_not_found.clone())), + }), + ); + + let timed_out = timed_out.try_recv().unwrap().unwrap_err(); + assert_eq!( + timed_out.api_failure(), + Some(&ApiFailure::new("WslcPullSessionImage")) + ); + let refused = refused.try_recv().unwrap().unwrap_err(); + assert_eq!(refused.kind(), ErrKind::Rejected); + assert_eq!(refused.api_failure(), Some(&image_not_found)); + } + #[test] fn a_retired_sandbox_stops_holding_the_daemon_open() { let mut worker = Worker::new(); @@ -5027,10 +5140,15 @@ mod tests { worker.attach_session_reply(TEST_SESSION, session_reply); let (reply, mut rx) = oneshot::channel(); worker.begin_provision(test_provision_config(), reply, &tx); + let create_failure = ApiFailure::new("WslcCreateSession").with_native_code("0x800705aa"); worker.finish_session( TEST_SESSION, - Err(WorkerError::Backend(anyhow::anyhow!("ERROR_NO_SYSTEM_RESOURCES")).into()), + Err(WorkerError::backend_with_api_failure( + anyhow::anyhow!("ERROR_NO_SYSTEM_RESOURCES"), + Some(Box::new(create_failure.clone())), + ) + .into()), &tx, ); @@ -5038,10 +5156,14 @@ mod tests { session_rx.try_recv().expect("the session asker").is_err(), "the client that asked for the session must be told it failed" ); - assert!( - rx.try_recv().expect("the parked provision").is_err(), + let parked = rx.try_recv().expect("the parked provision").expect_err( "a provision parked behind a boot that failed must be answered, not \ - left waiting on a session that will never exist" + left waiting on a session that will never exist", + ); + assert_eq!( + parked.api_failure(), + Some(&create_failure), + "the parked provision must name the call that failed the boot" ); assert!( worker.default_session.is_none(), @@ -5963,7 +6085,11 @@ mod tests { worker.finish_exec( "wslc:busy", - ExecReport::Unconfirmed("the exit callback never fired".to_string(), None), + ExecReport::Unconfirmed { + detail: "the exit callback never fired".to_string(), + api_failure: None, + held_stdin: None, + }, &tx, ); @@ -5983,7 +6109,11 @@ mod tests { worker.exec_in_flight.get_mut("wslc:busy").unwrap().slot = Some(Arc::new(permit)); worker.finish_exec( "wslc:busy", - ExecReport::Unconfirmed("the exit callback never fired".to_string(), None), + ExecReport::Unconfirmed { + detail: "the exit callback never fired".to_string(), + api_failure: None, + held_stdin: None, + }, &tx, ); assert_eq!( @@ -6012,15 +6142,102 @@ mod tests { worker.finish_exec( "wslc:busy", - ExecReport::Unconfirmed("the exit callback never fired".to_string(), None), + ExecReport::Unconfirmed { + detail: "the exit callback never fired".to_string(), + api_failure: Some(Box::new( + ApiFailure::new("WslcGetProcessExitEvent").with_native_code("0x80004005"), + )), + held_stdin: None, + }, &tx, ); let err = done.try_recv().unwrap().unwrap_err(); assert!(err.to_string().contains("quarantined"), "got {err}"); + let api_failure = err.api_failure().expect("primary api failure"); + assert_eq!(api_failure.operation, "WslcGetProcessExitEvent"); + assert_eq!(api_failure.native_code.as_deref(), Some("0x80004005")); + assert!(worker.containers["wslc:busy"].quarantined); + } + + fn exit_event_failure() -> ApiFailure { + ApiFailure::new("WslcGetProcessExitEvent").with_native_code("0x80004005") + } + + fn delete_failure() -> WslcStepError { + WslcStepError { + failure_phase: FailurePhase::LaunchFailed, + error_message: "WslcDeleteContainer failed: HRESULT 0x80070005".to_string(), + api_failure: Some(Box::new( + ApiFailure::new("WslcDeleteContainer").with_native_code("0x80070005"), + )), + } + } + + #[test] + fn a_failed_quarantine_delete_keeps_the_exec_failure() { + let mut worker = Worker::new(); + worker + .containers + .insert("wslc:busy".to_string(), started_entry()); + + let err = worker.finish_quarantine( + "wslc:busy", + "the exit event was unavailable", + Some(Box::new(exit_event_failure())), + None, + Some(Err(delete_failure())), + ); + + assert!( + err.to_string().contains("quarantine deletion failed"), + "got {err}" + ); + assert_eq!(err.api_failure(), Some(&exit_event_failure())); assert!(worker.containers["wslc:busy"].quarantined); } + #[test] + fn a_failed_quarantine_delete_names_itself_when_the_exec_named_nothing() { + let mut worker = Worker::new(); + worker + .containers + .insert("wslc:busy".to_string(), started_entry()); + + let err = worker.finish_quarantine( + "wslc:busy", + "the exit callback never fired", + None, + None, + Some(Err(delete_failure())), + ); + + assert_eq!(err.api_failure(), delete_failure().api_failure.as_deref()); + } + + #[test] + fn a_deleted_quarantine_keeps_the_exec_failure() { + let mut worker = Worker::new(); + worker + .containers + .insert("wslc:busy".to_string(), started_entry()); + + let err = worker.finish_quarantine( + "wslc:busy", + "the exit event was unavailable", + Some(Box::new(exit_event_failure())), + None, + Some(Ok(())), + ); + + assert!( + err.to_string().contains("quarantined and deleted"), + "got {err}" + ); + assert_eq!(err.api_failure(), Some(&exit_event_failure())); + assert!(!worker.containers.contains_key("wslc:busy")); + } + /// A report can outlive its slot: shutdown drains the run and clears the /// map before the posted command is serviced. #[test] diff --git a/tests/scripts/run_wslc_state_aware_tests.ps1 b/tests/scripts/run_wslc_state_aware_tests.ps1 index 5940985cab..272d1be406 100644 --- a/tests/scripts/run_wslc_state_aware_tests.ps1 +++ b/tests/scripts/run_wslc_state_aware_tests.ps1 @@ -1338,6 +1338,10 @@ try { $startEnv = Parse-Envelope -Stdout $s.Stdout $code = if ($startEnv) { $startEnv.error.code } else { '' } Assert-True ($code -eq 'backend_error') "the collision surfaced as 'backend_error' (got '$code')" + Assert-True ("$($startEnv.error.operation)" -eq 'WslcStartContainer') ` + "the collision names WslcStartContainer (got '$($startEnv.error.operation)')" + Assert-True ("$($startEnv.error.nativeCode)" -eq '0x80072740') ` + "the collision carries nativeCode 0x80072740 (got '$($startEnv.error.nativeCode)')" Assert-True ("$($startEnv.error.message)" -match '0x80072740') ` "the message carries the address-in-use HRESULT (got '$($startEnv.error.message)')" } else { From 95affa6b768792b1abf532d632fcbc173781b944 Mon Sep 17 00:00:00 2001 From: Soham Das Date: Tue, 10 Nov 2026 18:55:27 -0800 Subject: [PATCH 2/2] [WSLC] Address review feedback on lifecycle error metadata Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../backends/wslc/common/container_steps.rs | 69 ++++- .../src/bin/wslc_daemon/control_server.rs | 13 +- .../src/bin/wslc_daemon/session_manager.rs | 239 +++++++++++++----- tests/scripts/run_wslc_state_aware_tests.ps1 | 14 +- 4 files changed, 246 insertions(+), 89 deletions(-) diff --git a/src/mxc-sdk/src/backends/wslc/common/container_steps.rs b/src/mxc-sdk/src/backends/wslc/common/container_steps.rs index fdf8784ade..7412955819 100644 --- a/src/mxc-sdk/src/backends/wslc/common/container_steps.rs +++ b/src/mxc-sdk/src/backends/wslc/common/container_steps.rs @@ -42,6 +42,7 @@ use std::time::{Duration, Instant}; use crate::mxc_common::logger::Logger; use crate::mxc_common::models::{PortMapping, ScriptResponse}; +use crate::mxc_common::mxc_error::ApiFailure; use crate::mxc_common::string_util::{to_wide, CoTaskMemPWSTR}; use crate::wslc_common::daemon_protocol::ExecTerminal; @@ -1323,6 +1324,23 @@ fn classify_completion(exit_wait: ExitWait, confirmed: bool, exit_code: i32) -> } } +fn unconfirmed_api_failure( + completion: ProcessCompletion, + kill_hr: HRESULT, + exit_code_hr: HRESULT, +) -> Option> { + if completion != ProcessCompletion::TerminationUnconfirmed { + return None; + } + [ + ("WslcSignalProcess", kill_hr), + ("WslcGetProcessExitCode", exit_code_hr), + ] + .into_iter() + .find(|&(_, hr)| hr != S_OK) + .map(|(operation, hr)| Box::new(sdk_api_failure(operation, hr))) +} + /// Result of a daemon `exec`, including captured output retained by callbacks. #[derive(Debug)] pub struct ExecOutcome { @@ -1333,6 +1351,10 @@ pub struct ExecOutcome { /// unobservable. The daemon uses this context when quarantining. pub post_launch_error: Option, + /// For an unconfirmed termination, the first SDK call that failed while + /// stopping the process or reading its exit code. + pub unconfirmed_api_failure: Option>, + /// The process's stdin, still open because its termination was not /// confirmed. pub held_stdin: Option, @@ -1405,16 +1427,18 @@ pub unsafe fn exec_in_container( Err(error) => { // A process holding a stdin nobody writes or closes would never // see end-of-file, so it is killed rather than left to run. - let _ = + let kill_hr = sdk.WslcSignalProcess(process_guard.as_raw(), WslcSignal::WSLC_SIGNAL_SIGKILL); if wait_for_exit_callback(process_settings.io_ctx()) { return Err(error); } + let completion = ProcessCompletion::TerminationUnconfirmed; return Ok(ExecOutcome { - completion: ProcessCompletion::TerminationUnconfirmed, + completion, stdout: Vec::new(), stderr: Vec::new(), post_launch_error: Some(error), + unconfirmed_api_failure: unconfirmed_api_failure(completion, kill_hr, S_OK), held_stdin: None, }); } @@ -1429,6 +1453,7 @@ pub unsafe fn exec_in_container( stdout: Vec::new(), stderr: Vec::new(), post_launch_error: Some(sdk_step_error("WslcGetProcessExitEvent", hr, "")), + unconfirmed_api_failure: None, held_stdin: Some(stdin_guard), }); } @@ -1454,6 +1479,7 @@ pub unsafe fn exec_in_container( ExitWait::Interrupted(interruption) => Some(interruption), _ => None, }; + let mut kill_hr = S_OK; if let Some(interruption) = interruption { if forwards_stdin { stdin_guard.freeze(); @@ -1474,7 +1500,7 @@ pub unsafe fn exec_in_container( ); // Kill only this process; the keepalive init keeps the container up. - let kill_hr = + kill_hr = sdk.WslcSignalProcess(process_guard.as_raw(), WslcSignal::WSLC_SIGNAL_SIGKILL); if kill_hr != S_OK { let _ = writeln!( @@ -1505,16 +1531,16 @@ pub unsafe fn exec_in_container( } let mut exit_code: i32 = -1; - let hr = sdk.WslcGetProcessExitCode(process_guard.as_raw(), &mut exit_code); - if hr != S_OK { + let exit_code_hr = sdk.WslcGetProcessExitCode(process_guard.as_raw(), &mut exit_code); + if exit_code_hr != S_OK { if natural_exit_confirmed(exit_wait, confirmed) { - return Err(sdk_step_error("WslcGetProcessExitCode", hr, "")); + return Err(sdk_step_error("WslcGetProcessExitCode", exit_code_hr, "")); } let _ = writeln!( logger, "[WSLC][daemon] Warning: WslcGetProcessExitCode failed while termination was \ unconfirmed (hr=0x{:08X})", - hr as u32 + exit_code_hr as u32 ); } @@ -1580,6 +1606,7 @@ pub unsafe fn exec_in_container( stdout, stderr, post_launch_error: None, + unconfirmed_api_failure: unconfirmed_api_failure(completion, kill_hr, exit_code_hr), held_stdin, }) } @@ -1993,6 +2020,34 @@ mod tests { ); } + #[test] + fn an_unconfirmed_run_names_its_first_failed_kill_or_exit_code_call() { + let unconfirmed = ProcessCompletion::TerminationUnconfirmed; + let kill_failed = 0x8007_001f_u32 as HRESULT; + let kill_failure = ApiFailure::new("WslcSignalProcess").with_native_code("0x8007001f"); + let exit_code_failure = + ApiFailure::new("WslcGetProcessExitCode").with_native_code("0x80004005"); + + assert_eq!( + unconfirmed_api_failure(unconfirmed, kill_failed, E_FAIL).as_deref(), + Some(&kill_failure) + ); + assert_eq!( + unconfirmed_api_failure(unconfirmed, S_OK, E_FAIL).as_deref(), + Some(&exit_code_failure) + ); + assert_eq!(unconfirmed_api_failure(unconfirmed, S_OK, S_OK), None); + assert_eq!( + unconfirmed_api_failure( + ProcessCompletion::Confirmed(ExecTerminal::TimedOut), + kill_failed, + E_FAIL, + ), + None, + "a confirmed timeout must stay a timeout" + ); + } + /// A streaming caller reads its bytes from the sink, so a second capped /// copy would double the daemon's per-exec output memory. #[test] diff --git a/src/mxc-sdk/src/bin/wslc_daemon/control_server.rs b/src/mxc-sdk/src/bin/wslc_daemon/control_server.rs index 793a6addad..cd15f9d094 100644 --- a/src/mxc-sdk/src/bin/wslc_daemon/control_server.rs +++ b/src/mxc-sdk/src/bin/wslc_daemon/control_server.rs @@ -986,14 +986,13 @@ mod tests { } fn sdk_worker_error() -> WorkerError { - WorkerError::Step { - kind: ErrKind::Backend, - error: anyhow::anyhow!("start failed"), - api_failure: Some(Box::new( + WorkerError::backend_with_api_failure( + anyhow::anyhow!("start failed"), + Some(Box::new( mxc_sdk::mxc_common::mxc_error::ApiFailure::new("WslcStartContainer") .with_native_code("0x80072740"), )), - } + ) } #[test] @@ -1716,7 +1715,7 @@ mod tests { retry_undelivered_release("wslcsess:abc", |_| { attempts.set(attempts.get() + 1); std::future::ready(if attempts.get() < 2 { - Err(WorkerError::Backend(anyhow::anyhow!("busy"))) + Err(WorkerError::backend(anyhow::anyhow!("busy"))) } else { Ok(()) }) @@ -1737,7 +1736,7 @@ mod tests { retry_undelivered_release("wslcsess:abc", |_| { attempts.set(attempts.get() + 1); - std::future::ready(Err(WorkerError::Backend(anyhow::anyhow!("gone")))) + std::future::ready(Err(WorkerError::backend(anyhow::anyhow!("gone")))) }) .await; diff --git a/src/mxc-sdk/src/bin/wslc_daemon/session_manager.rs b/src/mxc-sdk/src/bin/wslc_daemon/session_manager.rs index e071de9b59..28ee82b680 100644 --- a/src/mxc-sdk/src/bin/wslc_daemon/session_manager.rs +++ b/src/mxc-sdk/src/bin/wslc_daemon/session_manager.rs @@ -165,15 +165,14 @@ fn sr_err(resp: impl Into) -> WorkerError { error_message, api_failure, } = resp.into(); - let kind = match failure_phase { - FailurePhase::BackendUnavailable => ErrKind::Unavailable, - FailurePhase::Rejected => ErrKind::Rejected, - _ => ErrKind::Backend, - }; - WorkerError::Step { - kind, + let failure = WorkerFailure { error: anyhow::anyhow!(error_message), api_failure, + }; + match failure_phase { + FailurePhase::BackendUnavailable => WorkerError::Unavailable(failure), + FailurePhase::Rejected => WorkerError::Rejected(failure), + _ => WorkerError::Backend(failure), } } @@ -194,17 +193,20 @@ pub enum WorkerError { /// An exec already holds this container's single-flight slot. Busy(String), + /// The host cannot run WSLc at all. + Unavailable(WorkerFailure), + /// The request cannot be honored as written. - Rejected(anyhow::Error), + Rejected(WorkerFailure), - /// A backend-level failure, or an internal worker/channel fault. - Backend(anyhow::Error), + /// A backend/SDK-level failure, or an internal worker/channel fault. + Backend(WorkerFailure), +} - Step { - kind: ErrKind, - error: anyhow::Error, - api_failure: Option>, - }, +#[derive(Debug)] +pub struct WorkerFailure { + error: anyhow::Error, + api_failure: Option>, } impl WorkerError { @@ -215,28 +217,40 @@ impl WorkerError { WorkerError::UnknownSession(_) => ErrKind::NotProvisioned, WorkerError::NotStarted(_) => ErrKind::NotStarted, WorkerError::Busy(_) => ErrKind::Busy, + WorkerError::Unavailable(_) => ErrKind::Unavailable, WorkerError::Rejected(_) => ErrKind::Rejected, WorkerError::Backend(_) => ErrKind::Backend, - WorkerError::Step { kind, .. } => *kind, } } pub fn api_failure(&self) -> Option<&ApiFailure> { match self { - WorkerError::Step { api_failure, .. } => api_failure.as_deref(), - _ => None, + WorkerError::Unavailable(failure) + | WorkerError::Rejected(failure) + | WorkerError::Backend(failure) => failure.api_failure.as_deref(), + WorkerError::NotProvisioned(_) + | WorkerError::UnknownSession(_) + | WorkerError::NotStarted(_) + | WorkerError::Busy(_) => None, } } - fn backend_with_api_failure( + pub fn rejected(error: anyhow::Error) -> Self { + Self::Rejected(WorkerFailure { + error, + api_failure: None, + }) + } + + pub fn backend(error: anyhow::Error) -> Self { + Self::backend_with_api_failure(error, None) + } + + pub fn backend_with_api_failure( error: anyhow::Error, api_failure: Option>, ) -> Self { - Self::Step { - kind: ErrKind::Backend, - error, - api_failure, - } + Self::Backend(WorkerFailure { error, api_failure }) } } @@ -247,8 +261,9 @@ impl std::fmt::Display for WorkerError { WorkerError::UnknownSession(id) => write!(f, "unknown session {id}"), WorkerError::NotStarted(id) => write!(f, "sandbox {id} is not started"), WorkerError::Busy(id) => write!(f, "sandbox {id} already has an exec in flight"), - WorkerError::Rejected(e) | WorkerError::Backend(e) => write!(f, "{e:#}"), - WorkerError::Step { error, .. } => write!(f, "{error:#}"), + WorkerError::Unavailable(failure) + | WorkerError::Rejected(failure) + | WorkerError::Backend(failure) => write!(f, "{:#}", failure.error), } } } @@ -257,7 +272,7 @@ impl std::error::Error for WorkerError {} impl From for WorkerError { fn from(e: anyhow::Error) -> Self { - WorkerError::Backend(e) + WorkerError::backend(e) } } @@ -418,7 +433,7 @@ fn start_exec( let sandbox_id = job.config.sandbox_id.clone(); let run = std::panic::AssertUnwindSafe(|| match ComApartment::enter() { - Err(e) => ExecReport::Finished(Err(WorkerError::Backend(e))), + Err(e) => ExecReport::Finished(Err(WorkerError::backend(e))), Ok(_apartment) => { let mut log = Logger::new(Mode::Console); run_exec(job, &mut log) @@ -515,20 +530,7 @@ fn run_exec(job: ExecJob, logger: &mut Logger) -> ExecReport { }; match outcome.completion { - container_steps::ProcessCompletion::TerminationUnconfirmed => { - let (detail, api_failure) = match outcome.post_launch_error { - Some(error) => (error.error_message, error.api_failure), - None => ( - "process termination could not be confirmed".to_string(), - None, - ), - }; - ExecReport::Unconfirmed { - detail, - api_failure, - held_stdin: outcome.held_stdin, - } - } + container_steps::ProcessCompletion::TerminationUnconfirmed => report_unconfirmed(outcome), container_steps::ProcessCompletion::Confirmed(terminal) => { match stdin_failure .lock() @@ -536,7 +538,7 @@ fn run_exec(job: ExecJob, logger: &mut Logger) -> ExecReport { .take() { Some(failure) => { - ExecReport::Finished(Err(WorkerError::Backend(anyhow::anyhow!(failure)))) + ExecReport::Finished(Err(WorkerError::backend(anyhow::anyhow!(failure)))) } None => ExecReport::Finished(Ok(terminal)), } @@ -544,6 +546,21 @@ fn run_exec(job: ExecJob, logger: &mut Logger) -> ExecReport { } } +fn report_unconfirmed(outcome: container_steps::ExecOutcome) -> ExecReport { + let (detail, api_failure) = match outcome.post_launch_error { + Some(error) => (error.error_message, error.api_failure), + None => ( + "process termination could not be confirmed".to_string(), + None, + ), + }; + ExecReport::Unconfirmed { + detail, + api_failure: api_failure.or(outcome.unconfirmed_api_failure), + held_stdin: outcome.held_stdin, + } +} + /// A chunk of live process output streamed from the worker to the pipe handler: /// which stream it came from and the bytes (owned, so it can cross the channel). pub type OutputChunk = (OutStream, Vec); @@ -719,7 +736,7 @@ pub(crate) fn register_exec( entry.insert(active_exec); } Entry::Occupied(_) => { - return Err(WorkerError::Rejected(anyhow::anyhow!( + return Err(WorkerError::rejected(anyhow::anyhow!( "exec id {exec_id:?} is already active" ))); } @@ -876,7 +893,7 @@ impl SessionHandle { run_token: &str, ) -> Result { let rejected = |reason: &str| { - Err(WorkerError::Rejected(anyhow::anyhow!( + Err(WorkerError::rejected(anyhow::anyhow!( "cannot attach stdin to exec {exec_id:?}: {reason}" ))) }; @@ -947,12 +964,12 @@ impl SessionHandle { fn send(&self, cmd: WorkerCommand) -> Result<(), WorkerError> { self.tx .send(cmd) - .map_err(|_| WorkerError::Backend(anyhow::anyhow!("WSLc worker thread is gone"))) + .map_err(|_| WorkerError::backend(anyhow::anyhow!("WSLc worker thread is gone"))) } } fn worker_gone(_e: oneshot::error::RecvError) -> WorkerError { - WorkerError::Backend(anyhow::anyhow!("WSLc worker dropped the reply channel")) + WorkerError::backend(anyhow::anyhow!("WSLc worker dropped the reply channel")) } /// Per-container bookkeeping held by the worker: whether the container is @@ -1036,7 +1053,7 @@ impl ContainerWork { /// Answer this work with `message`, for a daemon that is shutting down. fn refuse(self, message: &str) { - let error = WorkerError::Backend(anyhow::anyhow!("{message}")); + let error = WorkerError::backend(anyhow::anyhow!("{message}")); match self { ContainerWork::Exec(request) => { let _ = request.admit.send(Err(error)); @@ -1073,7 +1090,7 @@ fn start_run_on_sdk( worker: &mpsc::UnboundedSender, ) -> Result<(), WorkerError> { let Some(sdk) = state.sdk.as_ref() else { - return Err(WorkerError::Backend(anyhow::anyhow!( + return Err(WorkerError::backend(anyhow::anyhow!( "no active WSLc session" ))); }; @@ -1091,7 +1108,7 @@ fn start_run_on_sdk( }; start_exec(job, worker.clone(), &state.execs_in_flight).map_err(|e| { - WorkerError::Backend(anyhow::anyhow!( + WorkerError::backend(anyhow::anyhow!( "could not start a thread to run exec on sandbox {sandbox_id}: {e}" )) }) @@ -1190,7 +1207,7 @@ fn start_session_boot( let job = job; let outcome = match ComApartment::enter() { - Err(e) => Err(WorkerError::Backend(e).into()), + Err(e) => Err(WorkerError::backend(e).into()), Ok(_apartment) => { let mut log = Logger::new(Mode::Console); @@ -1290,7 +1307,7 @@ fn start_session_release( let mut log = Logger::new(Mode::Console); let mut slot_returned = false; let outcome = match ComApartment::enter() { - Err(e) => Err(WorkerError::Backend(e)), + Err(e) => Err(WorkerError::backend(e)), Ok(_apartment) => { let Some(session) = job.session.take() else { return; @@ -1581,7 +1598,7 @@ impl Worker { let (store, ()) = self .store_slots .lease_with(|_| Ok::<(), std::convert::Infallible>(())) - .map_err(|e| WorkerError::Backend(anyhow::anyhow!("{e}")))?; + .map_err(|e| WorkerError::backend(anyhow::anyhow!("{e}")))?; let sdk = self.sdk.as_ref().expect("sdk loaded above"); let job = SessionJob { @@ -1600,7 +1617,7 @@ impl Worker { &self.sessions_in_flight, ) { // The lease drops with `store` here, returning the slot. - return Err(WorkerError::Backend(anyhow::anyhow!( + return Err(WorkerError::backend(anyhow::anyhow!( "could not start a thread to boot session {session_ref}: {e}" ))); } @@ -1897,7 +1914,7 @@ impl Worker { // The implicit session is shared by every request that named none, so // no single caller may release it. if self.default_session.as_deref() == Some(session_ref) { - let _ = reply.send(Err(WorkerError::Rejected(anyhow::anyhow!( + let _ = reply.send(Err(WorkerError::rejected(anyhow::anyhow!( "session {session_ref} is the daemon's implicit session and is released only \ once nothing is using it" )))); @@ -1906,7 +1923,7 @@ impl Worker { let hold = self.session_hold(session_ref); if hold > 0 { - let _ = reply.send(Err(WorkerError::Rejected(anyhow::anyhow!( + let _ = reply.send(Err(WorkerError::rejected(anyhow::anyhow!( "session {session_ref} still has {hold} sandbox(es) or operation(s) in it; \ deprovision them first" )))); @@ -2017,13 +2034,13 @@ impl Worker { pending.abandoned = true; if let Some(reply) = pending.reply.take() { - let _ = reply.send(Err(WorkerError::Backend(anyhow::anyhow!( + let _ = reply.send(Err(WorkerError::backend(anyhow::anyhow!( "the WSLc session did not finish booting within {}s", deadline.as_secs() )))); } for (_, parked) in std::mem::take(&mut pending.parked) { - let _ = parked.send(Err(WorkerError::Backend(anyhow::anyhow!( + let _ = parked.send(Err(WorkerError::backend(anyhow::anyhow!( "the WSLc session this provision was waiting for did not finish \ booting within {}s", deadline.as_secs() @@ -2259,7 +2276,7 @@ impl Worker { // Parking without a deadline is how a provision waits // forever, so refuse instead. The pull carries on, counted // against its session, and warms the cache for a retry. - let _ = reply.send(Err(WorkerError::Backend(anyhow::anyhow!( + let _ = reply.send(Err(WorkerError::backend(anyhow::anyhow!( "could not arm the pull deadline for image '{}'", config.image )))); @@ -2411,7 +2428,7 @@ impl Worker { .get_mut(&config.sandbox_id) .expect("checked above"); if entry.quarantined { - return Err(WorkerError::Backend(anyhow::anyhow!( + return Err(WorkerError::backend(anyhow::anyhow!( "sandbox {} is quarantined after an exec whose termination could not be \ confirmed; deprovision it before reuse", config.sandbox_id @@ -2437,7 +2454,7 @@ impl Worker { fn validate_exec(&self, sandbox_id: &str) -> Result { match self.containers.get(sandbox_id) { None => Err(WorkerError::NotProvisioned(sandbox_id.to_string())), - Some(entry) if entry.quarantined => Err(WorkerError::Backend(anyhow::anyhow!( + Some(entry) if entry.quarantined => Err(WorkerError::backend(anyhow::anyhow!( "sandbox {sandbox_id} is quarantined after an exec whose termination could not \ be confirmed; deprovision it before reuse" ))), @@ -2779,12 +2796,12 @@ impl Worker { match cmd { WorkerCommand::ProvisionSession { reply, .. } => { - let _ = reply.send(Err(WorkerError::Backend(anyhow::anyhow!( + let _ = reply.send(Err(WorkerError::backend(anyhow::anyhow!( "{SHUTTING_DOWN}" )))); } WorkerCommand::DeprovisionSession { reply, .. } => { - let _ = reply.send(Err(WorkerError::Backend(anyhow::anyhow!( + let _ = reply.send(Err(WorkerError::backend(anyhow::anyhow!( "{SHUTTING_DOWN}" )))); } @@ -2797,7 +2814,7 @@ impl Worker { // nothing. WorkerCommand::SessionReleased { .. } => {} WorkerCommand::Provision { reply, .. } => { - let _ = reply.send(Err(WorkerError::Backend(anyhow::anyhow!( + let _ = reply.send(Err(WorkerError::backend(anyhow::anyhow!( "{SHUTTING_DOWN}" )))); } @@ -2831,7 +2848,7 @@ impl Worker { let deadline = Instant::now() + drain_budget; for (_, pending) in self.pending.drain() { - let _ = pending.reply.send(Err(WorkerError::Backend(anyhow::anyhow!( + let _ = pending.reply.send(Err(WorkerError::backend(anyhow::anyhow!( "the WSLc daemon shut down while pulling image '{}'", pending.config.image )))); @@ -2841,12 +2858,12 @@ impl Worker { // that asked for the session, and every provision parked behind it. for (session_ref, pending) in self.pending_sessions.iter_mut() { if let Some(reply) = pending.reply.take() { - let _ = reply.send(Err(WorkerError::Backend(anyhow::anyhow!( + let _ = reply.send(Err(WorkerError::backend(anyhow::anyhow!( "the WSLc daemon shut down while session {session_ref} was booting" )))); } for (_, parked) in std::mem::take(&mut pending.parked) { - let _ = parked.send(Err(WorkerError::Backend(anyhow::anyhow!( + let _ = parked.send(Err(WorkerError::backend(anyhow::anyhow!( "the WSLc daemon shut down while session {session_ref} was booting" )))); } @@ -2891,7 +2908,7 @@ impl Worker { for (sandbox_id, in_flight) in self.exec_in_flight.drain() { let _ = in_flight .done - .send(Err(WorkerError::Backend(anyhow::anyhow!( + .send(Err(WorkerError::backend(anyhow::anyhow!( "the WSLc daemon shut down while running an exec on sandbox {sandbox_id}" )))); for work in in_flight.parked { @@ -4953,7 +4970,7 @@ mod tests { worker.finish_session( TEST_SESSION, Err(BootFailure { - error: WorkerError::Backend(anyhow::anyhow!("store probe failed")), + error: WorkerError::backend(anyhow::anyhow!("store probe failed")), store_may_be_held: true, }), &worker_tx, @@ -4984,7 +5001,7 @@ mod tests { tx.send(WorkerCommand::SessionBooted { session_ref: TEST_SESSION.to_string(), outcome: Err(BootFailure { - error: WorkerError::Backend(anyhow::anyhow!("store probe failed")), + error: WorkerError::backend(anyhow::anyhow!("store probe failed")), store_may_be_held: true, }), }) @@ -6238,6 +6255,88 @@ mod tests { assert!(!worker.containers.contains_key("wslc:busy")); } + fn kill_failure() -> ApiFailure { + ApiFailure::new("WslcSignalProcess").with_native_code("0x8007001f") + } + + fn unconfirmed_outcome( + post_launch_error: Option, + unconfirmed_api_failure: Option, + ) -> container_steps::ExecOutcome { + container_steps::ExecOutcome { + completion: container_steps::ProcessCompletion::TerminationUnconfirmed, + stdout: Vec::new(), + stderr: Vec::new(), + post_launch_error, + unconfirmed_api_failure: unconfirmed_api_failure.map(Box::new), + held_stdin: None, + } + } + + #[test] + fn a_failed_kill_reaches_the_quarantine_error_whether_or_not_the_delete_works() { + for delete_result in [Some(Ok(())), Some(Err(delete_failure()))] { + let ExecReport::Unconfirmed { + detail, + api_failure, + .. + } = report_unconfirmed(unconfirmed_outcome(None, Some(kill_failure()))) + else { + panic!("an unconfirmed run must be reported as unconfirmed"); + }; + let mut worker = Worker::new(); + worker + .containers + .insert("wslc:busy".to_string(), started_entry()); + + let err = + worker.finish_quarantine("wslc:busy", &detail, api_failure, None, delete_result); + + assert!( + err.to_string() + .contains("process termination could not be confirmed"), + "got {err}" + ); + assert_eq!(err.api_failure(), Some(&kill_failure())); + } + } + + #[test] + fn an_unconfirmed_run_names_its_own_failure_ahead_of_a_failed_kill() { + let forwarding = WslcStepError { + failure_phase: FailurePhase::PostLaunchFailed, + error_message: "could not start forwarding stdin: broken pipe".to_string(), + api_failure: None, + }; + let ExecReport::Unconfirmed { + detail, + api_failure, + .. + } = report_unconfirmed(unconfirmed_outcome(Some(forwarding), Some(kill_failure()))) + else { + panic!("an unconfirmed run must be reported as unconfirmed"); + }; + assert_eq!(detail, "could not start forwarding stdin: broken pipe"); + assert_eq!(api_failure.as_deref(), Some(&kill_failure())); + + let io_handle = WslcStepError { + failure_phase: FailurePhase::LaunchFailed, + error_message: "WslcGetProcessIOHandle(stdin) failed: HRESULT 0x80070006".to_string(), + api_failure: Some(Box::new( + ApiFailure::new("WslcGetProcessIOHandle").with_native_code("0x80070006"), + )), + }; + let ExecReport::Unconfirmed { api_failure, .. } = + report_unconfirmed(unconfirmed_outcome(Some(io_handle), Some(kill_failure()))) + else { + panic!("an unconfirmed run must be reported as unconfirmed"); + }; + assert_eq!( + api_failure.as_deref(), + Some(&ApiFailure::new("WslcGetProcessIOHandle").with_native_code("0x80070006")) + ); + } + /// A report can outlive its slot: shutdown drains the run and clears the /// map before the posted command is serviced. #[test] diff --git a/tests/scripts/run_wslc_state_aware_tests.ps1 b/tests/scripts/run_wslc_state_aware_tests.ps1 index 272d1be406..266a802ddc 100644 --- a/tests/scripts/run_wslc_state_aware_tests.ps1 +++ b/tests/scripts/run_wslc_state_aware_tests.ps1 @@ -1304,11 +1304,15 @@ $cqSandbox = New-PortMappedSandbox $cqPort = $cqSandbox.Port $script:cqSandboxId = $cqSandbox.SandboxId try { - $cqReady = $false - if ($null -ne $script:cqSandboxId -and (Envelope-Arm (Parse-Envelope -Stdout $cqSandbox.Start.Stdout)) -eq 'result') { - $script:cqStarted = $true - $r = Invoke-StateAware -ConfigFile 'wslc_state_aware_exec_port_bind.json' -SandboxId $script:cqSandboxId - $cqReady = ($r.ExitCode -eq 0 -and $r.Stdout -match 'LISTENER_STARTED') + $cqReady = Run-StateAwareTest "CQ: a port-mapped sandbox starts and listens on its forwarded port" { + if (Assert-ResultEnvelope $cqSandbox.Provision "port-mapping provision") { + if (Assert-ResultEnvelope $cqSandbox.Start "port-mapping start") { + $script:cqStarted = $true + $r = Invoke-StateAware -ConfigFile 'wslc_state_aware_exec_port_bind.json' -SandboxId $script:cqSandboxId + Assert-True ($r.ExitCode -eq 0) "listener exec exit code = 0" + Assert-True ($r.Stdout -match 'LISTENER_STARTED') "container listener started" + } + } } if ($cqReady) {