microsoft / microsoft/mxc

[WSLC] State-aware daemon protocol flattens every backend failure to `backend_error`, discarding its typed classification

Open
#1,167 0 comments 0 reactions 1 assignee Claimed by @SohamDas2021 View on GitHub
Area-SDK-Api Container-WSLC Issue-Bug Needs-Triage OS-Windows Priority2
Dominant language
Rust
Stars
1.3k
Forks
79
Avg merge
2d 7h
Merged PRs (30d)
117

Description

### Relevant area(s)

Windows (WSLc backend)

### Brief description of your issue

Split out of #843, where this was **defect 4**. That issue records it as independent of the exec-streaming defects (1–3) and able to land on its own; this issue tracks it separately so the WSLc work can be scheduled without waiting on the cross-backend executor-framing work.

---

**The daemon protocol flattens every backend failure to `backend_error`, discarding its typed classification.**

Surfaced as a review comment on #1045. #794 gave the WSLc backend a typed `WslcError` whose variants carry a `FailurePhase` (`Unavailable` → `BackendUnavailable`, `Rejected` → `Rejected`, etc.), so the **one-shot / streaming** surface now reaches the Rust SDK with an accurate code via `mxc_engine::dispatch::map_spawn_error`. The **state-aware** surface does not, because the classification is destroyed crossing the daemon's named pipe: `sr_err` (`src/backends/wslc/daemon/src/session_manager.rs:59`) reduces the step helper's `ScriptResponse` to `anyhow!(resp.error_message)`, dropping `failure_phase`; that becomes `WorkerError::Backend` → `ErrKind::Backend` (`:82`); and `map_daemon_error` (`src/backends/wslc/common/src/state_aware.rs:310`) maps `ErrKind::Backend` to `MxcError::backend_error`.

Concretely: provisioning on a host with missing WSLc components returns `backend_error` to state-aware Rust SDK callers, where the one-shot surface returns `backend_unavailable`. There is no local preflight that could catch it earlier — `state_aware.rs` has no `ensure_host_supported` / `load_sdk_checked` call, and its only `backend_unavailable` (`connect_daemon`) covers an unreachable daemon, a different condition. So a caller still has to string-match the message to distinguish an unusable host from a rejected policy.

_Fix (moderate; daemon wire change):_ widen `ErrKind` with `Unavailable` and `Rejected` rather than plumbing `FailurePhase` through — `failure_phase` is a `ScriptResponse` concept that the state-aware boundary does not use, whereas `map_daemon_error` already maps `ErrKind` 1:1 onto `MxcError` codes, so the client side is a new match arm. Requires: a `PROTOCOL_VERSION` bump, somewhere on `WorkerError` to carry the classification so `sr_err` stops discarding it, skew handling for an **already-running** persistent daemon speaking the old protocol, and a `docs/wsl/wslc-state-aware.md` update.

### Steps to reproduce

1. On a Windows host **without** the WSLc components installed, build with `--features wslc`.
2. Drive a state-aware `provision` through the Rust SDK (`mxc_sdk::run_state_aware_json`) or the FFI (`mxc_state_aware`).
3. Observe the returned `MxcError` code.
4. Run the equivalent **one-shot** request against the same host and compare.

### Expected behavior

Both surfaces classify the same host condition identically: a host that cannot run WSLc returns `backend_unavailable`, and a policy the backend refuses returns `policy_validation` / `rejected`. A caller can branch on the code without string-matching the message.

### Actual behavior

Every daemon-side failure — unusable host, rejected policy, busy container, protocol fault — arrives as `backend_error`. The state-aware and one-shot surfaces disagree for the same condition.

### Additional context

- **Independent of the rest of #843.** The flattening affects **every phase** (provision / start / stop / deprovision as well as exec), because all of them return through the same `sr_err` → `ErrKind::Backend` → `backend_error` chain. It is not exec-specific and shares no files with #843's remaining defect 3, which lives in the shared executor (`src/core/wxc/src/main.rs`) and the shared Node SDK.
- **Pre-existing; not introduced by #1045.** #794's scope was the one-shot/streaming surface; the daemon path flattened errors before that PR and still does. #1045 was left as-is deliberately rather than growing a wire-protocol change late in review.
- **No new error codes are required.** `MxcErrorCode` already has `BackendUnavailable` and `PolicyValidation`, so this touches no arch-owned file and does not affect the C# `ErrorCode` parity gate.
- **The expensive part is protocol skew, not the code.** `PROTOCOL_VERSION` is currently `2` (`src/backends/wslc/common/src/daemon_protocol.rs:40`). Because the daemon is persistent and per-user, a new client can meet an already-running old daemon; that behaviour (refuse / force-restart / degrade) needs a deliberate decision.
- **Partial benefit lands immediately.** Four of the five phases (provision / start / stop / deprovision) surface the improved classification as soon as this ships, because they return a clean envelope through `parseNonExecResponse`. Only the post-admission **exec** failure path stays opaque until #843's defect 3 lands, and this change does not make that path worse.

**Current source references** (verified against `main`):

| Location | Role |
| --- | --- |
| `src/backends/wslc/daemon/src/session_manager.rs:59` | `sr_err` discards `failure_phase` |
| `src/backends/wslc/daemon/src/session_manager.rs:82` | `WorkerError::Backend` → `ErrKind::Backend` |
| `src/backends/wslc/common/src/state_aware.rs:310` | `map_daemon_error` collapses `Busy \| NotReady \| Protocol \| Backend` → `backend_error` |
| `src/backends/wslc/common/src/daemon_protocol.rs:40` | `PROTOCOL_VERSION` |
| `src/backends/wslc/common/src/daemon_protocol.rs:173` | `ErrKind` variants |

Source review comment: #1045 (`wslc/common/src/error.rs:6-12`). Split from #843.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.