diff --git a/docs/api-reference/dotnet/v1/types.md b/docs/api-reference/dotnet/v1/types.md index 589e84252..17f215e0f 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. `MxcPtyProcess` does not preserve these fields. + + ## `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 83d438ca1..8bb63bb28 100644 --- a/docs/api-reference/node/v1/types.md +++ b/docs/api-reference/node/v1/types.md @@ -469,6 +469,13 @@ export class MxcProcess { ``` +`wait()` rejects with `MxcError` when MXC cannot obtain a terminal process +result after admission. The rejection preserves `code`, `message`, +`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` 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 49954feb0..d01b52881 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 72f103f89..f7cc8ac74 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 5aad09dd8..2fbc68e6f 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,19 @@ 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. 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. + +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 cd15f3056..e160f7e23 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 61ad8488f..d197a0f15 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') { @@ -348,9 +359,28 @@ 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' = { + 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', + '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#' = { + Stop-WslcDaemons + 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 2d1a1a8b4..46610bba0 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 @@ -52,6 +57,7 @@ $(MxcTestRustcHostLine.Replace('host: ', '').Trim()) $(MxcTestCargoTargetDir)/$(MxcTestRustHostTriple)/$(MxcTestCargoProfileDir)/$(MxcTestNativeFileName) + $(MxcTestWslcCargoTargetDir)/$(MxcTestRustHostTriple)/$(MxcTestCargoProfileDir)/wxc-wslc-daemon.exe @@ -59,6 +65,17 @@ Text="The test-support native build did not produce $(MxcTestNativeOutput)." /> + + + + + + diff --git a/sdk/dotnet/Microsoft.Mxc.Sdk.Tests/V1/MxcContainerWslcE2ETests.cs b/sdk/dotnet/Microsoft.Mxc.Sdk.Tests/V1/MxcContainerWslcE2ETests.cs index cfc4d61b8..c196a4547 100644 --- a/sdk/dotnet/Microsoft.Mxc.Sdk.Tests/V1/MxcContainerWslcE2ETests.cs +++ b/sdk/dotnet/Microsoft.Mxc.Sdk.Tests/V1/MxcContainerWslcE2ETests.cs @@ -15,6 +15,9 @@ namespace Microsoft.Mxc.Sdk.Tests.V1; [Collection("MxcLiveHost")] public class MxcContainerWslcE2ETests { + private const string ForcedWaitFailureCommand = + "true # mxc-test-force-wslc-get-exit-code-failure"; + private static void RequireWslc() { #if MXC_WITH_WSLC @@ -26,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() { @@ -330,4 +340,64 @@ public async Task Run_GivesACommandThatReadsStdinEndOfFile() Assert.Equal(0, run.ExitCode); Assert.Equal("RUN-SAW-EOF", run.Stdout.Trim()); } + + [Fact] + public void Run_PreservesStructuredPostAdmissionWaitFailure() + { + RequireWslc(); + var containerName = UniqueContainerName(); + + var error = Assert.Throws(() => + MxcContainer.Run(Request(ForcedWaitFailureCommand, containerName))); + + AssertForcedWaitFailure(error); + AssertContainerNameReleased(containerName, "RUN-RECOVERED"); + } + + [Fact] + public async Task Spawn_WaitPreservesStructuredPostAdmissionWaitFailure() + { + RequireWslc(); + var containerName = UniqueContainerName(); + + using (var process = MxcContainer.Spawn(Request(ForcedWaitFailureCommand, containerName))) + { + AssertForcedWaitFailure(PollForWaitFailure(process)); + AssertForcedWaitFailure(Assert.Throws(() => process.Wait())); + AssertForcedWaitFailure(await Assert.ThrowsAsync( + () => process.WaitAsync(TestContext.Current.CancellationToken))); + } + + 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) + { + 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 00ac7792c..b2a59fb43 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( + "exec failed: 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 9f0bd4c65..5b8494857 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 185d60cc0..125a990c0 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 68801c275..ab5cfad8b 100644 --- a/sdk/dotnet/README.md +++ b/sdk/dotnet/README.md @@ -251,6 +251,12 @@ 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. +`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 a8cb95160..dc60ddbc9 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. `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/src/bindings/streaming.ts b/sdk/node/src/bindings/streaming.ts index 810b6bfdb..4ebd50831 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 a1780fa0f..e74a9a4ea 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'; @@ -26,6 +27,22 @@ 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'; +const uniqueContainerName = () => `mxc-wait-${randomUUID().replace(/-/g, '')}`; + +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 +233,40 @@ 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(namedRequest(forcedWaitFailureCommand, containerName)), + assertForcedWaitFailure, + ); + + await assertContainerNameReleased(containerName, 'RUN-RECOVERED'); + }); + + it('spawned process wait preserves structured post-admission wait failures', { timeout: 180_000 }, async () => { + const containerName = uniqueContainerName(); + const process = await sdk.spawn(namedRequest(forcedWaitFailureCommand, containerName)); + try { + await assert.rejects(process.wait(), assertForcedWaitFailure); + await assert.rejects(process.wait(), assertForcedWaitFailure); + } finally { + process.dispose(); + } + + await assertContainerNameReleased(containerName, '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 5ebd61374..1b416523c 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, + 'exec failed: 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 4bedacd6e..d3daaa698 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 a6510bf72..bd803cd41 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 2602b269a..0810ba6dd 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 331d6a801..05e92e3cc 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 bc6b196a4..6b38a97e6 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 741295581..7491e7040 100644 --- a/src/mxc-sdk/src/backends/wslc/common/container_steps.rs +++ b/src/mxc-sdk/src/backends/wslc/common/container_steps.rs @@ -1305,6 +1305,34 @@ fn natural_exit_confirmed(exit_wait: ExitWait, callback_confirmed: bool) -> bool || (matches!(exit_wait, ExitWait::NoEvent) && callback_confirmed) } +#[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"; + +#[cfg(any(test, all(feature = "test-support", debug_assertions)))] +fn forced_exit_code_result( + script_code: &str, + natural_exit_confirmed: bool, + result: HRESULT, +) -> HRESULT { + if result == S_OK && natural_exit_confirmed && script_code == TEST_EXIT_CODE_FAILURE_SCRIPT { + 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 +} + 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)); @@ -1531,9 +1559,14 @@ pub unsafe fn exec_in_container( } let mut exit_code: i32 = -1; - let exit_code_hr = sdk.WslcGetProcessExitCode(process_guard.as_raw(), &mut exit_code); + let natural_exit_confirmed = natural_exit_confirmed(exit_wait, confirmed); + let exit_code_hr = test_exit_code_result( + script_code, + natural_exit_confirmed, + sdk.WslcGetProcessExitCode(process_guard.as_raw(), &mut exit_code), + ); if exit_code_hr != S_OK { - if natural_exit_confirmed(exit_wait, confirmed) { + if natural_exit_confirmed { return Err(sdk_step_error("WslcGetProcessExitCode", exit_code_hr, "")); } let _ = writeln!( @@ -1738,6 +1771,49 @@ mod tests { use super::*; use crate::mxc_common::models::FailurePhase; + #[test] + fn test_exit_code_failure_requires_the_exact_successful_natural_exit() { + let access_denied = 0x8007_0005_u32 as HRESULT; + assert_eq!( + forced_exit_code_result(TEST_EXIT_CODE_FAILURE_SCRIPT, true, S_OK), + E_FAIL + ); + assert_eq!( + forced_exit_code_result(TEST_EXIT_CODE_FAILURE_SCRIPT, true, access_denied), + access_denied + ); + assert_eq!( + forced_exit_code_result(TEST_EXIT_CODE_FAILURE_SCRIPT, false, S_OK), + S_OK + ); + assert_eq!( + forced_exit_code_result( + "true # mxc-test-force-wslc-get-exit-code-failure ", + true, + S_OK + ), + S_OK + ); + } + + #[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/error.rs b/src/mxc-sdk/src/backends/wslc/common/error.rs index d64e92c3c..3c88202ac 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 46bb696bf..f4b508d6f 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 cfc5513e7..a45922e59 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 @@ -716,7 +722,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; @@ -761,7 +767,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(); @@ -825,9 +831,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 @@ -843,7 +853,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(); @@ -1358,6 +1368,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(), }) } @@ -1528,7 +1540,7 @@ impl WSLContainerRunner { // context and unloading the DLL under live callbacks, so settle // the container first. started.quiesce(logger); - e + e.into_response() } }; @@ -1909,6 +1921,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 { @@ -1937,7 +1951,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) } @@ -1945,17 +1959,32 @@ 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 exit_event = self - .exit_event() - .map_err(|e| WslcError::Runtime(e).into_response())?; + let _com = ComApartment::enter().map_err(|e| WslcError::Host(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) } } + 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); } @@ -1980,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); } @@ -2007,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. @@ -2021,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) @@ -2258,17 +2284,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 dd8934586..96873234d 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 928ee11c2..ec1507838 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,34 @@ impl fmt::Display for MxcError { impl std::error::Error for MxcError {} +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) + } +} + +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 { @@ -327,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/src/mxc-sdk/src/core/mxc_engine/error.rs b/src/mxc-sdk/src/core/mxc_engine/error.rs index c9eaf5009..c5a382647 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 3841b6b98..5ef4e946a 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 3584d7d22..ed81c4db9 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 { diff --git a/tests/configs/wslc_test_hook_marker.json b/tests/configs/wslc_test_hook_marker.json new file mode 100644 index 000000000..3e3df4dcf --- /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 9fc460b0c..0a3aca294 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