Uh oh!
There was an error while loading. Please reload this page.
[WSLC] Encapsulate backend errors in a typed enum for the Rust SDK - #1045
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
Adds typed WSLc failure classification so Rust SDK callers can distinguish unavailable backends and rejected policies.
Changes:
- Introduces
WslcErrorwith lifecycle-phase mapping. - Migrates WSLc error construction while preserving messages.
- Maps rejected spawn failures to policy validation errors.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/core/mxc_engine/src/dispatch.rs | Maps rejected failures to PolicyValidation. |
src/backends/wslc/common/src/error.rs | Defines typed WSLc errors. |
src/backends/wslc/common/src/lib.rs | Exposes the error module. |
src/backends/wslc/common/src/container_steps.rs | Converts shared helper failures. |
src/backends/wslc/common/src/wsl_container_runner.rs | Converts one-shot runner failures. |
Suppressed comments (1)
src/backends/wslc/common/src/error.rs:67
SdkandHostdo not always occur during launch.wait_for_processcallssdk_errorforWslcGetProcessExitEvent/WslcGetProcessExitCodeafter the container process is live, andStartedContainer::wait_for_exitmaps a COM-entry failure toHost; those failures are therefore incorrectly reported asLaunchFailedinstead ofPostLaunchFailed. Split launch-time and runtime SDK/host errors, or carry the phase at construction.
// The SDK call or host bring-up failed; generally worth retrying.
WslcError::Sdk { .. } | WslcError::Host(_) => FailurePhase::LaunchFailed,
// The container was up but the run broke.
WslcError::Runtime(_) => FailurePhase::PostLaunchFailed,
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
| //! WSLc previously built every failure as a free-text [`ScriptResponse`], which | ||
| //! left `failure_phase` at its `None` default. Because | ||
| //! `mxc_engine::dispatch::map_spawn_error` discriminates on exactly that field, | ||
| //! every WSLc failure reached the Rust SDK as an opaque `backend_error` — so a | ||
| //! caller could only tell a missing-WSL host from a rejected policy by parsing | ||
| //! the message. Each variant here attributes the failure to a lifecycle phase | ||
| //! instead. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
src/backends/wslc/common/src/container_steps.rs:676
- This phase is dropped on the state-aware path.
load_sdk_checkedis called bywslc/daemon/src/session_manager.rs:357, but that file'ssr_errconverts the response toanyhow!(resp.error_message)and every such failure becomesWorkerError::Backend/ErrKind::Backend. Consequently a missing SDK or prerequisite still reachesrun_state_aware_jsonasbackend_error, so the advertised typed distinction only works for one-shot/streaming. Preserve this classification throughWorkerError,ErrKind, andmap_daemon_errorrather than erasing it at the daemon boundary.
let sdk = WslcSdk::load().map_err(|e| WslcError::Unavailable(e).into_response())?;
let mut missing = WslcComponentFlags::WSLC_COMPONENT_FLAG_NONE;
let hr = sdk.WslcGetMissingComponents(&mut missing);
if hr != S_OK {
return Err(sdk_error("WslcGetMissingComponents failed", hr, ""));
}
if missing.any_missing() {
return Err(WslcError::Unavailable(wslc_prerequisite_error(missing)).into_response());
src/backends/wslc/common/src/error.rs:67
- Classifying by error source rather than lifecycle point makes several new phases incorrect. For example,
wait_for_processcallssdk_errorforWslcGetProcessExitEventandWslcGetProcessExitCodeafter the process exists (wsl_container_runner.rs:1106,1198), while adjacent wait failures correctly useRuntime; state-aware exec has the same pattern incontainer_steps.rs:945,1035. Those SDK failures are now reported asLaunchFailedinstead ofPostLaunchFailed. Split SDK errors into launch/runtime variants (or pass the phase at construction) and classify each call site according to whether process creation already succeeded.
// The SDK call or host bring-up failed; generally worth retrying.
WslcError::Sdk { .. } | WslcError::Host(_) => FailurePhase::LaunchFailed,
// The container was up but the run broke.
WslcError::Runtime(_) => FailurePhase::PostLaunchFailed,
📖 Description
Summary
WSLc built every failure as a free-text
ScriptResponseviaScriptResponse::error, which leavesfailure_phaseat itsNonedefault. Sincemxc_engine::dispatch::map_spawn_errordiscriminates on exactly that field, all 54 WSLc error sites reached the Rust SDK as an opaquebackend_error— a caller could only tell a missing WSL host from a rejected policy by parsing the message.This adds
wslc_common::error::WslcError, mirroring the existingwindows_sandbox_lifecycle::error::OneShotErrorpattern, and routes the WSLc error sites through it:UnavailableBackendUnavailablebackend_unavailableRejectedRejectedpolicy_validationSdk/HostLaunchFailedbackend_errorRuntimePostLaunchFailedbackend_errormap_spawn_errorgains theRejected → policy_validationarm. No newErrorCodevariants, so the closed cross-surface set (Rust → engine → C ABI → C#) is untouched and the parity gate is unaffected.No user-visible message changes.
Displayreproduces each message verbatim; a unit test transcribesmain's oldsdk_errorbody as an oracle and asserts byte-identical rendering. The only behavioral change is thatfailure_phasenow carries a real value instead ofNone— nothing intests/,sdk/, orscripts/asserts on it, and the only production readers aremap_spawn_errorand telemetry'sclassify_failure(total over the enum).Validation
cargo fmt --all -- --check,cargo clippy -p wslc_common -p mxc_engine --all-targets -- -D warnings— cleancargo test --workspace— 3,912 passed / 0 failedwslc_commonwithlink-wslcsdk— 188/188, including 4 newerror::testsrun_wslc_all_tests.ps1): 54/57 state-aware + 24/25 one-shot — byte-identical to a pristinemainbaseline built and run on the same host. All 110 PASS/FAIL lines match exactly.The 3 E2E failures are pre-existing on
mainand are not caused by this PR: they come from the WSLcnetwork.proxygate inconfig_parser.rs(a file this PR does not touch), fail during config parsing before any WSLc dispatch, and are fixed by #1042. Once #1042 lands and this rebases onto it, the suite is 57/57 + 25/25.Closes#794
📋 Issue Type
GitHub Actions runs the PR validation build automatically. The ADO pipeline
(
MXC-PR-Build) is the Azure version of the PR pipeline, kept in parity with the GitHubActions build; it runs on merge to
main, and Microsoft reviewers with write access can trigger iton a PR with
/azp run. See docs/pull-requests.md.If the
dependency-feed-checkcheck fails on a new dependency, the crate must be added tothe feed before the PR can pass. See docs/pull-requests.md
for the steps.
Microsoft Reviewers: Open in CodeFlow