From 802456bb80a79c6eab4dbbab89f78eb6572ed2a8 Mon Sep 17 00:00:00 2001 From: Soham Das Date: Wed, 4 Nov 2026 12:12:21 -0800 Subject: [PATCH 1/4] [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 41092e9a93..c821929c13 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 @@ -983,7 +983,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 15faf50d4d9891cbcedffbc2215a3ed936e6addc Mon Sep 17 00:00:00 2001 From: Soham Das Date: Fri, 6 Nov 2026 09:03:21 -0800 Subject: [PATCH 2/4] [WSLC] Preserve structured errors from process waits Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docs/api-reference/dotnet/v1/types.md | 7 + docs/api-reference/node/v1/types.md | 6 + docs/api-reference/rust/v1/types.md | 9 + docs/backends/wslc/wslc-state-aware.md | 15 +- .../architecture/container-lifecycle.md | 15 +- scripts/ci/build-windows-sdk-test-bundle.ps1 | 68 +++ scripts/ci/run_backend_validation_tests.ps1 | 23 +- .../Microsoft.Mxc.Sdk.Tests.csproj | 32 +- .../V1/MxcContainerWslcE2ETests.cs | 44 ++ .../V1/MxcLifecycleWslcE2ETests.cs | 56 ++ .../V1/MxcProcessCleanupTests.cs | 65 ++- sdk/dotnet/Microsoft.Mxc.Sdk/V1/MxcProcess.cs | 61 ++- sdk/dotnet/README.md | 5 + sdk/node/README.md | 5 + sdk/node/src/bindings/streaming.ts | 89 +++- sdk/node/tests/integration/wslc-e2e.test.ts | 38 ++ .../integration/wslc-state-aware.test.ts | 85 ++++ sdk/node/tests/unit/sandbox-process.test.ts | 31 ++ sdk/node/tests/unit/streaming-binding.test.ts | 106 +++- src/ffi/mxc_ffi/src/streaming.rs | 477 +++++++++++++++++- src/ffi/mxc_ffi/src/test_fake_sandbox.rs | 100 +++- src/mxc-sdk/README.md | 9 + .../backends/wslc/common/container_steps.rs | 56 +- src/mxc-sdk/src/backends/wslc/common/error.rs | 30 +- .../src/backends/wslc/common/sandbox.rs | 41 +- .../wslc/common/wsl_container_runner.rs | 70 ++- .../src/core/mxc_common/exec_stream.rs | 104 +++- src/mxc-sdk/src/core/mxc_common/mxc_error.rs | 22 + src/mxc-sdk/src/core/mxc_engine/error.rs | 18 + src/mxc-sdk/src/lib.rs | 7 +- src/mxc-sdk/src/sandbox.rs | 180 ++++++- 31 files changed, 1735 insertions(+), 139 deletions(-) diff --git a/docs/api-reference/dotnet/v1/types.md b/docs/api-reference/dotnet/v1/types.md index 589e84252b..960aefe246 100644 --- a/docs/api-reference/dotnet/v1/types.md +++ b/docs/api-reference/dotnet/v1/types.md @@ -725,6 +725,13 @@ public class MxcProcess : IMxcProcess ``` +`TryGetExitCode`, `Wait`, `WaitAsync`, and `WaitForExitWithOutputAsync` throw +`MxcException` when MXC cannot obtain a terminal process result after +admission. The exception preserves `Code`, `Message`, `Operation`, +`NativeCode`, and `Remediation`; the failure is not converted into an exit +code. + + ## `Microsoft.Mxc.Sdk.V1.MxcPtyProcess` A live container process attached to a caller-driven pseudo-terminal. diff --git a/docs/api-reference/node/v1/types.md b/docs/api-reference/node/v1/types.md index 83d438ca14..1f7876f2dd 100644 --- a/docs/api-reference/node/v1/types.md +++ b/docs/api-reference/node/v1/types.md @@ -469,6 +469,12 @@ export class MxcProcess { ``` +`wait()` rejects with `MxcError` when MXC cannot obtain a terminal process +result after admission. The rejection preserves `code`, `message`, +`operation`, `nativeCode`, `remediation`, and `details`; it is not converted +into an exit result. + + ## `@microsoft/mxc-sdk/v1::MxcPtyProcess` A container process attached to an MXC-owned pseudo-terminal. diff --git a/docs/api-reference/rust/v1/types.md b/docs/api-reference/rust/v1/types.md index 49954feb04..d01b52881a 100644 --- a/docs/api-reference/rust/v1/types.md +++ b/docs/api-reference/rust/v1/types.md @@ -329,15 +329,24 @@ impl MxcProcess { pub fn stdout_closer(&self) -> Option; pub fn stderr_closer(&self) -> Option; pub fn try_wait(&mut self) -> std::io::Result>; + pub fn try_wait_detailed(&mut self) -> Result, Error>; pub fn id(&self) -> u32; pub fn kill(&mut self) -> std::io::Result<()>; pub fn kill_for_timeout(&mut self) -> std::io::Result<()>; pub fn wait(&mut self) -> std::io::Result; + pub fn wait_detailed(&mut self) -> Result; pub fn wait_with_output(mut self) -> std::io::Result; + pub fn wait_with_output_detailed(self) -> Result; } ``` +The detailed wait methods preserve structured backend failures that occur +after process admission. The existing methods retain their `std::io::Error` +contracts and render the same message text. A cached terminal failure is +replayed on later waits and does not prove that the process exited. + + ## `mxc_sdk::v1::MxcPtyProcess` A live container process attached to a caller-controlled pseudo-terminal. diff --git a/docs/backends/wslc/wslc-state-aware.md b/docs/backends/wslc/wslc-state-aware.md index d57ed5c4e1..e854e7abf8 100644 --- a/docs/backends/wslc/wslc-state-aware.md +++ b/docs/backends/wslc/wslc-state-aware.md @@ -500,6 +500,12 @@ pipe from the `WithStdin` variants. Rust and CLI attached relay surfaces as a backend error because that path cannot return a typed timeout; the streaming path reports it through its wait result. +An exec can be admitted and produce output before a WSLc SDK call needed to +obtain its terminal status fails. Captured SDK execution then fails, and a live +process's wait rejects, with the original structured WSLc error rather than an +exit code. The error identifies the failed SDK function in `operation` and +includes its HRESULT in `nativeCode` when available. + ### exec stdin An exec reads the stdin its caller supplies: @@ -624,6 +630,12 @@ daemon attempts immediate deletion. If deletion fails, the quarantined entry is retained so `deprovision` can retry cleanup; it is not removed from tracking while a potentially running workload remains. +A terminal-status failure is cached by the live SDK process handle, so another +wait reports the same failure instead of claiming a different outcome. Because +that failure does not confirm the process is gone, explicit termination and +handle disposal still attempt cleanup. The daemon's quarantine behavior above +continues to protect the container when it cannot prove termination. + ## Supported and rejected policy fields WSLc networking is **all-or-nothing** (`WslcContainerNetworkingMode` `None` vs @@ -705,7 +717,8 @@ 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. +in `nativeCode` when the call returned one. Captured execution and live-process +waits in the Rust detailed API, .NET, and Node preserve those fields. `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 diff --git a/docs/development/architecture/container-lifecycle.md b/docs/development/architecture/container-lifecycle.md index 5aad09dd8c..ed05103c78 100644 --- a/docs/development/architecture/container-lifecycle.md +++ b/docs/development/architecture/container-lifecycle.md @@ -323,7 +323,8 @@ administrative restriction. Options do not change the owned wire contract. and `validateProcess` use native dry-run validation and return no execution result. There is no `dryRun` execution option or public attached-exec API. Captured and streaming execution are asynchronous. Live processes own wait, -kill, and disposal. +kill, and disposal. A failure reported after exec admission but before MXC can +confirm the terminal result is an operation failure, not an exit result. IsolationSession provides stdin, stdout, and stderr. WSLC provides stdout and stderr; only `spawnInContainerWithStdin` also gives it a writable stdin, as `spawnWithStdin` does for create-and-run execution. @@ -956,6 +957,18 @@ Process-runtime kill conditions (a script exceeding its `timeout`, a backend for terminating a process) are not represented as typed wire-format errors. They surface as sentinel exit codes from the exec process, matching the existing one-shot convention. +A backend can also fail while observing the process after admission. Captured +execution and the .NET and Node live-process wait APIs preserve the resulting +error code, message, operation, native code, and remediation instead of +reporting a successful exit. Rust `MxcProcess::try_wait`, `wait`, and +`wait_with_output` retain their existing `std::io::Error` contracts and +message text. Their `*_detailed` counterparts return `mxc_sdk::v1::Error` with +the structured fields intact. + +Once a live-process handle has observed a terminal failure, later wait calls +replay that failure. The failure does not prove that the process exited, so the +handle retains its termination path for an explicit kill or disposal cleanup. + ### 8.2 Details payload `details` (introduced in §7.3) is an open `Record`. Backends use it to diff --git a/scripts/ci/build-windows-sdk-test-bundle.ps1 b/scripts/ci/build-windows-sdk-test-bundle.ps1 index cd15f3056e..e160f7e232 100644 --- a/scripts/ci/build-windows-sdk-test-bundle.ps1 +++ b/scripts/ci/build-windows-sdk-test-bundle.ps1 @@ -76,6 +76,23 @@ function Invoke-CargoBuild { } | Where-Object { $_.reason -eq 'compiler-artifact' } } +function Invoke-CargoDebugBuild { + param([Parameter(Mandatory)][string[]]$Arguments) + + Push-Location $srcRoot + try { + $messages = Invoke-Checked "cargo $($Arguments -join ' ')" { + cargo @Arguments --locked --target $triple --message-format=json + } + } + finally { + Pop-Location + } + $messages | ForEach-Object { + try { $_ | ConvertFrom-Json -ErrorAction Stop } catch { } + } | Where-Object { $_.reason -eq 'compiler-artifact' } +} + function Get-CargoExecutable { param([object[]]$Artifacts, [string]$TargetName) @@ -115,7 +132,18 @@ $nativeLibrary = Invoke-CargoBuild @('build', '-p', 'mxc_ffi', '--features', 'is if (-not $nativeLibrary) { throw 'cargo reported no mxc_ffi.dll.' } +$testNativeLibrary = Invoke-CargoDebugBuild @( + 'build', '-p', 'mxc_ffi', '--features', 'isolation_session,wslc,test-support') | + Where-Object { $_.target.name -eq 'mxc_ffi' } | + ForEach-Object { $_.filenames } | + Where-Object { [IO.Path]::GetExtension($_) -eq '.dll' } | + Select-Object -Last 1 +if (-not $testNativeLibrary) { + throw 'cargo reported no test-support mxc_ffi.dll.' +} $wslcDaemon = Get-CargoExecutable (Invoke-CargoBuild @('build', '-p', 'mxc-sdk', '--features', 'wslc', '--bin', 'wxc-wslc-daemon')) 'wxc-wslc-daemon' +$testWslcDaemon = Get-CargoExecutable (Invoke-CargoDebugBuild @( + 'build', '-p', 'mxc-sdk', '--features', 'wslc,test-support', '--bin', 'wxc-wslc-daemon')) 'wxc-wslc-daemon' # The wslc build feature stages the WSLC SDK library beside mxc_ffi. $wslcSdk = Join-Path (Split-Path -Parent $nativeLibrary) 'wslcsdk.dll' @@ -145,6 +173,13 @@ if ($NuGetSource) { $publish += @('--source', $NuGetSource) } Invoke-Checked 'dotnet publish' { dotnet @publish } +foreach ($source in $testNativeLibrary, $testWslcDaemon) { + $destination = Join-Path $dotnetOut (Split-Path -Leaf $source) + Copy-Item -LiteralPath $source -Destination $destination -Force + if ((Get-FileHash -LiteralPath $source).Hash -ne (Get-FileHash -LiteralPath $destination).Hash) { + throw "The published .NET test bundle did not stage $(Split-Path -Leaf $source)." + } +} Copy-Item -LiteralPath $WxcExecPath -Destination (New-Directory (Join-Path $OutputDirectory "bin\$Architecture")) @@ -189,6 +224,14 @@ foreach ($name in 'bin', 'dist', 'node_modules', 'package.json', 'LICENSE.md') { Copy-Item -LiteralPath $source -Destination $sdkOut -Recurse -Force } } +$nodeSdkBin = Join-Path $sdkOut "bin\$Architecture" +foreach ($source in $testNativeLibrary, $testWslcDaemon) { + $destination = Join-Path $nodeSdkBin (Split-Path -Leaf $source) + Copy-Item -LiteralPath $source -Destination $destination -Force + if ((Get-FileHash -LiteralPath $source).Hash -ne (Get-FileHash -LiteralPath $destination).Hash) { + throw "The Node integration bundle did not stage $(Split-Path -Leaf $source)." + } +} $manifest = [ordered]@{ format_version = 1 @@ -227,6 +270,7 @@ $required = @( 'sdk-integration\dist\dev.test.js', 'sdk-integration\dist\wslc-state-aware.test.js', 'sdk-integration\dist\wslc-e2e.test.js', + "sdk-integration\node_modules\@microsoft\mxc-sdk\bin\$Architecture\mxc_ffi.dll", "sdk-integration\node_modules\@microsoft\mxc-sdk\bin\$Architecture\wxc-wslc-daemon.exe", "sdk-integration\node_modules\@microsoft\mxc-sdk\bin\$Architecture\wslcsdk.dll", 'test_scripts\run_isolation_session_tests.ps1', @@ -294,9 +338,21 @@ Assert-Discovery '.NET exact-JSON WSLC lifecycle tests' 'MxcLifecycleWslcE2ETest Assert-Discovery '.NET WSLC lifecycle tests' 'MxcLifecycleWslcE2ETests\.' { & (Join-Path $dotnetOut 'Microsoft.Mxc.Sdk.Tests.exe') -list full -class '*.MxcLifecycleWslcE2ETests' } +foreach ($test in 'RunInContainer_PreservesStructuredPostAdmissionWaitFailure', + 'SpawnInContainer_WaitPreservesStructuredPostAdmissionWaitFailure') { + Assert-Discovery ".NET WSLC $test" "MxcLifecycleWslcE2ETests\.${test}" { + & (Join-Path $dotnetOut 'Microsoft.Mxc.Sdk.Tests.exe') -list full -class '*.MxcLifecycleWslcE2ETests' + } +} Assert-Discovery '.NET WSLC create-and-run tests' 'MxcContainerWslcE2ETests\.' { & (Join-Path $dotnetOut 'Microsoft.Mxc.Sdk.Tests.exe') -list full -class '*.MxcContainerWslcE2ETests' } +foreach ($test in 'Run_PreservesStructuredPostAdmissionWaitFailure', + 'Spawn_WaitPreservesStructuredPostAdmissionWaitFailure') { + Assert-Discovery ".NET WSLC $test" "MxcContainerWslcE2ETests\.${test}" { + & (Join-Path $dotnetOut 'Microsoft.Mxc.Sdk.Tests.exe') -list full -class '*.MxcContainerWslcE2ETests' + } +} Push-Location $integrationOut $env:MXC_SKIP_OS_BUILD_DEPENDENT_TESTS = '1' try { @@ -312,9 +368,21 @@ try { Assert-Discovery 'Node WSLC lifecycle suite' 'WSLC state-aware lifecycle E2E' { & (Join-Path $OutputDirectory 'node\node.exe') --test --test-force-exit dist/wslc-state-aware.test.js } + foreach ($test in 'captured exec preserves structured post-admission wait failures', + 'process wait preserves structured post-admission wait failures') { + if (-not (Select-String -LiteralPath 'dist/wslc-state-aware.test.js' -SimpleMatch $test -Quiet)) { + throw "The Node WSLC lifecycle payload is missing $test." + } + } Assert-Discovery 'Node WSLC create-and-run suite' 'WSLC SDK E2E' { & (Join-Path $OutputDirectory 'node\node.exe') --test --test-force-exit dist/wslc-e2e.test.js } + foreach ($test in 'captured run preserves structured post-admission wait failures', + 'spawned process wait preserves structured post-admission wait failures') { + if (-not (Select-String -LiteralPath 'dist/wslc-e2e.test.js' -SimpleMatch $test -Quiet)) { + throw "The Node WSLC create-and-run payload is missing $test." + } + } } finally { Remove-Item Env:MXC_SKIP_OS_BUILD_DEPENDENT_TESTS diff --git a/scripts/ci/run_backend_validation_tests.ps1 b/scripts/ci/run_backend_validation_tests.ps1 index 2859dfa514..3cf2478d7b 100644 --- a/scripts/ci/run_backend_validation_tests.ps1 +++ b/scripts/ci/run_backend_validation_tests.ps1 @@ -348,9 +348,26 @@ function Invoke-WslcSuites { & (Join-Path $testScriptRoot 'run_wslc_all_tests.ps1') -WxcExecPath $wxc | Out-Host if ($LASTEXITCODE -ne 0) { "exited $LASTEXITCODE" } } - 'Node' = { Invoke-NodeIntegrationSuite -Bundle $bundle -Name 'wslc' -TestFile 'wslc-state-aware.test.js' -RequiredTests 'V1.Dev preserves WSLC lifecycle responses, captured exec, pipes, and caller ownership' } - 'Node create-and-run' = { Invoke-NodeIntegrationSuite -Bundle $bundle -Name 'wslc-create-and-run' -TestFile 'wslc-e2e.test.js' -RequiredTests 'should reject an unsupported targetOs before starting a container' } - 'C#' = { Invoke-XunitClasses -Bundle $bundle -Name 'wslc' -Classes 'MxcLifecycleWslcE2ETests', 'MxcContainerWslcE2ETests' -RequiredMethods 'DevJson_WslcLifecyclePreservesResponsesExecutionAndCallerOwnership' } + 'Node' = { + Invoke-NodeIntegrationSuite -Bundle $bundle -Name 'wslc' -TestFile 'wslc-state-aware.test.js' ` + -RequiredTests @('V1.Dev preserves WSLC lifecycle responses, captured exec, pipes, and caller ownership', + 'captured exec preserves structured post-admission wait failures', + 'process wait preserves structured post-admission wait failures') + } + 'Node create-and-run' = { + Invoke-NodeIntegrationSuite -Bundle $bundle -Name 'wslc-create-and-run' -TestFile 'wslc-e2e.test.js' ` + -RequiredTests @('should reject an unsupported targetOs before starting a container', + 'captured run preserves structured post-admission wait failures', + 'spawned process wait preserves structured post-admission wait failures') + } + 'C#' = { + Invoke-XunitClasses -Bundle $bundle -Name 'wslc' -Classes 'MxcLifecycleWslcE2ETests', 'MxcContainerWslcE2ETests' ` + -RequiredMethods @('DevJson_WslcLifecyclePreservesResponsesExecutionAndCallerOwnership', + 'RunInContainer_PreservesStructuredPostAdmissionWaitFailure', + 'SpawnInContainer_WaitPreservesStructuredPostAdmissionWaitFailure', + 'Run_PreservesStructuredPostAdmissionWaitFailure', + 'Spawn_WaitPreservesStructuredPostAdmissionWaitFailure') + } }) } diff --git a/sdk/dotnet/Microsoft.Mxc.Sdk.Tests/Microsoft.Mxc.Sdk.Tests.csproj b/sdk/dotnet/Microsoft.Mxc.Sdk.Tests/Microsoft.Mxc.Sdk.Tests.csproj index 2d1a1a8b4a..433bb0d901 100644 --- a/sdk/dotnet/Microsoft.Mxc.Sdk.Tests/Microsoft.Mxc.Sdk.Tests.csproj +++ b/sdk/dotnet/Microsoft.Mxc.Sdk.Tests/Microsoft.Mxc.Sdk.Tests.csproj @@ -14,6 +14,11 @@ $(DefineConstants);MXC_WITH_ISOLATION_SESSION $(MSBuildProjectDirectory)/../../../src $(MxcSrcDir)/target/dotnet-test-support + $(CARGO_TARGET_DIR)/dotnet-wslc-test-support + $([System.IO.Path]::GetFileName($([System.IO.Path]::GetFullPath('$(MSBuildProjectDirectory)/../../..')))) + $(LOCALAPPDATA) + $(TEMP) + $(MxcTestWslcCargoRoot)/mxct/dotnet-wslc-test-support/$(MxcTestRepositoryName) debug @@ -33,7 +38,7 @@ + Condition="'$(TargetFramework)' != '' and '$(DesignTimeBuild)' != 'true'"> $(MxcTestRustcHostLine.Replace('host: ', '').Trim()) $(MxcTestCargoTargetDir)/$(MxcTestRustHostTriple)/$(MxcTestCargoProfileDir)/$(MxcTestNativeFileName) + $(MxcTestWslcCargoTargetDir)/$(MxcTestRustHostTriple)/$(MxcTestCargoProfileDir)/wxc-wslc-daemon.exe - - - + + + DestinationFolder="$(TargetDir)" + Condition="'$(MxcUsePrebuiltTestNativeLibrary)' != 'true'" /> + + + + (() => + MxcContainer.Run(Request(ForcedWaitFailureCommand))); + + AssertForcedWaitFailure(error); + Assert.Equal( + "RUN-RECOVERED", + MxcContainer.Run(Request("echo RUN-RECOVERED")).Stdout.Trim()); + } + + [Fact] + public void Spawn_WaitPreservesStructuredPostAdmissionWaitFailure() + { + RequireWslc(); + + using (var process = MxcContainer.Spawn(Request(ForcedWaitFailureCommand))) + { + AssertForcedWaitFailure(Assert.Throws(() => process.Wait())); + AssertForcedWaitFailure(Assert.Throws(() => process.Wait())); + } + + Assert.Equal( + "PROCESS-RECOVERED", + MxcContainer.Run(Request("echo PROCESS-RECOVERED")).Stdout.Trim()); + } + + private static void AssertForcedWaitFailure(MxcException error) + { + Assert.Equal(ErrorCode.BackendError, error.Code); + Assert.Equal( + "WslcGetProcessExitCode failed: HRESULT 0x80004005", + error.Message); + Assert.Equal("WslcGetProcessExitCode", error.Operation); + Assert.Equal("0x80004005", error.NativeCode); + Assert.Null(error.Remediation); + } } diff --git a/sdk/dotnet/Microsoft.Mxc.Sdk.Tests/V1/MxcLifecycleWslcE2ETests.cs b/sdk/dotnet/Microsoft.Mxc.Sdk.Tests/V1/MxcLifecycleWslcE2ETests.cs index 00ac7792cb..5d1627bcb0 100644 --- a/sdk/dotnet/Microsoft.Mxc.Sdk.Tests/V1/MxcLifecycleWslcE2ETests.cs +++ b/sdk/dotnet/Microsoft.Mxc.Sdk.Tests/V1/MxcLifecycleWslcE2ETests.cs @@ -18,6 +18,9 @@ namespace Microsoft.Mxc.Sdk.Tests.V1; [Collection("MxcLiveHost")] public class MxcLifecycleWslcE2ETests { + private const string ForcedWaitFailureCommand = + "true # mxc-test-force-wslc-get-exit-code-failure"; + [Fact] public async Task DevJson_WslcLifecyclePreservesResponsesExecutionAndCallerOwnership() { @@ -300,4 +303,57 @@ public void SpawnInContainer_GivesACommandThatReadsStdinEndOfFile() Assert.Equal(0, result.ExitCode); Assert.Equal("SPAWN-SAW-EOF", stdout.Trim()); } + + [Fact] + public void RunInContainer_PreservesStructuredPostAdmissionWaitFailure() + { + RequireWslc(); + using var container = new StartedContainer(); + + var error = Assert.Throws(() => + MxcLifecycle.RunInContainer( + container.Id, + new ExecutionRequest(ForcedWaitFailureCommand) { TimeoutMs = 60_000 })); + + AssertForcedWaitFailure(error); + Assert.Equal( + "CAPTURED-RECOVERED", + MxcLifecycle.RunInContainer( + container.Id, + new ExecutionRequest("echo CAPTURED-RECOVERED") { TimeoutMs = 60_000 }) + .Stdout + .Trim()); + } + + [Fact] + public void SpawnInContainer_WaitPreservesStructuredPostAdmissionWaitFailure() + { + RequireWslc(); + using var container = new StartedContainer(); + using var process = MxcLifecycle.SpawnInContainer( + container.Id, + new ExecutionRequest(ForcedWaitFailureCommand) { TimeoutMs = 60_000 }); + + var error = Assert.Throws(() => process.Wait()); + + AssertForcedWaitFailure(error); + Assert.Equal( + "PROCESS-RECOVERED", + MxcLifecycle.RunInContainer( + container.Id, + new ExecutionRequest("echo PROCESS-RECOVERED") { TimeoutMs = 60_000 }) + .Stdout + .Trim()); + } + + private static void AssertForcedWaitFailure(MxcException error) + { + Assert.Equal(ErrorCode.BackendError, error.Code); + Assert.Equal( + "WslcGetProcessExitCode failed: HRESULT 0x80004005", + error.Message); + Assert.Equal("WslcGetProcessExitCode", error.Operation); + Assert.Equal("0x80004005", error.NativeCode); + Assert.Null(error.Remediation); + } } diff --git a/sdk/dotnet/Microsoft.Mxc.Sdk.Tests/V1/MxcProcessCleanupTests.cs b/sdk/dotnet/Microsoft.Mxc.Sdk.Tests/V1/MxcProcessCleanupTests.cs index 9f0bd4c654..5b84948574 100644 --- a/sdk/dotnet/Microsoft.Mxc.Sdk.Tests/V1/MxcProcessCleanupTests.cs +++ b/sdk/dotnet/Microsoft.Mxc.Sdk.Tests/V1/MxcProcessCleanupTests.cs @@ -94,6 +94,39 @@ public async Task ControlDuringCleanupAfterATimeoutKillDoesNotWaitForIt() Assert.True((await wait).TimedOut); } + [Fact] + public void TryGetExitCodePreservesStructuredWaitFailure() + { + using var fake = FakeSandbox.CreateTryWaitFailure(); + + var error = Assert.Throws(() => fake.Process.TryGetExitCode(out _)); + + AssertWaitError(error); + } + + [Fact] + public async Task WaitAsyncPreservesStructuredWaitFailureAfterRunningPoll() + { + using var fake = FakeSandbox.CreateWaitFailure(); + + var first = await Assert.ThrowsAsync( + () => fake.Process.WaitAsync(TestContext.Current.CancellationToken)); + var second = await Assert.ThrowsAsync( + () => fake.Process.WaitAsync(TestContext.Current.CancellationToken)); + + AssertWaitError(first); + AssertWaitError(second); + } + + private static void AssertWaitError(MxcException error) + { + Assert.Equal(ErrorCode.BackendUnavailable, error.Code); + Assert.Equal("test process wait failed", error.Message); + Assert.Equal("TestSandbox.Wait", error.Operation); + Assert.Equal("0x80004005", error.NativeCode); + Assert.Equal("Retry the test process wait.", error.Remediation); + } + private static async Task CompletesWithinAsync(Func read, string message) { var task = Task.Run(read, TestContext.Current.CancellationToken); @@ -115,6 +148,12 @@ private sealed unsafe class FakeSandbox : IDisposable private static readonly delegate* unmanaged New = (delegate* unmanaged)Export("mxc_test_fake_sandbox_new"); + private static readonly delegate* unmanaged NewTryWaitFailure = + (delegate* unmanaged)Export("mxc_test_fake_sandbox_new_try_wait_failure"); + + private static readonly delegate* unmanaged NewWaitFailure = + (delegate* unmanaged)Export("mxc_test_fake_sandbox_new_wait_failure"); + private static readonly delegate* unmanaged AwaitWait = (delegate* unmanaged)Export("mxc_test_fake_control_await_wait"); @@ -145,6 +184,20 @@ public static FakeSandbox Create(int exitCode, bool exited = true, bool heldStdo return new FakeSandbox(new MxcProcess(MxcSandboxHandle.FromRaw(sandbox), timeoutMs), control); } + public static FakeSandbox CreateTryWaitFailure() + { + NativeSandbox* sandbox = null; + Assert.Equal(0, NewTryWaitFailure(&sandbox)); + return new FakeSandbox(new MxcProcess(MxcSandboxHandle.FromRaw(sandbox)), IntPtr.Zero); + } + + public static FakeSandbox CreateWaitFailure() + { + NativeSandbox* sandbox = null; + Assert.Equal(0, NewWaitFailure(&sandbox)); + return new FakeSandbox(new MxcProcess(MxcSandboxHandle.FromRaw(sandbox)), IntPtr.Zero); + } + public void AssertCleanupStarted() => Assert.True(AwaitWait(_control, 10_000) != 0, "the wait never started the native cleanup"); @@ -152,10 +205,16 @@ public void AssertCleanupStarted() => public void Dispose() { - ReleaseWaitExport(_control); - ReleaseOutputExport(_control); + if (_control != IntPtr.Zero) + { + ReleaseWaitExport(_control); + ReleaseOutputExport(_control); + } Process.Dispose(); - FreeControl(_control); + if (_control != IntPtr.Zero) + { + FreeControl(_control); + } } // Loaded through the SDK's resolver, so the fake and the SDK share one native module. diff --git a/sdk/dotnet/Microsoft.Mxc.Sdk/V1/MxcProcess.cs b/sdk/dotnet/Microsoft.Mxc.Sdk/V1/MxcProcess.cs index 185d60cc0f..125a990c0c 100644 --- a/sdk/dotnet/Microsoft.Mxc.Sdk/V1/MxcProcess.cs +++ b/sdk/dotnet/Microsoft.Mxc.Sdk/V1/MxcProcess.cs @@ -512,20 +512,33 @@ private bool TryGetTerminalStatus(out int exitCode, out bool timedOut) var running = 1; var nativeExitCode = 0; var nativeTimedOut = 0; - int status; - unsafe + MxcErrorDetail error = default; + try { - status = NativeMethods.mxc_sandbox_try_wait( - _handle.Ptr, - &nativeExitCode, - &running, - &nativeTimedOut); + int status; + unsafe + { + status = NativeMethods.mxc_sandbox_try_wait_with_error( + _handle.Ptr, + &nativeExitCode, + &running, + &nativeTimedOut, + &error); + } + if (status != (int)ErrorCode.Success) + { + throw NativeError.ToException( + status, + error, + "querying the sandbox process status failed"); + } } - if (status != (int)ErrorCode.Success) + finally { - throw new MxcException( - (ErrorCode)status, - "querying the sandbox process status failed"); + unsafe + { + NativeMethods.mxc_error_detail_free(&error); + } } exitCode = nativeExitCode; timedOut = nativeTimedOut != 0; @@ -679,23 +692,35 @@ private WaitResult FinishWait(bool timedOut) { int exit = 0; int nativeTimedOut = 0; - int status; + MxcErrorDetail error = default; try { + int status; unsafe { - status = NativeMethods.mxc_sandbox_wait(_handle.Ptr, &exit, &nativeTimedOut); + status = NativeMethods.mxc_sandbox_wait_with_error( + _handle.Ptr, + &exit, + &nativeTimedOut, + &error); + } + if (status != (int)ErrorCode.Success) + { + throw NativeError.ToException( + status, + error, + "waiting on the sandbox failed"); } + return new WaitResult { ExitCode = exit, TimedOut = timedOut || nativeTimedOut != 0 }; } finally { + unsafe + { + NativeMethods.mxc_error_detail_free(&error); + } EndFinishing(); } - if (status != (int)ErrorCode.Success) - { - throw new MxcException((ErrorCode)status, "waiting on the sandbox failed"); - } - return new WaitResult { ExitCode = exit, TimedOut = timedOut || nativeTimedOut != 0 }; } // A Dispose during the native wait leaves the release to here, before any diff --git a/sdk/dotnet/README.md b/sdk/dotnet/README.md index 68801c275f..b89af4a9a8 100644 --- a/sdk/dotnet/README.md +++ b/sdk/dotnet/README.md @@ -251,6 +251,11 @@ for complete signatures and types. Native failures are surfaced as `MxcException`; inspect `Code`, `Operation`, `NativeCode`, and `Remediation` when available. Security and operational warnings are available on `ExecutionResult.Warnings` and `MxcProcess.Warnings`. +Failures that occur after a process starts but before MXC can confirm its +terminal result retain those fields from `MxcContainer.Run`, +`MxcLifecycle.RunInContainer`, `MxcProcess.TryGetExitCode`, `Wait`, and +`WaitAsync`; they are not converted into an exit code. Repeated waits report +the same failure, while disposal still performs process cleanup. Provision returns identity, optional typed metadata, and `ProvisionResult.Warnings`. Start, stop, and deprovision return `LifecycleResult.Warnings`. Omitted native warnings become an empty array; malformed warnings fail explicitly. When diff --git a/sdk/node/README.md b/sdk/node/README.md index a8cb95160f..858d9443b5 100644 --- a/sdk/node/README.md +++ b/sdk/node/README.md @@ -253,6 +253,11 @@ for complete signatures and types. Native errors are surfaced as `MxcError` with a typed error code and optional operation, native status, and remediation. Security and operational warnings are returned in `ExecutionResult.warnings` and `MxcProcess.warnings`. +Failures that occur after a process starts but before MXC can confirm its +terminal result retain those fields from `run`, `runInContainer`, and +`MxcProcess.wait`; they are not converted into an exit result. Every caller +awaiting the process receives the same rejection, and `dispose()` still +performs process cleanup. Captured `stdout` and `stderr` are workload output; warnings and structured denial-capture metadata are not appended to those streams. Validation warnings are returned in `ValidationResult.warnings`. diff --git a/sdk/node/src/bindings/streaming.ts b/sdk/node/src/bindings/streaming.ts index 810b6bfdbc..4ebd50831b 100644 --- a/sdk/node/src/bindings/streaming.ts +++ b/sdk/node/src/bindings/streaming.ts @@ -56,11 +56,13 @@ export interface LifecycleNativeFacade { outExit: number[], outRunning: number[], outTimedOut: number[], + error: AbiErrorDetail, ): number; wait( handle: Pointer, outExit: number[], outTimedOut: number[], + error: AbiErrorDetail, completion: NativeWaitCompletion, ): void; kill(handle: Pointer): number; @@ -172,13 +174,15 @@ function bindStreamingNativeFacade( sandbox: Pointer, outExit: number[], outTimedOut: number[], + error: AbiErrorDetail, ) => number>>(handle, { - symbol: 'mxc_sandbox_wait', + symbol: 'mxc_sandbox_wait_with_error', result: 'int32_t', parameters: [ sandboxPointer, koffi.out(koffi.pointer('int32_t')), koffi.out(koffi.pointer('int32_t')), + koffi.out(koffi.pointer(AbiErrorDetailType)), ], }); @@ -262,18 +266,25 @@ function bindStreamingNativeFacade( }), tryWait: bindNativeFunction(handle, { - symbol: 'mxc_sandbox_try_wait', + symbol: 'mxc_sandbox_try_wait_with_error', result: 'int32_t', parameters: [ sandboxPointer, koffi.out(koffi.pointer('int32_t')), koffi.out(koffi.pointer('int32_t')), koffi.out(koffi.pointer('int32_t')), + koffi.out(koffi.pointer(AbiErrorDetailType)), ], }), - wait(sandbox, outExit, outTimedOut, completion) { - waitAsyncBinding.async(sandbox, outExit, outTimedOut, completion); + wait(sandbox, outExit, outTimedOut, error, completion) { + waitAsyncBinding.async( + sandbox, + outExit, + outTimedOut, + error, + completion, + ); }, kill: bindNativeFunction(handle, { @@ -380,15 +391,25 @@ class KoffiLifecycleDriver implements NativeLifecycleDriver { this.pollExit[0] = 0; this.pollRunning[0] = 1; this.pollTimedOut[0] = 0; - throwIfFailed( - this.native.tryWait( + const error = {} as AbiErrorDetail; + try { + const status = this.native.tryWait( this.handle, this.pollExit, this.pollRunning, this.pollTimedOut, - ), - 'polling sandbox process failed', - ); + error, + ); + if (status !== 0) { + throw nativeStatusError( + status, + error, + 'polling sandbox process failed', + ); + } + } finally { + this.native.freeError(error); + } return { exitCode: this.pollExit[0], running: this.pollRunning[0] !== 0, @@ -400,21 +421,41 @@ class KoffiLifecycleDriver implements NativeLifecycleDriver { return new Promise((resolve, reject) => { const exit = [0]; const timedOut = [0]; - this.native.wait(this.handle, exit, timedOut, (error, status) => { - if (error !== null) { - reject(error); - return; - } - try { - throwIfFailed(status, 'waiting for sandbox process failed'); - resolve({ - exitCode: exit[0], - timedOut: timedOut[0] !== 0, - }); - } catch (failure) { - reject(failure); - } - }); + const detail = {} as AbiErrorDetail; + let freed = false; + const freeDetail = (): void => { + if (freed) return; + freed = true; + this.native.freeError(detail); + }; + try { + this.native.wait(this.handle, exit, timedOut, detail, (error, status) => { + try { + if (error !== null) { + reject(error); + return; + } + if (status !== 0) { + throw nativeStatusError( + status, + detail, + 'waiting for sandbox process failed', + ); + } + resolve({ + exitCode: exit[0], + timedOut: timedOut[0] !== 0, + }); + } catch (failure) { + reject(failure); + } finally { + freeDetail(); + } + }); + } catch (failure) { + freeDetail(); + reject(failure); + } }); } diff --git a/sdk/node/tests/integration/wslc-e2e.test.ts b/sdk/node/tests/integration/wslc-e2e.test.ts index a1780fa0f5..53e3d3d492 100644 --- a/sdk/node/tests/integration/wslc-e2e.test.ts +++ b/sdk/node/tests/integration/wslc-e2e.test.ts @@ -26,6 +26,21 @@ import { sdk } from './test-helpers.js'; // WSLC tests require a Windows machine with WSL2 and WSLC SDK installed. // Opt-in via MXC_ENABLE_WSLC_TESTS=1 since most CI agents lack the runtime. const isWslcAvailable = os.platform() === 'win32' && process.env.MXC_ENABLE_WSLC_TESTS === '1'; +const forcedWaitFailureCommand = + 'true # mxc-test-force-wslc-get-exit-code-failure'; + +function assertForcedWaitFailure(error: unknown): boolean { + assert.ok(error instanceof MxcError); + assert.equal(error.code, 'backend_error'); + assert.equal( + error.message, + 'WslcGetProcessExitCode failed: HRESULT 0x80004005', + ); + assert.equal(error.operation, 'WslcGetProcessExitCode'); + assert.equal(error.nativeCode, '0x80004005'); + assert.equal(error.remediation, undefined); + return true; +} // Probe a small range of host ports and return the first one we can // successfully bind to. Avoids both the fixed-port collision risk (any @@ -216,6 +231,29 @@ srv.handle_request() containment: { type: 'wslc' as const, config: { image: 'alpine:latest' } }, }); + it('captured run preserves structured post-admission wait failures', { timeout: 180_000 }, async () => { + await assert.rejects( + sdk.run(stdinRequest(forcedWaitFailureCommand)), + assertForcedWaitFailure, + ); + + const recovered = await sdk.run(stdinRequest('echo RUN-RECOVERED')); + assert.equal(recovered.stdout.trim(), 'RUN-RECOVERED'); + }); + + it('spawned process wait preserves structured post-admission wait failures', { timeout: 180_000 }, async () => { + const process = await sdk.spawn(stdinRequest(forcedWaitFailureCommand)); + try { + await assert.rejects(process.wait(), assertForcedWaitFailure); + await assert.rejects(process.wait(), assertForcedWaitFailure); + } finally { + process.dispose(); + } + + const recovered = await sdk.run(stdinRequest('echo PROCESS-RECOVERED')); + assert.equal(recovered.stdout.trim(), 'PROCESS-RECOVERED'); + }); + function collectBytes(stream: NodeJS.ReadableStream | null): Promise { if (stream === null) return Promise.resolve(Buffer.alloc(0)); return new Promise((resolve, reject) => { diff --git a/sdk/node/tests/integration/wslc-state-aware.test.ts b/sdk/node/tests/integration/wslc-state-aware.test.ts index 5ebd613743..b81a9e9a4e 100644 --- a/sdk/node/tests/integration/wslc-state-aware.test.ts +++ b/sdk/node/tests/integration/wslc-state-aware.test.ts @@ -33,6 +33,8 @@ import { safeDeprovision } from './test-helpers.js'; import * as dev from '@microsoft/mxc-sdk/v1/dev'; const wslcImage = process.env.MXC_WSLC_TEST_IMAGE ?? 'alpine:latest'; +const forcedWaitFailureCommand = + 'true # mxc-test-force-wslc-get-exit-code-failure'; // WSLC tests require a Windows machine with WSL2 and WSLC SDK installed. // Opt-in via MXC_ENABLE_WSLC_TESTS=1 since most CI agents lack the runtime. @@ -74,6 +76,43 @@ function collectBytes(stream: NodeJS.ReadableStream | null): Promise { }); } +function assertForcedWaitFailure(error: unknown): boolean { + assert.ok(error instanceof MxcError); + assert.equal(error.code, 'backend_error'); + assert.equal( + error.message, + 'WslcGetProcessExitCode failed: HRESULT 0x80004005', + ); + assert.equal(error.operation, 'WslcGetProcessExitCode'); + assert.equal(error.nativeCode, '0x80004005'); + assert.equal(error.remediation, undefined); + return true; +} + +async function withStartedContainer( + action: (containerId: ContainerId) => Promise, +): Promise { + const { containerId } = await provisionContainer({ + containment: 'wslc', + image: wslcImage, + }); + let started = false; + try { + await startContainer(containerId); + started = true; + await action(containerId); + } finally { + if (started) { + try { + await stopContainer(containerId); + } catch (error) { + console.error(`Cleanup stop failed for ${containerId}: ${error}`); + } + } + await safeDeprovision(containerId); + } +} + describe('WSLC state-aware lifecycle E2E', { skip: !isWslcAvailable ? 'WSLC tests require MXC_ENABLE_WSLC_TESTS=1 on Windows with WSL2 and WSLC SDK' @@ -269,4 +308,50 @@ timeoutMs: 750 }); } }, ); + + it( + 'captured exec preserves structured post-admission wait failures', + { timeout: 180_000 }, + async () => { + await withStartedContainer(async (containerId) => { + await assert.rejects( + runInContainer(containerId, { + command: forcedWaitFailureCommand, + timeoutMs: 60_000, + }), + assertForcedWaitFailure, + ); + + const recovered = await runInContainer(containerId, { + command: 'echo CAPTURED-RECOVERED', + timeoutMs: 60_000, + }); + assert.equal(recovered.stdout.trim(), 'CAPTURED-RECOVERED'); + }); + }, + ); + + it( + 'process wait preserves structured post-admission wait failures', + { timeout: 180_000 }, + async () => { + await withStartedContainer(async (containerId) => { + const process = await spawnInContainer(containerId, { + command: forcedWaitFailureCommand, + timeoutMs: 60_000, + }); + try { + await assert.rejects(process.wait(), assertForcedWaitFailure); + } finally { + process.dispose(); + } + + const recovered = await runInContainer(containerId, { + command: 'echo PROCESS-RECOVERED', + timeoutMs: 60_000, + }); + assert.equal(recovered.stdout.trim(), 'PROCESS-RECOVERED'); + }); + }, + ); }); diff --git a/sdk/node/tests/unit/sandbox-process.test.ts b/sdk/node/tests/unit/sandbox-process.test.ts index 58b000061b..3ac77ebf55 100644 --- a/sdk/node/tests/unit/sandbox-process.test.ts +++ b/sdk/node/tests/unit/sandbox-process.test.ts @@ -12,6 +12,7 @@ import { type NativeLifecycleDriver, type NativeLifecycleStatus, } from '../../src/v1/container-process.js'; +import { MxcError } from '../../src/v1/errors.js'; class FakeDriver implements NativeLifecycleDriver { readonly standardInput = new PassThrough(); @@ -25,6 +26,7 @@ class FakeDriver implements NativeLifecycleDriver { timedOut: false, }; pollError: Error | undefined; + waitError: Error | undefined; timeoutKillError: Error | undefined; completeOnTimeoutKill: NativeLifecycleStatus | undefined; completeOnPoll: number | undefined; @@ -54,6 +56,7 @@ class FakeDriver implements NativeLifecycleDriver { async wait(): Promise<{ exitCode: number; timedOut: boolean }> { this.waitCount += 1; + if (this.waitError !== undefined) throw this.waitError; if (this.deferWait) { this.waitInFlight = true; return new Promise((resolve) => { @@ -276,6 +279,34 @@ describe('native sandbox process', () => { assert.strictEqual(driver.freeCount, 1); }); + it('shares one typed blocking-wait rejection and cleanup', async () => { + const driver = new FakeDriver(); + const scheduler = new ManualScheduler(); + const waitError = new MxcError({ + code: 'backend_unavailable', + message: 'test process wait failed', + operation: 'TestSandbox.Wait', + nativeCode: '0x80004005', + remediation: 'Retry the test process wait.', + }); + driver.waitError = waitError; + const proc = new MxcProcess(driver, undefined, scheduler); + + const first = proc.wait(); + const second = proc.wait(); + assert.strictEqual(first, second); + + driver.complete(23); + scheduler.advance(100); + + await assert.rejects(first, (error: unknown) => error === waitError); + await assert.rejects(second, (error: unknown) => error === waitError); + assert.strictEqual(driver.waitCount, 1); + assert.strictEqual(driver.freeCount, 1); + proc.dispose(); + assert.strictEqual(driver.freeCount, 1); + }); + it('disposes streams and native lifecycle ownership exactly once', async () => { const driver = new FakeDriver(); const proc = new MxcProcess(driver); diff --git a/sdk/node/tests/unit/streaming-binding.test.ts b/sdk/node/tests/unit/streaming-binding.test.ts index a6510bf721..bd803cd413 100644 --- a/sdk/node/tests/unit/streaming-binding.test.ts +++ b/sdk/node/tests/unit/streaming-binding.test.ts @@ -16,6 +16,8 @@ import { type NodeStreamDependencies, type NativeStreamFactory, } from '../../src/bindings/native-stdio.js'; +import type { AbiErrorDetail } from '../../src/bindings/native-error.js'; +import { MxcError } from '../../src/v1/errors.js'; class FakeNative implements StreamingNativeFacade { readonly handle = {}; @@ -38,6 +40,11 @@ class FakeNative implements StreamingNativeFacade { waitCount = 0; freeCount = 0; freeErrorCount = 0; + tryWaitStatus = 0; + waitStatus = 0; + readonly tryWaitDetails: AbiErrorDetail[] = []; + readonly waitDetails: AbiErrorDetail[] = []; + readonly freedErrors = new Set(); warningsStatus = 0; outputMetadataStatus = 0; deferWait = false; @@ -45,6 +52,7 @@ class FakeNative implements StreamingNativeFacade { | { exit: number[]; timedOut: number[]; + error: AbiErrorDetail; completion: (error: Error | null, status: number) => void; } | undefined; @@ -173,28 +181,34 @@ class FakeNative implements StreamingNativeFacade { exit: number[], running: number[], timedOut: number[], + error: AbiErrorDetail, ): number { + this.tryWaitDetails.push(error); exit[0] = 0; running[0] = 1; timedOut[0] = 0; - return 0; + this.fillWaitError(error, this.tryWaitStatus); + return this.tryWaitStatus; } wait( _handle: unknown, exit: number[], timedOut: number[], + error: AbiErrorDetail, completion: (error: Error | null, status: number) => void, ): void { this.waitCount += 1; + this.waitDetails.push(error); + this.fillWaitError(error, this.waitStatus); if (this.deferWait) { - this.pendingWait = { exit, timedOut, completion }; + this.pendingWait = { exit, timedOut, error, completion }; return; } queueMicrotask(() => { exit[0] = 0; timedOut[0] = 0; - completion(null, 0); + completion(null, this.waitStatus); }); } @@ -204,7 +218,7 @@ class FakeNative implements StreamingNativeFacade { this.pendingWait = undefined; pending!.exit[0] = exitCode; pending!.timedOut[0] = timedOut ? 1 : 0; - pending!.completion(null, 0); + pending!.completion(null, this.waitStatus); } kill(): number { @@ -232,13 +246,23 @@ class FakeNative implements StreamingNativeFacade { queueMicrotask(() => completion(null)); } - freeError(): void { + freeError(error: AbiErrorDetail): void { + assert.strictEqual(this.freedErrors.has(error), false, 'error detail freed twice'); + this.freedErrors.add(error); this.freeErrorCount += 1; } freeString(value: unknown): void { this.freedStrings.push(value); } + + private fillWaitError(error: AbiErrorDetail, status: number): void { + if (status === 0) return; + error.message = Buffer.from('test process wait failed\0'); + error.operation = Buffer.from('TestSandbox.Wait\0'); + error.nativeCode = Buffer.from('0x80004005\0'); + error.remediation = Buffer.from('Retry the test process wait.\0'); + } } class FakeStreams implements NativeStreamFactory { @@ -482,10 +506,82 @@ describe('native streaming binding ownership', () => { native.completeWait(31, true); assert.deepStrictEqual(await wait, { exitCode: 31, timedOut: true }); + assert.strictEqual(native.freeErrorCount, 1); await driver.free(); assert.strictEqual(native.freeCount, 1); }); + it('decodes and frees a structured polling failure', async () => { + const native = new FakeNative(); + native.tryWaitStatus = 4; + const driver = await createStreamingDriver( + {} as never, + native, + new FakeStreams(), + ); + + assert.throws( + () => driver.poll(), + (error: unknown) => { + assert.ok(error instanceof MxcError); + assert.strictEqual(error.code, 'backend_unavailable'); + assert.strictEqual(error.message, 'test process wait failed'); + assert.strictEqual(error.operation, 'TestSandbox.Wait'); + assert.strictEqual(error.nativeCode, '0x80004005'); + assert.strictEqual(error.remediation, 'Retry the test process wait.'); + return true; + }, + ); + assert.strictEqual(native.tryWaitDetails.length, 1); + assert.strictEqual(native.freeErrorCount, 1); + await driver.free(); + }); + + it('decodes and frees a structured blocking-wait failure', async () => { + const native = new FakeNative(); + native.waitStatus = 4; + const driver = await createStreamingDriver( + {} as never, + native, + new FakeStreams(), + ); + + await assert.rejects( + driver.wait(), + (error: unknown) => { + assert.ok(error instanceof MxcError); + assert.strictEqual(error.code, 'backend_unavailable'); + assert.strictEqual(error.message, 'test process wait failed'); + assert.strictEqual(error.operation, 'TestSandbox.Wait'); + assert.strictEqual(error.nativeCode, '0x80004005'); + assert.strictEqual(error.remediation, 'Retry the test process wait.'); + return true; + }, + ); + assert.strictEqual(native.waitDetails.length, 1); + assert.strictEqual(native.freeErrorCount, 1); + await driver.free(); + }); + + it('uses and frees a fresh error detail for each wait operation', async () => { + const native = new FakeNative(); + const driver = await createStreamingDriver( + {} as never, + native, + new FakeStreams(), + ); + + driver.poll(); + driver.poll(); + await driver.wait(); + + assert.strictEqual(native.tryWaitDetails.length, 2); + assert.notStrictEqual(native.tryWaitDetails[0], native.tryWaitDetails[1]); + assert.notStrictEqual(native.tryWaitDetails[1], native.waitDetails[0]); + assert.strictEqual(native.freeErrorCount, 3); + await driver.free(); + }); + it('treats platform-specific missing endpoint sentinels as absent', async () => { const unixNative = new FakeNative(); unixNative.stdinHandle = -1; diff --git a/src/ffi/mxc_ffi/src/streaming.rs b/src/ffi/mxc_ffi/src/streaming.rs index 2602b269a9..0810ba6dd1 100644 --- a/src/ffi/mxc_ffi/src/streaming.rs +++ b/src/ffi/mxc_ffi/src/streaming.rs @@ -70,7 +70,9 @@ use std::ptr; use mxc_sdk::__ffi::{spawn_container_json, spawn_container_with_stdin_json}; use mxc_sdk::mxc_common::models::SandboxOutputMetadata; use mxc_sdk::mxc_common::sandbox_process::NativeStdio; -use mxc_sdk::v1::{MxcProcess, MxcPtyProcess, MxcPtySize, StreamCloser, WaitResult}; +use mxc_sdk::v1::{ + Error, ErrorCode, MxcProcess, MxcPtyProcess, MxcPtySize, StreamCloser, WaitResult, +}; use crate::{ alloc_cstring, cstr_to_str, status_from_error_code, MxcErrorDetail, MXC_STATUS_BACKEND_ERROR, @@ -116,6 +118,8 @@ impl MxcSandbox { } } +pub(crate) type DetailedWaitResult = Result; + pub(crate) trait LiveSandbox: Send { fn warnings(&self) -> Vec; fn output_metadata(&self) -> Option; @@ -126,10 +130,31 @@ pub(crate) trait LiveSandbox: Send { fn stdout_closer(&self) -> Option; fn stderr_closer(&self) -> Option; fn try_wait(&mut self) -> std::io::Result>; + fn try_wait_detailed(&mut self) -> DetailedWaitResult> { + match self.try_wait() { + Ok(Some(code)) => Ok(Some(WaitResult::Exited(code))), + Ok(None) => Ok(None), + Err(error) if error.kind() == std::io::ErrorKind::TimedOut => { + Ok(Some(WaitResult::TimedOut)) + } + Err(error) => Err(Error::new( + ErrorCode::BackendError, + format!("querying the sandbox process status failed: {error}"), + )), + } + } fn id(&self) -> u32; fn kill(&mut self) -> std::io::Result<()>; fn kill_for_timeout(&mut self) -> std::io::Result<()>; fn wait(&mut self) -> std::io::Result; + fn wait_detailed(&mut self) -> DetailedWaitResult { + self.wait().map_err(|error| { + Error::new( + ErrorCode::BackendError, + format!("waiting for the sandbox process failed: {error}"), + ) + }) + } fn resize_pty(&self, _size: MxcPtySize) -> std::io::Result<()> { Err(std::io::Error::new( std::io::ErrorKind::Unsupported, @@ -175,6 +200,10 @@ impl LiveSandbox for MxcProcess { self.try_wait() } + fn try_wait_detailed(&mut self) -> DetailedWaitResult> { + MxcProcess::try_wait_detailed(self) + } + fn id(&self) -> u32 { self.id() } @@ -190,6 +219,10 @@ impl LiveSandbox for MxcProcess { fn wait(&mut self) -> std::io::Result { self.wait() } + + fn wait_detailed(&mut self) -> DetailedWaitResult { + MxcProcess::wait_detailed(self) + } } #[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] @@ -1050,7 +1083,7 @@ pub unsafe extern "C" fn mxc_sandbox_try_wait( let status = catch_unwind(AssertUnwindSafe(|| { // SAFETY: non-null live handle per the caller contract. let sandbox = unsafe { &mut *handle }; - match try_wait_result_to_abi(sandbox.inner.try_wait()) { + match try_wait_result_to_abi(sandbox.inner.try_wait_detailed()) { Ok((exit, running, timed_out)) => { // SAFETY: out-params non-null writable per the caller contract. unsafe { @@ -1060,7 +1093,7 @@ pub unsafe extern "C" fn mxc_sandbox_try_wait( } MXC_STATUS_SUCCESS } - Err(status) => status, + Err(_) => MXC_STATUS_BACKEND_ERROR, } })); status.unwrap_or_else(|panic| { @@ -1069,12 +1102,82 @@ pub unsafe extern "C" fn mxc_sandbox_try_wait( }) } -fn try_wait_result_to_abi(result: std::io::Result>) -> Result<(i32, i32, i32), i32> { +fn try_wait_result_to_abi( + result: DetailedWaitResult>, +) -> DetailedWaitResult<(i32, i32, i32)> { match result { - Ok(Some(code)) => Ok((code, 0, 0)), + Ok(Some(WaitResult::Exited(code))) => Ok((code, 0, 0)), + Ok(Some(WaitResult::TimedOut)) => Ok((-1, 0, 1)), Ok(None) => Ok((0, 1, 0)), - Err(error) if error.kind() == std::io::ErrorKind::TimedOut => Ok((-1, 0, 1)), - Err(_) => Err(MXC_STATUS_BACKEND_ERROR), + Err(error) => Err(error), + } +} + +/// Non-blocking completion check that preserves structured wait failures. +/// +/// `out_error` may be null. A non-null detail must be freed with +/// [`crate::mxc_error_detail_free`]. +/// +/// # Safety +/// `handle` and the numeric outputs follow [`mxc_sandbox_try_wait`]'s contract. +/// `out_error`, when non-null, must point to writable [`MxcErrorDetail`] storage. +#[no_mangle] +pub unsafe extern "C" fn mxc_sandbox_try_wait_with_error( + handle: *mut MxcSandbox, + out_exit: *mut i32, + out_running: *mut i32, + out_timed_out: *mut i32, + out_error: *mut MxcErrorDetail, +) -> i32 { + unsafe { + if !out_exit.is_null() { + *out_exit = 0; + } + if !out_running.is_null() { + *out_running = 0; + } + if !out_timed_out.is_null() { + *out_timed_out = 0; + } + if !out_error.is_null() { + ptr::write(out_error, MxcErrorDetail::none()); + } + } + if handle.is_null() || out_exit.is_null() || out_running.is_null() || out_timed_out.is_null() { + return MXC_STATUS_NULL_ARGUMENT; + } + + let outcome = catch_unwind(AssertUnwindSafe(|| { + let sandbox = unsafe { &mut *handle }; + try_wait_result_to_abi(sandbox.inner.try_wait_detailed()) + })); + match outcome { + Ok(Ok((exit, running, timed_out))) => { + unsafe { + *out_exit = exit; + *out_running = running; + *out_timed_out = timed_out; + } + MXC_STATUS_SUCCESS + } + Ok(Err(error)) => { + if !out_error.is_null() { + unsafe { ptr::write(out_error, MxcErrorDetail::from_error(&error)) }; + } + status_from_error_code(error.code) + } + Err(panic) => { + crate::report_panic("mxc_sandbox_try_wait_with_error", &*panic); + if !out_error.is_null() { + unsafe { + ptr::write( + out_error, + MxcErrorDetail::from_message("the mxc engine panicked"), + ) + }; + } + MXC_STATUS_PANIC + } } } @@ -1109,20 +1212,12 @@ pub unsafe extern "C" fn mxc_sandbox_wait( let status = catch_unwind(AssertUnwindSafe(|| { // SAFETY: non-null live handle per the caller contract. let sandbox = unsafe { &mut *handle }; - match sandbox.inner.wait() { - Ok(WaitResult::Exited(code)) => { + match wait_result_to_abi(sandbox.inner.wait_detailed()) { + Ok((code, timed_out)) => { // SAFETY: out-params non-null writable per the caller contract. unsafe { *out_exit = code; - *out_timed_out = 0; - } - MXC_STATUS_SUCCESS - } - Ok(WaitResult::TimedOut) => { - // SAFETY: out-params non-null writable per the caller contract. - unsafe { - *out_exit = -1; - *out_timed_out = 1; + *out_timed_out = timed_out; } MXC_STATUS_SUCCESS } @@ -1135,6 +1230,77 @@ pub unsafe extern "C" fn mxc_sandbox_wait( }) } +fn wait_result_to_abi(result: DetailedWaitResult) -> DetailedWaitResult<(i32, i32)> { + match result { + Ok(WaitResult::Exited(code)) => Ok((code, 0)), + Ok(WaitResult::TimedOut) => Ok((-1, 1)), + Err(error) => Err(error), + } +} + +/// Blocking wait that preserves structured backend failures. +/// +/// `out_error` may be null. A non-null detail must be freed with +/// [`crate::mxc_error_detail_free`]. +/// +/// # Safety +/// `handle` and the numeric outputs follow [`mxc_sandbox_wait`]'s contract. +/// `out_error`, when non-null, must point to writable [`MxcErrorDetail`] storage. +#[no_mangle] +pub unsafe extern "C" fn mxc_sandbox_wait_with_error( + handle: *mut MxcSandbox, + out_exit: *mut i32, + out_timed_out: *mut i32, + out_error: *mut MxcErrorDetail, +) -> i32 { + unsafe { + if !out_exit.is_null() { + *out_exit = 0; + } + if !out_timed_out.is_null() { + *out_timed_out = 0; + } + if !out_error.is_null() { + ptr::write(out_error, MxcErrorDetail::none()); + } + } + if handle.is_null() || out_exit.is_null() || out_timed_out.is_null() { + return MXC_STATUS_NULL_ARGUMENT; + } + + let outcome = catch_unwind(AssertUnwindSafe(|| { + let sandbox = unsafe { &mut *handle }; + wait_result_to_abi(sandbox.inner.wait_detailed()) + })); + match outcome { + Ok(Ok((exit, timed_out))) => { + unsafe { + *out_exit = exit; + *out_timed_out = timed_out; + } + MXC_STATUS_SUCCESS + } + Ok(Err(error)) => { + if !out_error.is_null() { + unsafe { ptr::write(out_error, MxcErrorDetail::from_error(&error)) }; + } + status_from_error_code(error.code) + } + Err(panic) => { + crate::report_panic("mxc_sandbox_wait_with_error", &*panic); + if !out_error.is_null() { + unsafe { + ptr::write( + out_error, + MxcErrorDetail::from_message("the mxc engine panicked"), + ) + }; + } + MXC_STATUS_PANIC + } + } +} + /// Kill the child and its whole process tree. Reaping happens in a subsequent /// [`mxc_sandbox_wait`] / [`mxc_sandbox_try_wait`] or in [`mxc_sandbox_free`]. /// @@ -1276,27 +1442,284 @@ pub unsafe extern "C" fn mxc_stream_closer_free(closer: *mut MxcStreamCloser) { #[cfg(test)] mod tests { use super::*; + use std::ffi::{CStr, CString}; use std::sync::atomic::{AtomicU64, Ordering}; #[test] fn try_wait_preserves_timeout_as_a_terminal_outcome() { - assert_eq!(try_wait_result_to_abi(Ok(Some(42))), Ok((42, 0, 0))); + assert_eq!( + try_wait_result_to_abi(Ok(Some(WaitResult::Exited(42)))), + Ok((42, 0, 0)) + ); assert_eq!(try_wait_result_to_abi(Ok(None)), Ok((0, 1, 0))); assert_eq!( - try_wait_result_to_abi(Err(std::io::Error::new( - std::io::ErrorKind::TimedOut, - "deadline elapsed" - ))), + try_wait_result_to_abi(Ok(Some(WaitResult::TimedOut))), Ok((-1, 0, 1)) ); + let error = Error::new(ErrorCode::BackendError, "wait failed"); + assert_eq!(try_wait_result_to_abi(Err(error.clone())), Err(error)); + } + use crate::MXC_STATUS_MALFORMED_REQUEST; + + fn error_field(value: *mut c_char) -> Option { + (!value.is_null()).then(|| { + unsafe { CStr::from_ptr(value) } + .to_string_lossy() + .into_owned() + }) + } + + #[cfg(feature = "test-support")] + fn assert_test_wait_error(detail: &MxcErrorDetail) { assert_eq!( - try_wait_result_to_abi(Err(std::io::Error::other("wait failed"))), - Err(MXC_STATUS_BACKEND_ERROR) + error_field(detail.message_utf8).as_deref(), + Some("test process wait failed") + ); + assert_eq!( + error_field(detail.operation_utf8).as_deref(), + Some("TestSandbox.Wait") + ); + assert_eq!( + error_field(detail.native_code_utf8).as_deref(), + Some("0x80004005") + ); + assert_eq!( + error_field(detail.remediation_utf8).as_deref(), + Some("Retry the test process wait.") ); } - use std::ffi::CString; - use crate::MXC_STATUS_MALFORMED_REQUEST; + #[cfg(feature = "test-support")] + #[test] + fn detailed_try_wait_preserves_fields_and_legacy_status() { + let mut handle = ptr::null_mut(); + assert_eq!( + unsafe { + crate::test_fake_sandbox::mxc_test_fake_sandbox_new_try_wait_failure(&mut handle) + }, + MXC_STATUS_SUCCESS + ); + + for _ in 0..2 { + let mut exit = 91; + let mut running = 92; + let mut timed_out = 93; + let mut detail = MxcErrorDetail::none(); + let status = unsafe { + mxc_sandbox_try_wait_with_error( + handle, + &mut exit, + &mut running, + &mut timed_out, + &mut detail, + ) + }; + + assert_eq!(status, MXC_STATUS_BACKEND_UNAVAILABLE); + assert_eq!((exit, running, timed_out), (0, 0, 0)); + assert_test_wait_error(&detail); + unsafe { crate::mxc_error_detail_free(&mut detail) }; + assert!(detail.message_utf8.is_null()); + } + + let mut exit = 91; + let mut running = 92; + let mut timed_out = 93; + assert_eq!( + unsafe { mxc_sandbox_try_wait(handle, &mut exit, &mut running, &mut timed_out) }, + MXC_STATUS_BACKEND_ERROR + ); + assert_eq!((exit, running, timed_out), (0, 0, 0)); + + unsafe { mxc_sandbox_free(handle) }; + } + + #[cfg(feature = "test-support")] + #[test] + fn detailed_wait_preserves_fields_after_a_running_poll() { + let mut handle = ptr::null_mut(); + assert_eq!( + unsafe { + crate::test_fake_sandbox::mxc_test_fake_sandbox_new_wait_failure(&mut handle) + }, + MXC_STATUS_SUCCESS + ); + + let mut exit = 91; + let mut running = 92; + let mut timed_out = 93; + let mut detail = MxcErrorDetail::none(); + assert_eq!( + unsafe { + mxc_sandbox_try_wait_with_error( + handle, + &mut exit, + &mut running, + &mut timed_out, + &mut detail, + ) + }, + MXC_STATUS_SUCCESS + ); + assert_eq!((exit, running, timed_out), (0, 1, 0)); + assert!(detail.message_utf8.is_null()); + + assert_eq!( + unsafe { + mxc_sandbox_try_wait_with_error( + handle, + &mut exit, + &mut running, + &mut timed_out, + ptr::null_mut(), + ) + }, + MXC_STATUS_SUCCESS + ); + assert_eq!((exit, running, timed_out), (23, 0, 0)); + + for _ in 0..2 { + let mut detail = MxcErrorDetail::none(); + assert_eq!( + unsafe { + mxc_sandbox_wait_with_error(handle, &mut exit, &mut timed_out, &mut detail) + }, + MXC_STATUS_BACKEND_UNAVAILABLE + ); + assert_eq!((exit, timed_out), (0, 0)); + assert_test_wait_error(&detail); + unsafe { crate::mxc_error_detail_free(&mut detail) }; + } + + assert_eq!( + unsafe { mxc_sandbox_wait(handle, &mut exit, &mut timed_out) }, + MXC_STATUS_BACKEND_ERROR + ); + assert_eq!((exit, timed_out), (0, 0)); + + unsafe { mxc_sandbox_free(handle) }; + } + + #[test] + fn detailed_wait_initializes_outputs_before_rejecting_a_null_handle() { + let mut exit = 91; + let mut running = 92; + let mut timed_out = 93; + let mut detail = MxcErrorDetail::none(); + assert_eq!( + unsafe { + mxc_sandbox_try_wait_with_error( + ptr::null_mut(), + &mut exit, + &mut running, + &mut timed_out, + &mut detail, + ) + }, + MXC_STATUS_NULL_ARGUMENT + ); + assert_eq!((exit, running, timed_out), (0, 0, 0)); + assert!(detail.message_utf8.is_null()); + } + + struct PanicWaitSandbox; + + impl LiveSandbox for PanicWaitSandbox { + fn warnings(&self) -> Vec { + Vec::new() + } + + fn output_metadata(&self) -> Option { + None + } + + fn take_native_stdio(&mut self) -> std::io::Result> { + Ok(None) + } + + fn take_stdin(&mut self) -> Option> { + None + } + + fn take_stdout(&mut self) -> Option> { + None + } + + fn take_stderr(&mut self) -> Option> { + None + } + + fn stdout_closer(&self) -> Option { + None + } + + fn stderr_closer(&self) -> Option { + None + } + + fn try_wait(&mut self) -> std::io::Result> { + panic!("forced try-wait panic") + } + + fn id(&self) -> u32 { + 1 + } + + fn kill(&mut self) -> std::io::Result<()> { + Ok(()) + } + + fn kill_for_timeout(&mut self) -> std::io::Result<()> { + Ok(()) + } + + fn wait(&mut self) -> std::io::Result { + panic!("forced wait panic") + } + } + + #[test] + fn detailed_wait_catches_panics_and_sets_error_detail() { + let mut handle = MxcSandbox { + inner: Box::new(PanicWaitSandbox), + }; + let mut exit = 91; + let mut running = 92; + let mut timed_out = 93; + let mut detail = MxcErrorDetail::none(); + + assert_eq!( + unsafe { + mxc_sandbox_try_wait_with_error( + &mut handle, + &mut exit, + &mut running, + &mut timed_out, + &mut detail, + ) + }, + MXC_STATUS_PANIC + ); + assert_eq!((exit, running, timed_out), (0, 0, 0)); + assert_eq!( + error_field(detail.message_utf8).as_deref(), + Some("the mxc engine panicked") + ); + unsafe { crate::mxc_error_detail_free(&mut detail) }; + + assert_eq!( + unsafe { + mxc_sandbox_wait_with_error(&mut handle, &mut exit, &mut timed_out, &mut detail) + }, + MXC_STATUS_PANIC + ); + assert_eq!((exit, timed_out), (0, 0)); + assert_eq!( + error_field(detail.message_utf8).as_deref(), + Some("the mxc engine panicked") + ); + unsafe { crate::mxc_error_detail_free(&mut detail) }; + } static NEXT_CONTAINER_ID: AtomicU64 = AtomicU64::new(0); diff --git a/src/ffi/mxc_ffi/src/test_fake_sandbox.rs b/src/ffi/mxc_ffi/src/test_fake_sandbox.rs index 331d6a8018..05e92e3cc2 100644 --- a/src/ffi/mxc_ffi/src/test_fake_sandbox.rs +++ b/src/ffi/mxc_ffi/src/test_fake_sandbox.rs @@ -10,7 +10,7 @@ use std::time::Duration; use mxc_sdk::mxc_common::models::SandboxOutputMetadata; use mxc_sdk::mxc_common::sandbox_process::NativeStdio; -use mxc_sdk::v1::{StreamCloser, WaitResult}; +use mxc_sdk::v1::{Error, ErrorCode, StreamCloser, WaitResult}; use crate::streaming::{LiveSandbox, MxcSandbox}; use crate::{MXC_STATUS_NULL_ARGUMENT, MXC_STATUS_PANIC, MXC_STATUS_SUCCESS}; @@ -57,6 +57,24 @@ struct FakeSandbox { control: Arc, stdout: Option, exit_code: i32, + wait_failure: WaitFailure, + poll_count: u32, +} + +#[derive(Clone, Copy, Default)] +enum WaitFailure { + #[default] + None, + TryWait, + WaitAfterRunningPoll, +} + +fn test_wait_error() -> Error { + let mut error = Error::new(ErrorCode::BackendUnavailable, "test process wait failed"); + error.operation = Some("TestSandbox.Wait".to_string()); + error.native_code = Some("0x80004005".to_string()); + error.remediation = Some("Retry the test process wait.".to_string()); + error } /// Reports end-of-file only once the test releases it, like a pipe a @@ -106,13 +124,36 @@ impl LiveSandbox for FakeSandbox { } fn try_wait(&mut self) -> std::io::Result> { + match self.try_wait_detailed() { + Ok(Some(WaitResult::Exited(code))) => Ok(Some(code)), + Ok(Some(WaitResult::TimedOut)) => Err(std::io::Error::new( + std::io::ErrorKind::TimedOut, + "the fake process timed out", + )), + Ok(None) => Ok(None), + Err(error) => Err(std::io::Error::other(error.message)), + } + } + + fn try_wait_detailed(&mut self) -> super::streaming::DetailedWaitResult> { + match self.wait_failure { + WaitFailure::TryWait => return Err(test_wait_error()), + WaitFailure::WaitAfterRunningPoll if self.poll_count == 0 => { + self.poll_count += 1; + return Ok(None); + } + WaitFailure::WaitAfterRunningPoll => { + return Ok(Some(WaitResult::Exited(self.exit_code))); + } + WaitFailure::None => {} + } let exited = self .control .state .lock() .unwrap_or_else(|e| e.into_inner()) .exited; - Ok(exited.then_some(self.exit_code)) + Ok(exited.then_some(WaitResult::Exited(self.exit_code))) } fn id(&self) -> u32 { @@ -129,6 +170,14 @@ impl LiveSandbox for FakeSandbox { } fn wait(&mut self) -> std::io::Result { + self.wait_detailed() + .map_err(|error| std::io::Error::other(error.message)) + } + + fn wait_detailed(&mut self) -> super::streaming::DetailedWaitResult { + if matches!(self.wait_failure, WaitFailure::WaitAfterRunningPoll) { + return Err(test_wait_error()); + } self.control.update(|s| s.wait_entered = true); self.control.wait_until(|s| s.wait_released, None); Ok(WaitResult::Exited(self.exit_code)) @@ -162,6 +211,8 @@ pub unsafe extern "C" fn mxc_test_fake_sandbox_new( control: Arc::clone(&control), stdout: (held_stdout != 0).then(|| HeldOutput(Arc::clone(&control))), exit_code, + wait_failure: WaitFailure::None, + poll_count: 0, }; // SAFETY: both out pointers are non-null and writable per the caller contract. unsafe { @@ -173,6 +224,51 @@ pub unsafe extern "C" fn mxc_test_fake_sandbox_new( .unwrap_or(MXC_STATUS_PANIC) } +unsafe fn new_wait_failure_sandbox(failure: WaitFailure, out_sandbox: *mut *mut MxcSandbox) -> i32 { + if !out_sandbox.is_null() { + unsafe { *out_sandbox = std::ptr::null_mut() }; + } + if out_sandbox.is_null() { + return MXC_STATUS_NULL_ARGUMENT; + } + catch_unwind(AssertUnwindSafe(|| { + let sandbox = FakeSandbox { + control: Arc::new(MxcTestFakeControl::default()), + stdout: None, + exit_code: 23, + wait_failure: failure, + poll_count: 0, + }; + unsafe { + *out_sandbox = Box::into_raw(Box::new(MxcSandbox::from_live(Box::new(sandbox)))); + } + MXC_STATUS_SUCCESS + })) + .unwrap_or(MXC_STATUS_PANIC) +} + +/// Create a fake sandbox whose first status poll returns a structured failure. +/// +/// # Safety +/// `out_sandbox` must be null or writable pointer storage. +#[no_mangle] +pub unsafe extern "C" fn mxc_test_fake_sandbox_new_try_wait_failure( + out_sandbox: *mut *mut MxcSandbox, +) -> i32 { + unsafe { new_wait_failure_sandbox(WaitFailure::TryWait, out_sandbox) } +} + +/// Create a fake sandbox whose blocking wait fails after one running poll. +/// +/// # Safety +/// `out_sandbox` must be null or writable pointer storage. +#[no_mangle] +pub unsafe extern "C" fn mxc_test_fake_sandbox_new_wait_failure( + out_sandbox: *mut *mut MxcSandbox, +) -> i32 { + unsafe { new_wait_failure_sandbox(WaitFailure::WaitAfterRunningPoll, out_sandbox) } +} + /// Return 1 once the fake sandbox's native wait has started, or 0 if it has /// not started within `timeout_ms`. /// diff --git a/src/mxc-sdk/README.md b/src/mxc-sdk/README.md index 0044616aba..e21c6f3969 100644 --- a/src/mxc-sdk/README.md +++ b/src/mxc-sdk/README.md @@ -54,6 +54,15 @@ Captured execution drains both streams concurrently. Output-read failures are returned as errors; timeouts cancel outstanding reads and return the partial output with `WaitResult::TimedOut`. +`run` and the container lifecycle captured-run helpers preserve structured +backend failures that occur while obtaining the terminal process result. +For live processes, use `try_wait_detailed`, `wait_detailed`, or +`wait_with_output_detailed` when the caller needs `ErrorCode`, `operation`, +`native_code`, `remediation`, and backend `details`. The existing `try_wait`, +`wait`, and `wait_with_output` methods keep their `std::io::Error` signatures +and message-only rendering for compatibility. A terminal wait failure is +replayed by later waits and does not disable kill or drop cleanup. + Creation takes `RunOptions`, `SpawnOptions`, or `SpawnWithPtyOptions` after the request. `SpawnWithPtyOptions.size` carries initial dimensions and defaults to 24 rows by 80 columns. 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..7db668d036 100644 --- a/src/mxc-sdk/src/backends/wslc/common/container_steps.rs +++ b/src/mxc-sdk/src/backends/wslc/common/container_steps.rs @@ -1304,6 +1304,23 @@ fn natural_exit_confirmed(exit_wait: ExitWait, callback_confirmed: bool) -> bool || (matches!(exit_wait, ExitWait::NoEvent) && callback_confirmed) } +#[cfg(all(feature = "test-support", debug_assertions))] +const TEST_EXIT_CODE_FAILURE_SCRIPT: &str = "true # mxc-test-force-wslc-get-exit-code-failure"; + +pub(crate) fn test_exit_code_result( + script_code: &str, + natural_exit_confirmed: bool, + result: HRESULT, +) -> HRESULT { + #[cfg(all(feature = "test-support", debug_assertions))] + if result == S_OK && natural_exit_confirmed && script_code == TEST_EXIT_CODE_FAILURE_SCRIPT { + return 0x8000_4005_u32 as HRESULT; + } + + let _ = (script_code, natural_exit_confirmed); + result +} + fn classify_completion(exit_wait: ExitWait, confirmed: bool, exit_code: i32) -> ProcessCompletion { if natural_exit_confirmed(exit_wait, confirmed) { return ProcessCompletion::Confirmed(ExecTerminal::Exited(exit_code)); @@ -1505,9 +1522,14 @@ pub unsafe fn exec_in_container( } let mut exit_code: i32 = -1; - let hr = sdk.WslcGetProcessExitCode(process_guard.as_raw(), &mut exit_code); + let natural_exit_confirmed = natural_exit_confirmed(exit_wait, confirmed); + let hr = test_exit_code_result( + script_code, + natural_exit_confirmed, + sdk.WslcGetProcessExitCode(process_guard.as_raw(), &mut exit_code), + ); if hr != S_OK { - if natural_exit_confirmed(exit_wait, confirmed) { + if natural_exit_confirmed { return Err(sdk_step_error("WslcGetProcessExitCode", hr, "")); } let _ = writeln!( @@ -1711,6 +1733,36 @@ mod tests { use super::*; use crate::mxc_common::models::FailurePhase; + #[cfg(all(feature = "test-support", debug_assertions))] + #[test] + fn test_exit_code_failure_requires_the_exact_successful_natural_exit() { + let forced = 0x8000_4005_u32 as HRESULT; + assert_eq!( + test_exit_code_result(TEST_EXIT_CODE_FAILURE_SCRIPT, true, S_OK), + forced + ); + assert_eq!( + test_exit_code_result( + TEST_EXIT_CODE_FAILURE_SCRIPT, + true, + 0x8007_0005_u32 as HRESULT + ), + 0x8007_0005_u32 as HRESULT + ); + assert_eq!( + test_exit_code_result(TEST_EXIT_CODE_FAILURE_SCRIPT, false, S_OK), + S_OK + ); + assert_eq!( + test_exit_code_result( + "true # mxc-test-force-wslc-get-exit-code-failure ", + true, + S_OK + ), + S_OK + ); + } + /// A modest machine: 16 GB and 8 logical processors. const TEST_HOST: HostCapacity = HostCapacity { total_memory_mb: 16 * 1024, diff --git a/src/mxc-sdk/src/backends/wslc/common/error.rs b/src/mxc-sdk/src/backends/wslc/common/error.rs index d64e92c3c3..3c88202ac9 100644 --- a/src/mxc-sdk/src/backends/wslc/common/error.rs +++ b/src/mxc-sdk/src/backends/wslc/common/error.rs @@ -17,7 +17,7 @@ use std::fmt; use crate::mxc_common::models::{FailurePhase, ScriptResponse}; -use crate::mxc_common::mxc_error::ApiFailure; +use crate::mxc_common::mxc_error::{ApiFailure, MxcError}; use crate::wslc_common::wslc_bindings::HRESULT; @@ -37,6 +37,18 @@ impl WslcStepError { pub fn into_response(self) -> ScriptResponse { ScriptResponse::failed(self.failure_phase, &self.error_message) } + + pub(crate) fn into_mxc_error(self) -> MxcError { + let error = match self.failure_phase { + FailurePhase::BackendUnavailable => MxcError::backend_unavailable(self.error_message), + FailurePhase::Rejected => MxcError::policy_validation(self.error_message), + _ => MxcError::backend_error(self.error_message), + }; + match self.api_failure { + Some(failure) => error.with_api_failure(*failure), + None => error, + } + } } impl From for WslcStepError { @@ -226,6 +238,22 @@ mod tests { ); } + #[test] + fn a_step_error_preserves_api_failure_in_mxc_error() { + let hr = 0x8000_4005_u32 as HRESULT; + let error = WslcError::sdk("WslcGetProcessExitCode failed", hr, "") + .into_step_error() + .with_api_failure(sdk_api_failure("WslcGetProcessExitCode", hr)) + .into_mxc_error(); + + assert_eq!( + error.message, + "WslcGetProcessExitCode failed: HRESULT 0x80004005" + ); + assert_eq!(error.operation(), Some("WslcGetProcessExitCode")); + assert_eq!(error.native_code(), Some("0x80004005")); + } + /// 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/sandbox.rs b/src/mxc-sdk/src/backends/wslc/common/sandbox.rs index 37c30fc346..2259b3404e 100644 --- a/src/mxc-sdk/src/backends/wslc/common/sandbox.rs +++ b/src/mxc-sdk/src/backends/wslc/common/sandbox.rs @@ -35,6 +35,7 @@ use std::sync::mpsc::{sync_channel, SyncSender}; use crate::mxc_common::logger::{Logger, Mode}; use crate::mxc_common::models::{ExecutionRequest, ScriptResponse}; use crate::mxc_common::module_lifetime::pin_current_module; +use crate::mxc_common::mxc_error::{into_io_error, MxcError}; use crate::mxc_common::process_util::{ create_local_pipe, InterruptiblePipeReader, InterruptiblePipeWriter, PipeWriteCanceller, PipeWriter, @@ -50,7 +51,7 @@ use windows::core::PCWSTR; use windows::Win32::Foundation::{HANDLE, WAIT_OBJECT_0}; use windows::Win32::System::Threading::{CreateEventW, SetEvent, WaitForMultipleObjects}; -use crate::wslc_common::error::WslcError; +use crate::wslc_common::error::{WslcError, WslcStepError}; use crate::wslc_common::stream_buffer::{StreamCanceller, StreamReader}; use crate::wslc_common::wsl_container_runner::{ InitStdin, OutputMode, StartedContainer, WSLContainerRunner, @@ -197,6 +198,7 @@ struct WslcSandboxProcess { /// Exit code cached by the first successful wait, so repeat waits are /// idempotent (and don't re-run teardown). exit: Option, + wait_failure: Option, /// Set once the wait deadline elapsed *and* the kill was confirmed, so /// `TimedOut` is reported stickily rather than a later-observed exit code. /// Deliberately narrower than [`deadline_elapsed`](Self::deadline_elapsed): @@ -301,6 +303,7 @@ impl WslcSandboxProcess { stdin_failure: None, started, exit: None, + wait_failure: None, timed_out: false, deadline_elapsed: false, torn_down: false, @@ -313,6 +316,16 @@ impl WslcSandboxProcess { settled_outcome(self.timed_out, self.deadline_elapsed, self.exit) } + fn replay_wait_failure(&self) -> Option { + self.wait_failure.clone().map(into_io_error) + } + + fn cache_wait_failure(&mut self, error: WslcStepError) -> std::io::Error { + let error = error.into_mxc_error(); + self.wait_failure = Some(error.clone()); + into_io_error(error) + } + /// Start closing stdin when the process exits, if it has a stdin to close. fn watch_exit(&mut self) -> std::io::Result<()> { if self.stdin.is_none() { @@ -549,6 +562,10 @@ impl SandboxProcess for WslcSandboxProcess { } fn try_wait(&mut self) -> std::io::Result> { + if let Some(error) = self.replay_wait_failure() { + return Err(error); + } + // The settled outcome is checked exactly as in `wait`. Without this, // the timeout path — which kills the container — leaves `has_exited()` // true below, so a caller that saw `wait` fail with `TimedOut` would @@ -581,9 +598,10 @@ impl SandboxProcess for WslcSandboxProcess { // turning a `TimedOut` into a normal exit. Re-querying is a cheap // SDK call. // SAFETY: `started` owns live process and SDK handles. - unsafe { self.started.exit_code() } - .map_err(std::io::Error::other) - .map(Some) + match unsafe { self.started.confirmed_exit_code() } { + Ok(code) => Ok(Some(code)), + Err(error) => Err(self.cache_wait_failure(error)), + } } /// Always `0` — the sandboxed process runs inside the WSL VM and has no @@ -605,6 +623,10 @@ impl SandboxProcess for WslcSandboxProcess { } fn wait(&mut self) -> std::io::Result { + if let Some(error) = self.replay_wait_failure() { + return Err(error); + } + match self.settled() { // Sticky: a confirmed timeout must keep being reported as one, // never as a later-observed exit code. @@ -646,13 +668,10 @@ impl SandboxProcess for WslcSandboxProcess { // `WslcGetProcessExitEvent`), and closing the SDK-side writers then // would EOF a caller-taken reader and discard every later callback for // a process that is still producing output. - let (exit_code, outcome) = waited.map_err(|response| { - std::io::Error::other(if response.error_message.is_empty() { - "waiting for the WSL container process failed".to_string() - } else { - response.error_message - }) - })?; + let (exit_code, outcome) = match waited { + Ok(result) => result, + Err(error) => return Err(self.cache_wait_failure(error)), + }; // The process is gone (or was killed): close the SDK-side write ends so // every reader — ours and the caller's — sees EOF once it has consumed 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 c821929c13..001c8c950d 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 @@ -33,7 +33,7 @@ use crate::mxc_common::string_util::{to_wide, CoTaskMemPWSTR}; use crate::mxc_common::validator::validate_network_policy_support; use crate::wslc_common::container_steps::{self, sdk_error, SESSION_BOOT_TIMEOUT_MS}; -use crate::wslc_common::error::WslcError; +use crate::wslc_common::error::{sdk_api_failure, WslcError, WslcStepError}; use crate::wslc_common::image; use crate::wslc_common::policy; use crate::wslc_common::policy_mapping; @@ -42,6 +42,12 @@ use crate::wslc_common::sdk_init; use crate::wslc_common::stream_buffer::{stream_pair, StreamReader, StreamWriter}; use crate::wslc_common::wslc_bindings::*; +fn sdk_wait_error(operation: &str, hr: HRESULT, sdk_msg: &str) -> WslcStepError { + WslcError::sdk(format!("{operation} failed"), hr, sdk_msg) + .into_step_error() + .with_api_failure(sdk_api_failure(operation, hr)) +} + /// Where the bytes the WSLC SDK hands us in its stdout/stderr callbacks go. /// /// The WSLC SDK never exposes the container's pipe ends — it pushes output to @@ -690,7 +696,7 @@ impl WSLContainerRunner { exit_event: HANDLE, abort_event: HANDLE, logger: &mut Logger, - ) -> Result<(i32, WaitOutcome), ScriptResponse> { + ) -> Result<(i32, WaitOutcome), WslcStepError> { let sdk = started.sdk; let process_guard = &started.process_guard; let container_guard = &started.container_guard; @@ -735,7 +741,7 @@ impl WSLContainerRunner { 0, ); } - Err(message) => return Err(WslcError::Runtime(message).into_response()), + Err(message) => return Err(WslcError::Runtime(message).into_step_error()), } } let exit_seen = std::time::Instant::now(); @@ -799,9 +805,13 @@ impl WSLContainerRunner { io_ctx.close_streams(); let mut exit_code: i32 = -1; - let hr = sdk.WslcGetProcessExitCode(process_guard.as_raw(), &mut exit_code); + let natural_exit_confirmed = exit_signalled || confirmed; + let hr = started.test_exit_code_result( + natural_exit_confirmed, + sdk.WslcGetProcessExitCode(process_guard.as_raw(), &mut exit_code), + ); if hr != S_OK && stopped.is_none() { - return Err(sdk_error("WslcGetProcessExitCode failed", hr, "")); + return Err(sdk_wait_error("WslcGetProcessExitCode", hr, "")); } let outcome = match (stopped, confirmed) { // Reported only on positive evidence the process ended: the exit @@ -817,7 +827,7 @@ impl WSLContainerRunner { SDK's exit callback did not fire, so the container may still be running" .to_string(), ) - .into_response()); + .into_step_error()); } (Some(StopReason::ForwardingFailure), _) => { let confirmed = confirmed || started.exit_signalled(); @@ -1332,6 +1342,8 @@ impl WSLContainerRunner { settled: AtomicBool::new(false), destroy_on_exit: request.lifecycle.destroy_on_exit, timeout_ms: wait_timeout_ms(request), + #[cfg(all(feature = "test-support", debug_assertions))] + test_script_code: request.script_code.clone().into_boxed_str(), }) } @@ -1502,7 +1514,7 @@ impl WSLContainerRunner { // context and unloading the DLL under live callbacks, so settle // the container first. started.quiesce(logger); - e + e.into_response() } }; @@ -1883,6 +1895,8 @@ pub(crate) struct StartedContainer { pub(crate) destroy_on_exit: bool, /// Wait deadline in milliseconds; `u32::MAX` waits forever. pub(crate) timeout_ms: u32, + #[cfg(all(feature = "test-support", debug_assertions))] + test_script_code: Box, } impl Drop for StartedContainer { @@ -1911,7 +1925,7 @@ impl StartedContainer { pub(crate) fn wait_for_exit( &self, logger: &mut Logger, - ) -> Result<(i32, WaitOutcome), ScriptResponse> { + ) -> Result<(i32, WaitOutcome), WslcStepError> { self.wait_for_exit_or_abort(ptr::null_mut(), logger) } @@ -1919,17 +1933,34 @@ impl StartedContainer { &self, abort_event: HANDLE, logger: &mut Logger, - ) -> Result<(i32, WaitOutcome), ScriptResponse> { + ) -> Result<(i32, WaitOutcome), WslcStepError> { // The handle is `Send`, so this may run on a thread that never entered // the apartment `init_and_load_sdk` established; join it for the call. - let _com = ComApartment::enter().map_err(|e| WslcError::Host(e).into_response())?; + let _com = ComApartment::enter().map_err(|e| WslcError::Host(e).into_step_error())?; let exit_event = self .exit_event() - .map_err(|e| WslcError::Runtime(e).into_response())?; + .map_err(|e| WslcError::Runtime(e).into_step_error())?; // SAFETY: `self` owns live process / container handles and a live SDK. unsafe { WSLContainerRunner::wait_for_process(self, exit_event, abort_event, logger) } } + fn test_exit_code_result(&self, natural_exit_confirmed: bool, result: HRESULT) -> HRESULT { + #[cfg(all(feature = "test-support", debug_assertions))] + { + container_steps::test_exit_code_result( + &self.test_script_code, + natural_exit_confirmed, + result, + ) + } + + #[cfg(not(all(feature = "test-support", debug_assertions)))] + { + let _ = natural_exit_confirmed; + result + } + } + pub(crate) fn add_stdin_closer(&self, closer: Box) { lock(&self.stdin_closers).push(closer); } @@ -2232,17 +2263,16 @@ impl StartedContainer { /// /// # Safety /// Requires a live process handle and SDK, which `self` owns. - pub(crate) unsafe fn exit_code(&self) -> Result { - let _com = ComApartment::enter()?; + pub(crate) unsafe fn confirmed_exit_code(&self) -> Result { + let _com = ComApartment::enter().map_err(|e| WslcError::Host(e).into_step_error())?; let mut exit_code: i32 = -1; - let hr = self - .sdk - .WslcGetProcessExitCode(self.process_guard.as_raw(), &mut exit_code); + let hr = self.test_exit_code_result( + true, + self.sdk + .WslcGetProcessExitCode(self.process_guard.as_raw(), &mut exit_code), + ); if hr != S_OK { - return Err(format!( - "WslcGetProcessExitCode failed (HRESULT 0x{:08X})", - hr as u32 - )); + return Err(sdk_wait_error("WslcGetProcessExitCode", hr, "")); } Ok(exit_code) } diff --git a/src/mxc-sdk/src/core/mxc_common/exec_stream.rs b/src/mxc-sdk/src/core/mxc_common/exec_stream.rs index dd89345860..96873234d4 100644 --- a/src/mxc-sdk/src/core/mxc_common/exec_stream.rs +++ b/src/mxc-sdk/src/core/mxc_common/exec_stream.rs @@ -318,10 +318,7 @@ impl ExecSandboxProcess { }) } - /// Join the waiter thread, caching and returning its outcome. - /// - /// The outcome is cached rather than the exit code, because a timeout has - /// no code to cache and repeat calls must still be idempotent. + /// Join the waiter thread, caching and returning its terminal result. fn join_waiter(&mut self) -> std::io::Result { if let Some(result) = &self.terminal { return terminal_to_io(result); @@ -677,7 +674,7 @@ fn outcome_to_io(outcome: ExecOutcome) -> std::io::Result { fn terminal_to_io(result: &Result) -> std::io::Result { match result { Ok(outcome) => outcome_to_io(*outcome), - Err(error) => Err(std::io::Error::other(error.message.clone())), + Err(error) => Err(super::mxc_error::into_io_error(error.clone())), } } @@ -1619,6 +1616,101 @@ mod tests { drop(stdout_r); } + #[test] + fn waiter_errors_are_cached_with_all_structured_fields() { + let (stdout_r, stdout_w) = std::io::pipe().expect("pipe"); + drop(stdout_w); + let expected = MxcError::backend_unavailable("could not query the process") + .with_details(serde_json::json!({"phase": "wait"})) + .with_api_failure( + crate::mxc_common::mxc_error::ApiFailure::new("WslcGetProcessExitCode") + .with_native_code("0x80004005"), + ) + .with_remediation("Retry the process wait."); + let waiter_error = expected.clone(); + let handle = ExecHandle { + stdin_closer: None, + stdout: reader_handle(&stdout_r), + stderr: null_pipe_handle(), + stdin: null_pipe_handle(), + waiter: Box::new(move || Err(waiter_error)), + terminator: Box::new(|| Ok(())), + }; + let mut process = ExecSandboxProcess::from_exec_handle(handle).unwrap(); + + for error in [ + process.wait().expect_err("the waiter fails"), + process.wait().expect_err("the cached waiter result fails"), + process + .try_wait() + .expect_err("polling replays the cached waiter result"), + ] { + assert_eq!(error.kind(), std::io::ErrorKind::Other); + assert_eq!(error.to_string(), expected.message); + assert_eq!( + crate::mxc_common::mxc_error::from_io_error(&error), + Some(expected.clone()) + ); + } + + drop(stdout_r); + } + + #[test] + fn a_cached_waiter_error_does_not_retire_the_terminator() { + let (stdout_r, stdout_w) = std::io::pipe().expect("pipe"); + drop(stdout_w); + let terminated = Arc::new(std::sync::atomic::AtomicBool::new(false)); + let terminated_by_kill = Arc::clone(&terminated); + let handle = ExecHandle { + stdin_closer: None, + stdout: reader_handle(&stdout_r), + stderr: null_pipe_handle(), + stdin: null_pipe_handle(), + waiter: Box::new(|| Err(MxcError::backend_error("wait failed"))), + terminator: Box::new(move || { + terminated_by_kill.store(true, std::sync::atomic::Ordering::SeqCst); + Ok(()) + }), + }; + let mut process = ExecSandboxProcess::from_exec_handle(handle).unwrap(); + + process.wait().expect_err("the waiter fails"); + process + .kill() + .expect("an unknown process outcome must remain killable"); + + assert!(terminated.load(std::sync::atomic::Ordering::SeqCst)); + drop(stdout_r); + } + + #[test] + fn a_waiter_panic_is_cached_as_a_structured_backend_error() { + let (stdout_r, stdout_w) = std::io::pipe().expect("pipe"); + drop(stdout_w); + let handle = ExecHandle { + stdin_closer: None, + stdout: reader_handle(&stdout_r), + stderr: null_pipe_handle(), + stdin: null_pipe_handle(), + waiter: Box::new(|| panic!("forced waiter panic")), + terminator: Box::new(|| Ok(())), + }; + let mut process = ExecSandboxProcess::from_exec_handle(handle).unwrap(); + + for error in [ + process.wait().expect_err("the waiter panics"), + process.wait().expect_err("the cached panic fails"), + ] { + let structured = crate::mxc_common::mxc_error::from_io_error(&error) + .expect("structured waiter error"); + assert_eq!(structured.code, MxcErrorCode::BackendError); + assert_eq!(structured.message, "exec waiter thread panicked"); + } + + drop(stdout_r); + } + /// `wait` must **tear its discard down**, not walk away from it. /// /// This is the regression the cancellable drain exists for, and the other @@ -1720,7 +1812,7 @@ mod tests { /// A prior `try_wait()` must not let `wait()` skip the drain. /// - /// `self.exit` records that the child finished; it says nothing about + /// A cached success records that the child finished; it says nothing about /// whether anything was drained. Returning early on it would silently /// break the contract for exactly the caller who polls before waiting. /// diff --git a/src/mxc-sdk/src/core/mxc_common/mxc_error.rs b/src/mxc-sdk/src/core/mxc_common/mxc_error.rs index 928ee11c28..2d2de11717 100644 --- a/src/mxc-sdk/src/core/mxc_common/mxc_error.rs +++ b/src/mxc-sdk/src/core/mxc_common/mxc_error.rs @@ -176,6 +176,28 @@ impl fmt::Display for MxcError { impl std::error::Error for MxcError {} +#[derive(Debug)] +struct MxcIoError(MxcError); + +impl fmt::Display for MxcIoError { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str(&self.0.message) + } +} + +impl std::error::Error for MxcIoError {} + +pub(crate) fn into_io_error(error: MxcError) -> std::io::Error { + std::io::Error::other(MxcIoError(error)) +} + +pub(crate) fn from_io_error(error: &std::io::Error) -> Option { + error + .get_ref() + .and_then(|inner| inner.downcast_ref::()) + .map(|inner| inner.0.clone()) +} + impl MxcError { pub fn new(code: MxcErrorCode, message: impl Into) -> Self { Self { diff --git a/src/mxc-sdk/src/core/mxc_engine/error.rs b/src/mxc-sdk/src/core/mxc_engine/error.rs index c9eaf5009a..c5a382647e 100644 --- a/src/mxc-sdk/src/core/mxc_engine/error.rs +++ b/src/mxc-sdk/src/core/mxc_engine/error.rs @@ -215,6 +215,7 @@ impl Error { mod tests { use super::*; use crate::mxc_common::mxc_error::{ApiFailure as InnerFailure, MxcError}; + /// The conversion carries the API detail across, rather than keeping only /// the code and message. /// @@ -225,6 +226,7 @@ mod tests { #[test] fn the_conversion_preserves_the_api_detail() { let inner = MxcError::backend_error("The provision was not found.") + .with_details(serde_json::json!({"backend": "test"})) .with_api_failure( InnerFailure::new("IsoSessionOps.StopSessionAsync").with_native_code("0x80070490"), ) @@ -234,6 +236,10 @@ mod tests { assert_eq!(error.code, ErrorCode::BackendError); assert_eq!(error.message, "The provision was not found."); + assert_eq!( + error.details, + Some(Box::new(serde_json::json!({"backend": "test"}))) + ); assert_eq!( error.operation.as_deref(), Some("IsoSessionOps.StopSessionAsync") @@ -268,12 +274,24 @@ mod tests { assert_eq!(converted.operation, None); assert_eq!(converted.native_code, None); assert_eq!(converted.remediation, None); + assert_eq!(converted.details, None); let constructed = Error::new(ErrorCode::BackendError, "x"); assert_eq!(constructed.operation, None); assert_eq!(constructed.native_code, None); assert_eq!(constructed.remediation, None); + assert_eq!(constructed.details, None); + } + + #[test] + fn the_conversion_round_trip_preserves_details() { + let inner = MxcError::backend_error("boom") + .with_details(serde_json::json!({"phase": "wait"})) + .with_api_failure(InnerFailure::new("Iface.Wait").with_native_code("0x80004005")) + .with_remediation("Retry."); + + assert_eq!(Error::from(inner.clone()).into_mxc_error(), inner); } /// A hint with no API call behind it survives the conversion, and `Display` diff --git a/src/mxc-sdk/src/lib.rs b/src/mxc-sdk/src/lib.rs index 3841b6b989..5ef4e946ad 100644 --- a/src/mxc-sdk/src/lib.rs +++ b/src/mxc-sdk/src/lib.rs @@ -552,10 +552,5 @@ fn captured_execution_result( } fn wait_with_output(sandbox: MxcProcess) -> Result { - sandbox.wait_with_output().map_err(|e| { - Error::new( - ErrorCode::BackendError, - format!("waiting for the sandbox to complete failed: {e}"), - ) - }) + sandbox.wait_with_output_detailed() } diff --git a/src/mxc-sdk/src/sandbox.rs b/src/mxc-sdk/src/sandbox.rs index 3584d7d22f..ed81c4db99 100644 --- a/src/mxc-sdk/src/sandbox.rs +++ b/src/mxc-sdk/src/sandbox.rs @@ -378,6 +378,21 @@ impl MxcProcess { self.inner.try_wait() } + /// Non-blocking terminal check that preserves structured backend failures. + pub fn try_wait_detailed(&mut self) -> Result, Error> { + match self.inner.try_wait() { + Ok(Some(code)) => Ok(Some(WaitResult::Exited(code))), + Ok(None) => Ok(None), + Err(error) if error.kind() == std::io::ErrorKind::TimedOut => { + Ok(Some(WaitResult::TimedOut)) + } + Err(error) => Err(detailed_wait_error( + error, + "querying the sandbox process status failed", + )), + } + } + /// The child's process id. pub fn id(&self) -> u32 { self.inner.id() @@ -409,6 +424,18 @@ impl MxcProcess { } } + /// Wait for the child to finish while preserving structured backend failures. + pub fn wait_detailed(&mut self) -> Result { + match self.inner.wait() { + Ok(code) => Ok(WaitResult::Exited(code)), + Err(error) if error.kind() == std::io::ErrorKind::TimedOut => Ok(WaitResult::TimedOut), + Err(error) => Err(detailed_wait_error( + error, + "waiting for the sandbox process failed", + )), + } + } + /// Wait for the child to exit, capturing its stdout and stderr. /// /// The safe alternative to [`take_stdout`](Self::take_stdout) + @@ -420,7 +447,28 @@ impl MxcProcess { /// as [`ExecutionResult`] with `outcome: WaitResult::TimedOut` and whatever each /// stream captured before cancellation. Timeout and wait failures cancel /// outstanding reads so descendants retaining pipe ends cannot block capture. - pub fn wait_with_output(mut self) -> std::io::Result { + pub fn wait_with_output(self) -> std::io::Result { + self.wait_with_output_impl(|process| process.wait(), |_, error| error) + } + + /// Capture stdout and stderr while preserving structured backend wait failures. + pub fn wait_with_output_detailed(self) -> Result { + self.wait_with_output_impl( + |process| process.wait_detailed(), + |stream, error| { + Error::new( + crate::ErrorCode::BackendError, + format!("reading sandbox {stream} failed: {error}"), + ) + }, + ) + } + + fn wait_with_output_impl( + mut self, + wait: impl FnOnce(&mut Self) -> Result, + map_output_error: impl Fn(&str, std::io::Error) -> E, + ) -> Result { fn capture( stream: Option>, ) -> std::thread::JoinHandle>> { @@ -439,7 +487,7 @@ impl MxcProcess { let stderr_closer = self.stderr_closer(); let stdout = capture(self.inner.take_stdout()); let stderr = capture(self.inner.take_stderr()); - let outcome = self.wait(); + let outcome = wait(&mut self); // Sampled after the wait: a backend whose teardown runs there reports // its failures here. let warnings = self.inner.warnings(); @@ -466,13 +514,23 @@ impl MxcProcess { Ok(ExecutionResult { outcome: outcome?, warnings, - stdout: stdout?, - stderr: stderr?, + stdout: stdout.map_err(|error| map_output_error("stdout", error))?, + stderr: stderr.map_err(|error| map_output_error("stderr", error))?, output_metadata, }) } } +fn detailed_wait_error(error: std::io::Error, context: &str) -> Error { + match crate::mxc_common::mxc_error::from_io_error(&error) { + Some(error) => error.into(), + None => Error::new( + crate::ErrorCode::BackendError, + format!("{context}: {error}"), + ), + } +} + /// A live container process attached to a caller-controlled pseudo-terminal. pub struct MxcPtyProcess { process: Arc>>, @@ -749,6 +807,7 @@ impl StreamCloser { #[cfg(test)] mod tests { use super::*; + use crate::mxc_common::mxc_error::{ApiFailure, MxcError}; struct FakeProcess { warnings: Vec, @@ -804,6 +863,11 @@ mod tests { struct NativeOnlyFake; + struct StructuredWaitErrorFake { + error: MxcError, + stdout: Option>, + } + impl SandboxProcess for NativeOnlyFake { fn warnings(&self) -> Vec { Vec::new() @@ -903,6 +967,49 @@ mod tests { } } + impl SandboxProcess for StructuredWaitErrorFake { + fn take_stdin(&mut self) -> Option> { + None + } + + fn take_stdout(&mut self) -> Option> { + self.stdout.take() + } + + fn take_stderr(&mut self) -> Option> { + None + } + + fn try_wait(&mut self) -> std::io::Result> { + Err(crate::mxc_common::mxc_error::into_io_error( + self.error.clone(), + )) + } + + fn id(&self) -> u32 { + 1 + } + + fn kill(&mut self) -> std::io::Result<()> { + Ok(()) + } + + fn wait(&mut self) -> std::io::Result { + Err(crate::mxc_common::mxc_error::into_io_error( + self.error.clone(), + )) + } + } + + fn structured_wait_error() -> MxcError { + MxcError::backend_unavailable("the process status is unavailable") + .with_details(serde_json::json!({"phase": "wait"})) + .with_api_failure( + ApiFailure::new("WslcGetProcessExitCode").with_native_code("0x80004005"), + ) + .with_remediation("Retry the process wait.") + } + #[test] fn sandbox_and_output_expose_security_warnings() { let warning = "permissive mode weakens containment".to_string(); @@ -1000,6 +1107,71 @@ mod tests { assert_eq!(error.to_string(), "capture read failed"); } + #[test] + fn detailed_wait_methods_preserve_structured_backend_errors() { + let expected = Error::from(structured_wait_error()); + let mut process = MxcProcess::new(Box::new(StructuredWaitErrorFake { + error: structured_wait_error(), + stdout: None, + })); + + assert_eq!( + process + .try_wait_detailed() + .expect_err("polling must preserve the failure"), + expected + ); + assert_eq!( + process + .wait_detailed() + .expect_err("waiting must preserve the failure"), + expected + ); + + let legacy = process.wait().expect_err("legacy wait still fails"); + assert_eq!(legacy.kind(), std::io::ErrorKind::Other); + assert_eq!(legacy.to_string(), expected.message); + } + + #[test] + fn detailed_capture_prefers_the_wait_failure_over_a_read_failure() { + let expected = Error::from(structured_wait_error()); + let process = MxcProcess::new(Box::new(StructuredWaitErrorFake { + error: structured_wait_error(), + stdout: Some(Box::new(FailedReader)), + })); + + assert_eq!( + process + .wait_with_output_detailed() + .expect_err("the structured wait failure wins"), + expected + ); + } + + #[test] + fn detailed_capture_contextualizes_output_read_failures() { + let process = MxcProcess::new(Box::new(FakeProcess { + warnings: Vec::new(), + wait_warning: None, + output_metadata: None, + stdout: Some(Box::new(FailedReader)), + release_output: None, + output_closer: None, + timed_out: false, + })); + + let error = process + .wait_with_output_detailed() + .expect_err("read must fail"); + + assert_eq!(error.code, crate::ErrorCode::BackendError); + assert_eq!( + error.message, + "reading sandbox stdout failed: capture read failed" + ); + } + #[test] fn native_stdio_is_rejected_after_an_individual_stream_is_taken() { let mut sandbox = MxcProcess::new(Box::new(FakeProcess { From 6d84e49a2d3ae505e9e736bc3b13ed52a5dd5582 Mon Sep 17 00:00:00 2001 From: Soham Das Date: Mon, 9 Nov 2026 11:04:40 -0800 Subject: [PATCH 3/4] [WSLC] Expect the exec failed prefix in lifecycle wait-failure tests Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Microsoft.Mxc.Sdk.Tests/V1/MxcLifecycleWslcE2ETests.cs | 2 +- sdk/node/tests/integration/wslc-state-aware.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/dotnet/Microsoft.Mxc.Sdk.Tests/V1/MxcLifecycleWslcE2ETests.cs b/sdk/dotnet/Microsoft.Mxc.Sdk.Tests/V1/MxcLifecycleWslcE2ETests.cs index 5d1627bcb0..b2a59fb43d 100644 --- a/sdk/dotnet/Microsoft.Mxc.Sdk.Tests/V1/MxcLifecycleWslcE2ETests.cs +++ b/sdk/dotnet/Microsoft.Mxc.Sdk.Tests/V1/MxcLifecycleWslcE2ETests.cs @@ -350,7 +350,7 @@ private static void AssertForcedWaitFailure(MxcException error) { Assert.Equal(ErrorCode.BackendError, error.Code); Assert.Equal( - "WslcGetProcessExitCode failed: HRESULT 0x80004005", + "exec failed: WslcGetProcessExitCode failed: HRESULT 0x80004005", error.Message); Assert.Equal("WslcGetProcessExitCode", error.Operation); Assert.Equal("0x80004005", error.NativeCode); diff --git a/sdk/node/tests/integration/wslc-state-aware.test.ts b/sdk/node/tests/integration/wslc-state-aware.test.ts index b81a9e9a4e..1b416523c3 100644 --- a/sdk/node/tests/integration/wslc-state-aware.test.ts +++ b/sdk/node/tests/integration/wslc-state-aware.test.ts @@ -81,7 +81,7 @@ function assertForcedWaitFailure(error: unknown): boolean { assert.equal(error.code, 'backend_error'); assert.equal( error.message, - 'WslcGetProcessExitCode failed: HRESULT 0x80004005', + 'exec failed: WslcGetProcessExitCode failed: HRESULT 0x80004005', ); assert.equal(error.operation, 'WslcGetProcessExitCode'); assert.equal(error.nativeCode, '0x80004005'); From c567eb132d2f6e83d13972840dc492df1b7b2f49 Mon Sep 17 00:00:00 2001 From: Soham Das Date: Thu, 12 Nov 2026 13:07:28 -0800 Subject: [PATCH 4/4] Addressed PR comments Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docs/api-reference/dotnet/v1/types.md | 2 +- docs/api-reference/node/v1/types.md | 5 +- .../architecture/container-lifecycle.md | 3 +- scripts/ci/run_backend_validation_tests.ps1 | 13 +++++ .../Microsoft.Mxc.Sdk.Tests.csproj | 20 +++---- .../V1/MxcContainerWslcE2ETests.cs | 48 ++++++++++++---- sdk/dotnet/README.md | 1 + sdk/node/README.md | 2 +- sdk/node/tests/integration/wslc-e2e.test.ts | 25 +++++++-- .../backends/wslc/common/container_steps.rs | 56 +++++++++++++------ .../wslc/common/wsl_container_runner.rs | 15 ++--- src/mxc-sdk/src/core/mxc_common/mxc_error.rs | 20 ++++++- tests/configs/wslc_test_hook_marker.json | 15 +++++ tests/scripts/run_wslc_all_tests.ps1 | 1 + 14 files changed, 167 insertions(+), 59 deletions(-) create mode 100644 tests/configs/wslc_test_hook_marker.json diff --git a/docs/api-reference/dotnet/v1/types.md b/docs/api-reference/dotnet/v1/types.md index 960aefe246..17f215e0f7 100644 --- a/docs/api-reference/dotnet/v1/types.md +++ b/docs/api-reference/dotnet/v1/types.md @@ -729,7 +729,7 @@ public class MxcProcess : IMxcProcess `MxcException` when MXC cannot obtain a terminal process result after admission. The exception preserves `Code`, `Message`, `Operation`, `NativeCode`, and `Remediation`; the failure is not converted into an exit -code. +code. `MxcPtyProcess` does not preserve these fields. ## `Microsoft.Mxc.Sdk.V1.MxcPtyProcess` diff --git a/docs/api-reference/node/v1/types.md b/docs/api-reference/node/v1/types.md index 1f7876f2dd..8bb63bb28d 100644 --- a/docs/api-reference/node/v1/types.md +++ b/docs/api-reference/node/v1/types.md @@ -471,8 +471,9 @@ export class MxcProcess { `wait()` rejects with `MxcError` when MXC cannot obtain a terminal process result after admission. The rejection preserves `code`, `message`, -`operation`, `nativeCode`, `remediation`, and `details`; it is not converted -into an exit result. +`operation`, `nativeCode`, and `remediation`; it is not converted into an exit +result. Its `details` carry only the native status code (`ffiStatus`), not +backend details. `MxcPtyProcess` does not preserve these fields. ## `@microsoft/mxc-sdk/v1::MxcPtyProcess` diff --git a/docs/development/architecture/container-lifecycle.md b/docs/development/architecture/container-lifecycle.md index ed05103c78..2fbc68e6f4 100644 --- a/docs/development/architecture/container-lifecycle.md +++ b/docs/development/architecture/container-lifecycle.md @@ -960,7 +960,8 @@ sentinel exit codes from the exec process, matching the existing one-shot conven A backend can also fail while observing the process after admission. Captured execution and the .NET and Node live-process wait APIs preserve the resulting error code, message, operation, native code, and remediation instead of -reporting a successful exit. Rust `MxcProcess::try_wait`, `wait`, and +reporting a successful exit. PTY processes do not preserve these fields. Rust +`MxcProcess::try_wait`, `wait`, and `wait_with_output` retain their existing `std::io::Error` contracts and message text. Their `*_detailed` counterparts return `mxc_sdk::v1::Error` with the structured fields intact. diff --git a/scripts/ci/run_backend_validation_tests.ps1 b/scripts/ci/run_backend_validation_tests.ps1 index dad901feb0..d197a0f154 100644 --- a/scripts/ci/run_backend_validation_tests.ps1 +++ b/scripts/ci/run_backend_validation_tests.ps1 @@ -319,6 +319,17 @@ function Invoke-NamedSuites { } } +# Daemon discovery reuses any running daemon, including one another suite started. +function Stop-WslcDaemons { + foreach ($daemon in @(Get-Process -Name 'wxc-wslc-daemon' -ErrorAction SilentlyContinue)) { + Write-Host "Stopping wxc-wslc-daemon $($daemon.Id) from an earlier suite." + Stop-Process -Id $daemon.Id -Force -ErrorAction SilentlyContinue + if (-not $daemon.WaitForExit(30000)) { + throw "wxc-wslc-daemon $($daemon.Id) did not exit within 30 s." + } + } +} + function Invoke-WslcSuites { $bundle = Join-Path $binaryDirectoryPath 'windows-sdk-bundle' foreach ($file in 'dotnet\Microsoft.Mxc.Sdk.Tests.exe', 'node\node.exe', 'sdk-integration\dist\wslc-state-aware.test.js', 'sdk-integration\dist\wslc-e2e.test.js') { @@ -349,6 +360,7 @@ function Invoke-WslcSuites { if ($LASTEXITCODE -ne 0) { "exited $LASTEXITCODE" } } 'Node' = { + Stop-WslcDaemons Invoke-NodeIntegrationSuite -Bundle $bundle -Name 'wslc' -TestFile 'wslc-state-aware.test.js' ` -RequiredTests @('V1.Dev preserves WSLC lifecycle responses, captured exec, pipes, and caller ownership', 'captured exec preserves structured post-admission wait failures', @@ -361,6 +373,7 @@ function Invoke-WslcSuites { 'spawned process wait preserves structured post-admission wait failures') } 'C#' = { + Stop-WslcDaemons Invoke-XunitClasses -Bundle $bundle -Name 'wslc' -Classes 'MxcLifecycleWslcE2ETests', 'MxcContainerWslcE2ETests' ` -RequiredMethods @('DevJson_WslcLifecyclePreservesResponsesExecutionAndCallerOwnership', 'RunInContainer_PreservesStructuredPostAdmissionWaitFailure', diff --git a/sdk/dotnet/Microsoft.Mxc.Sdk.Tests/Microsoft.Mxc.Sdk.Tests.csproj b/sdk/dotnet/Microsoft.Mxc.Sdk.Tests/Microsoft.Mxc.Sdk.Tests.csproj index 433bb0d901..46610bba00 100644 --- a/sdk/dotnet/Microsoft.Mxc.Sdk.Tests/Microsoft.Mxc.Sdk.Tests.csproj +++ b/sdk/dotnet/Microsoft.Mxc.Sdk.Tests/Microsoft.Mxc.Sdk.Tests.csproj @@ -38,7 +38,7 @@ + Condition="'$(TargetFramework)' != '' and '$(DesignTimeBuild)' != 'true' and '$(MxcUsePrebuiltTestNativeLibrary)' != 'true'"> $(MxcTestCargoTargetDir)/$(MxcTestRustHostTriple)/$(MxcTestCargoProfileDir)/$(MxcTestNativeFileName) $(MxcTestWslcCargoTargetDir)/$(MxcTestRustHostTriple)/$(MxcTestCargoProfileDir)/wxc-wslc-daemon.exe - - - + + + DestinationFolder="$(TargetDir)" /> @@ -90,6 +85,11 @@ Text="The prebuilt test-support native library does not exist at $(MxcTestPrebuiltNativeLibrary)." /> + + diff --git a/sdk/dotnet/Microsoft.Mxc.Sdk.Tests/V1/MxcContainerWslcE2ETests.cs b/sdk/dotnet/Microsoft.Mxc.Sdk.Tests/V1/MxcContainerWslcE2ETests.cs index cda42cc092..c196a45470 100644 --- a/sdk/dotnet/Microsoft.Mxc.Sdk.Tests/V1/MxcContainerWslcE2ETests.cs +++ b/sdk/dotnet/Microsoft.Mxc.Sdk.Tests/V1/MxcContainerWslcE2ETests.cs @@ -29,12 +29,19 @@ private static void RequireWslc() #endif } - private static ContainerRequest Request(string command) => new(command) + private static ContainerRequest Request(string command, string? containerName = null) => new(command) { TimeoutMs = 60_000, + ContainerName = containerName, Containment = new Containment.Wslc(), }; + private static string UniqueContainerName() => $"mxc-wait-{Guid.NewGuid():N}"; + + // WSLc refuses a session name that is still in use. + private static void AssertContainerNameReleased(string containerName, string marker) => + Assert.Equal(marker, MxcContainer.Run(Request($"echo {marker}", containerName)).Stdout.Trim()); + [Fact] public async Task SpawnWithStdin_ForwardsEveryByteAndClosingStdinEndsInput() { @@ -338,30 +345,49 @@ public async Task Run_GivesACommandThatReadsStdinEndOfFile() public void Run_PreservesStructuredPostAdmissionWaitFailure() { RequireWslc(); + var containerName = UniqueContainerName(); var error = Assert.Throws(() => - MxcContainer.Run(Request(ForcedWaitFailureCommand))); + MxcContainer.Run(Request(ForcedWaitFailureCommand, containerName))); AssertForcedWaitFailure(error); - Assert.Equal( - "RUN-RECOVERED", - MxcContainer.Run(Request("echo RUN-RECOVERED")).Stdout.Trim()); + AssertContainerNameReleased(containerName, "RUN-RECOVERED"); } [Fact] - public void Spawn_WaitPreservesStructuredPostAdmissionWaitFailure() + public async Task Spawn_WaitPreservesStructuredPostAdmissionWaitFailure() { RequireWslc(); + var containerName = UniqueContainerName(); - using (var process = MxcContainer.Spawn(Request(ForcedWaitFailureCommand))) + using (var process = MxcContainer.Spawn(Request(ForcedWaitFailureCommand, containerName))) { + AssertForcedWaitFailure(PollForWaitFailure(process)); AssertForcedWaitFailure(Assert.Throws(() => process.Wait())); - AssertForcedWaitFailure(Assert.Throws(() => process.Wait())); + AssertForcedWaitFailure(await Assert.ThrowsAsync( + () => process.WaitAsync(TestContext.Current.CancellationToken))); } - Assert.Equal( - "PROCESS-RECOVERED", - MxcContainer.Run(Request("echo PROCESS-RECOVERED")).Stdout.Trim()); + AssertContainerNameReleased(containerName, "PROCESS-RECOVERED"); + } + + private static MxcException PollForWaitFailure(MxcProcess process) + { + var deadline = DateTime.UtcNow.AddSeconds(60); + while (true) + { + try + { + Assert.False(process.TryGetExitCode(out var exitCode), $"the poll reported exit code {exitCode}"); + } + catch (MxcException error) + { + return error; + } + + Assert.True(DateTime.UtcNow < deadline, "the poll did not report the wait failure within 60 s"); + Thread.Sleep(100); + } } private static void AssertForcedWaitFailure(MxcException error) diff --git a/sdk/dotnet/README.md b/sdk/dotnet/README.md index b89af4a9a8..ab5cfad8b1 100644 --- a/sdk/dotnet/README.md +++ b/sdk/dotnet/README.md @@ -256,6 +256,7 @@ terminal result retain those fields from `MxcContainer.Run`, `MxcLifecycle.RunInContainer`, `MxcProcess.TryGetExitCode`, `Wait`, and `WaitAsync`; they are not converted into an exit code. Repeated waits report the same failure, while disposal still performs process cleanup. +`MxcPtyProcess` does not preserve these fields. Provision returns identity, optional typed metadata, and `ProvisionResult.Warnings`. Start, stop, and deprovision return `LifecycleResult.Warnings`. Omitted native warnings become an empty array; malformed warnings fail explicitly. When diff --git a/sdk/node/README.md b/sdk/node/README.md index 858d9443b5..dc60ddbc99 100644 --- a/sdk/node/README.md +++ b/sdk/node/README.md @@ -257,7 +257,7 @@ Failures that occur after a process starts but before MXC can confirm its terminal result retain those fields from `run`, `runInContainer`, and `MxcProcess.wait`; they are not converted into an exit result. Every caller awaiting the process receives the same rejection, and `dispose()` still -performs process cleanup. +performs process cleanup. `MxcPtyProcess` does not preserve these fields. Captured `stdout` and `stderr` are workload output; warnings and structured denial-capture metadata are not appended to those streams. Validation warnings are returned in `ValidationResult.warnings`. diff --git a/sdk/node/tests/integration/wslc-e2e.test.ts b/sdk/node/tests/integration/wslc-e2e.test.ts index 53e3d3d492..e74a9a4ea5 100644 --- a/sdk/node/tests/integration/wslc-e2e.test.ts +++ b/sdk/node/tests/integration/wslc-e2e.test.ts @@ -16,6 +16,7 @@ import { describe, it } from 'node:test'; import assert from 'node:assert'; +import { randomUUID } from 'node:crypto'; import fs from 'node:fs'; import net from 'node:net'; import path from 'node:path'; @@ -28,6 +29,7 @@ import { sdk } from './test-helpers.js'; const isWslcAvailable = os.platform() === 'win32' && process.env.MXC_ENABLE_WSLC_TESTS === '1'; const forcedWaitFailureCommand = 'true # mxc-test-force-wslc-get-exit-code-failure'; +const uniqueContainerName = () => `mxc-wait-${randomUUID().replace(/-/g, '')}`; function assertForcedWaitFailure(error: unknown): boolean { assert.ok(error instanceof MxcError); @@ -231,18 +233,30 @@ srv.handle_request() containment: { type: 'wslc' as const, config: { image: 'alpine:latest' } }, }); + const namedRequest = (command: string, containerName: string) => ({ + ...stdinRequest(command), + containerName, + }); + + // WSLc refuses a session name that is still in use. + async function assertContainerNameReleased(containerName: string, marker: string): Promise { + const recovered = await sdk.run(namedRequest(`echo ${marker}`, containerName)); + assert.equal(recovered.stdout.trim(), marker); + } + it('captured run preserves structured post-admission wait failures', { timeout: 180_000 }, async () => { + const containerName = uniqueContainerName(); await assert.rejects( - sdk.run(stdinRequest(forcedWaitFailureCommand)), + sdk.run(namedRequest(forcedWaitFailureCommand, containerName)), assertForcedWaitFailure, ); - const recovered = await sdk.run(stdinRequest('echo RUN-RECOVERED')); - assert.equal(recovered.stdout.trim(), 'RUN-RECOVERED'); + await assertContainerNameReleased(containerName, 'RUN-RECOVERED'); }); it('spawned process wait preserves structured post-admission wait failures', { timeout: 180_000 }, async () => { - const process = await sdk.spawn(stdinRequest(forcedWaitFailureCommand)); + const containerName = uniqueContainerName(); + const process = await sdk.spawn(namedRequest(forcedWaitFailureCommand, containerName)); try { await assert.rejects(process.wait(), assertForcedWaitFailure); await assert.rejects(process.wait(), assertForcedWaitFailure); @@ -250,8 +264,7 @@ srv.handle_request() process.dispose(); } - const recovered = await sdk.run(stdinRequest('echo PROCESS-RECOVERED')); - assert.equal(recovered.stdout.trim(), 'PROCESS-RECOVERED'); + await assertContainerNameReleased(containerName, 'PROCESS-RECOVERED'); }); function collectBytes(stream: NodeJS.ReadableStream | null): Promise { 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 d9dff9d566..7491e70407 100644 --- a/src/mxc-sdk/src/backends/wslc/common/container_steps.rs +++ b/src/mxc-sdk/src/backends/wslc/common/container_steps.rs @@ -1305,18 +1305,29 @@ fn natural_exit_confirmed(exit_wait: ExitWait, callback_confirmed: bool) -> bool || (matches!(exit_wait, ExitWait::NoEvent) && callback_confirmed) } -#[cfg(all(feature = "test-support", debug_assertions))] +#[cfg(any(test, all(feature = "test-support", debug_assertions)))] const TEST_EXIT_CODE_FAILURE_SCRIPT: &str = "true # mxc-test-force-wslc-get-exit-code-failure"; -pub(crate) fn test_exit_code_result( +#[cfg(any(test, all(feature = "test-support", debug_assertions)))] +fn forced_exit_code_result( script_code: &str, natural_exit_confirmed: bool, result: HRESULT, ) -> HRESULT { - #[cfg(all(feature = "test-support", debug_assertions))] if result == S_OK && natural_exit_confirmed && script_code == TEST_EXIT_CODE_FAILURE_SCRIPT { - return 0x8000_4005_u32 as HRESULT; + E_FAIL + } else { + result } +} + +pub(crate) fn test_exit_code_result( + script_code: &str, + natural_exit_confirmed: bool, + result: HRESULT, +) -> HRESULT { + #[cfg(all(feature = "test-support", debug_assertions))] + let result = forced_exit_code_result(script_code, natural_exit_confirmed, result); let _ = (script_code, natural_exit_confirmed); result @@ -1760,28 +1771,23 @@ mod tests { use super::*; use crate::mxc_common::models::FailurePhase; - #[cfg(all(feature = "test-support", debug_assertions))] #[test] fn test_exit_code_failure_requires_the_exact_successful_natural_exit() { - let forced = 0x8000_4005_u32 as HRESULT; + let access_denied = 0x8007_0005_u32 as HRESULT; assert_eq!( - test_exit_code_result(TEST_EXIT_CODE_FAILURE_SCRIPT, true, S_OK), - forced + forced_exit_code_result(TEST_EXIT_CODE_FAILURE_SCRIPT, true, S_OK), + E_FAIL ); assert_eq!( - test_exit_code_result( - TEST_EXIT_CODE_FAILURE_SCRIPT, - true, - 0x8007_0005_u32 as HRESULT - ), - 0x8007_0005_u32 as HRESULT + forced_exit_code_result(TEST_EXIT_CODE_FAILURE_SCRIPT, true, access_denied), + access_denied ); assert_eq!( - test_exit_code_result(TEST_EXIT_CODE_FAILURE_SCRIPT, false, S_OK), + forced_exit_code_result(TEST_EXIT_CODE_FAILURE_SCRIPT, false, S_OK), S_OK ); assert_eq!( - test_exit_code_result( + forced_exit_code_result( "true # mxc-test-force-wslc-get-exit-code-failure ", true, S_OK @@ -1790,6 +1796,24 @@ mod tests { ); } + #[cfg(all(feature = "test-support", debug_assertions))] + #[test] + fn the_exit_code_failure_marker_fails_the_call_with_test_support() { + assert_eq!( + test_exit_code_result(TEST_EXIT_CODE_FAILURE_SCRIPT, true, S_OK), + E_FAIL + ); + } + + #[cfg(not(all(feature = "test-support", debug_assertions)))] + #[test] + fn the_exit_code_failure_marker_is_inert_without_test_support() { + assert_eq!( + test_exit_code_result(TEST_EXIT_CODE_FAILURE_SCRIPT, true, S_OK), + S_OK + ); + } + /// A modest machine: 16 GB and 8 logical processors. const TEST_HOST: HostCapacity = HostCapacity { total_memory_mb: 16 * 1024, 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 1ac159810a..f3ada5c47d 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 @@ -1963,9 +1963,7 @@ impl StartedContainer { // The handle is `Send`, so this may run on a thread that never entered // the apartment `init_and_load_sdk` established; join it for the call. let _com = ComApartment::enter().map_err(|e| WslcError::Host(e).into_step_error())?; - let exit_event = self - .exit_event() - .map_err(|e| WslcError::Runtime(e).into_step_error())?; + let exit_event = self.exit_event()?; // SAFETY: `self` owns live process / container handles and a live SDK. unsafe { WSLContainerRunner::wait_for_process(self, exit_event, abort_event, logger) } } @@ -2011,7 +2009,7 @@ impl StartedContainer { use windows::Win32::Foundation::{DuplicateHandle, DUPLICATE_SAME_ACCESS}; use windows::Win32::System::Threading::GetCurrentProcess; - let event = self.exit_event()?; + let event = self.exit_event().map_err(|error| error.error_message)?; if event.is_null() { return Ok(None); } @@ -2038,12 +2036,12 @@ impl StartedContainer { } /// The process's exit event, or a null handle if the SDK has none. - fn exit_event(&self) -> Result { + fn exit_event(&self) -> Result { let mut cached = lock(&self.exit_event); if let Some(event) = *cached { return Ok(event as HANDLE); } - let _com = ComApartment::enter()?; + let _com = ComApartment::enter().map_err(|e| WslcError::Host(e).into_step_error())?; let mut event: HANDLE = ptr::null_mut(); // SAFETY: `self` owns a live process handle and SDK. @@ -2052,10 +2050,7 @@ impl StartedContainer { .WslcGetProcessExitEvent(self.process_guard.as_raw(), &mut event) }; if hr != S_OK { - return Err(format!( - "WslcGetProcessExitEvent failed: HRESULT 0x{:08X}", - hr as u32 - )); + return Err(sdk_wait_error("WslcGetProcessExitEvent", hr, "")); } *cached = Some(event as usize); Ok(event) diff --git a/src/mxc-sdk/src/core/mxc_common/mxc_error.rs b/src/mxc-sdk/src/core/mxc_common/mxc_error.rs index 2d2de11717..ec15078386 100644 --- a/src/mxc-sdk/src/core/mxc_common/mxc_error.rs +++ b/src/mxc-sdk/src/core/mxc_common/mxc_error.rs @@ -176,9 +176,15 @@ impl fmt::Display for MxcError { impl std::error::Error for MxcError {} -#[derive(Debug)] struct MxcIoError(MxcError); +// Backend details can hold sensitive identifiers. +impl fmt::Debug for MxcIoError { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Debug::fmt(&self.0.message, formatter) + } +} + impl fmt::Display for MxcIoError { fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { formatter.write_str(&self.0.message) @@ -349,6 +355,18 @@ mod tests { use super::*; use serde_json::json; + #[test] + fn a_wrapped_io_error_debug_prints_only_its_message() { + let error = MxcError::backend_error("wait failed") + .with_details(json!({"recoveryId": "secret"})) + .with_api_failure(ApiFailure::new("Iface.Wait").with_native_code("0x80004005")); + + assert_eq!( + format!("{:?}", into_io_error(error)), + format!("{:?}", std::io::Error::other("wait failed")) + ); + } + #[test] fn every_code_serialises_to_its_wire_string() { let cases = [ diff --git a/tests/configs/wslc_test_hook_marker.json b/tests/configs/wslc_test_hook_marker.json new file mode 100644 index 0000000000..3e3df4dcf5 --- /dev/null +++ b/tests/configs/wslc_test_hook_marker.json @@ -0,0 +1,15 @@ +{ + "$schema": "../../schemas/stable/mxc-config.schema.1.0.0.json", + "version": "1.0.0", + "containerId": "wslc-test-hook-marker", + "containment": "wslc", + "process": { + "commandLine": "true # mxc-test-force-wslc-get-exit-code-failure" + }, + "network": { + "egress": { "default": "deny" } + }, + "wslc": { + "image": "alpine:latest" + } +} diff --git a/tests/scripts/run_wslc_all_tests.ps1 b/tests/scripts/run_wslc_all_tests.ps1 index 9fc460b0c1..0a3aca294f 100644 --- a/tests/scripts/run_wslc_all_tests.ps1 +++ b/tests/scripts/run_wslc_all_tests.ps1 @@ -244,6 +244,7 @@ $null = $results.Add((Run-WslcTest "wslc_large_output.json")) # omitted one used to produce (WSLC_E_INVALID_SESSION_NAME, 0x80040608). Every # other config here sets the field, so this is the only guard on that path. $null = $results.Add((Run-WslcTest "wslc_no_container_id.json" -OutputContains "NO_CONTAINER_ID_OK")) +$null = $results.Add((Run-WslcTest "wslc_test_hook_marker.json" -As "the test-only exit-code failure command runs normally")) Write-Host "`n--- Environment Tests ---" -ForegroundColor Cyan # PYTHON_VERSION is baked into the image by ENV. Unlike PATH and TERM, no shell