fix(cli): surface cloud-exec stream failures instead of silent success (#804)

DorianZheng · web-flow · commit d9b36fef621a · 2026-06-16T20:39:06.000+08:00
## What
Cloud `exec` streams stdout/stderr over a WebSocket. When that attach
fails
(e.g. a 401 on the upgrade), the client recovered the exit code via a
status
probe and reported a silent `exit 0` with no output — indistinguishable
from a
command that genuinely produced none.

## Changes
- `emit_or_fallback` now carries the disconnect cause in
`ExecResult.error_message`
  (was `None`); it is only reached when the output stream failed.
- `StreamManager::start()` surfaces a non-empty `error_message` as an
error
  (exit 1 + message) instead of dropping it.
- REST `401`/`403` mapping reads `unauthorized (HTTP 401)` / `forbidden
(HTTP 403)`
  instead of a bare `auth: &lt;body&gt;`.
- Adds `debug!`/`trace!` to the previously-uninstrumented WS attach path
(connect, per-frame, stdout/stderr, exit-frame, status-probe fallback).

## Verification
- Real cloud `exec`: before = silent `exit 0`, empty; after = `exit 1` +
clear cause.
- Unit reproducer
`terminal::tests::stream_manager_surfaces_exec_error_message_as_error`
verified RED→GREEN.
- 4 WS attach unit tests still pass.


&lt;!-- This is an auto-generated comment: release notes by coderabbit.ai
--&gt;

## Summary by CodeRabbit

* **Bug Fixes**
* Distinguished error messages for authentication (HTTP 401) versus
authorization (HTTP 403) failures
* Improved error reporting when process execution streaming is
interrupted

* **Chores**
  * Enhanced diagnostic logging for WebSocket connection handling

&lt;!-- end of auto-generated comment: release notes by coderabbit.ai --&gt;
diff --git a/src/boxlite/src/rest/error.rs b/src/boxlite/src/rest/error.rs
@@ -56,7 +56,13 @@ pub(crate) fn map_http_error(status: StatusCode, body: &ErrorModel) -> BoxliteEr
 pub(crate) fn map_http_status(status: StatusCode, text: &str) -> BoxliteError {
     match status.as_u16() {
         404 => BoxliteError::NotFound(text.to_string()),
-        401 | 403 => BoxliteError::Config(format!("auth: {}", text)),
+        // Keep the `auth:` prefix (callers key on it) but state the actual
+        // failure: 401 = credentials rejected (expired, or wrong credential
+        // type for this endpoint — e.g. cloud exec's WS attach requires an API
+        // key, not a browser/OIDC token); 403 = authenticated but not allowed
+        // (often a stale org/path_prefix — `auth login` re-resolves it).
+        401 => BoxliteError::Config(format!("auth: unauthorized (HTTP 401): {}", text)),
+        403 => BoxliteError::Config(format!("auth: forbidden (HTTP 403): {}", text)),
         // Bare 5xx with no envelope ⇒ an intermediary spoke, not us.
         // The most common cause is a proxy / load balancer that
         // couldn't reach the destination (Clash returns 502 with
diff --git a/src/boxlite/src/rest/litebox.rs b/src/boxlite/src/rest/litebox.rs
@@ -490,8 +490,12 @@ async fn attach_ws(
 ) {
     let path = format!("/boxes/{}/executions/{}/attach", box_id, execution_id);
     let stream = match client.connect_ws(&path).await {
-        Ok(s) => s,
+        Ok(s) => {
+            tracing::debug!(path = %path, "WS attach: connected");
+            s
+        }
         Err(e) => {
+            tracing::debug!(path = %path, error = %e, "WS attach: connect failed; falling back to status probe (output will not stream)");
             emit_or_fallback(
                 client,
                 box_id,
@@ -631,15 +635,30 @@ async fn attach_ws_pump(
                     // idle watchdog for the rest of the session.
                     first_frame_seen = true;
 
+                    tracing::trace!(
+                        kind = match &frame {
+                            Message::Binary(_) => "binary",
+                            Message::Text(_) => "text",
+                            Message::Close(_) => "close",
+                            Message::Ping(_) => "ping",
+                            Message::Pong(_) => "pong",
+                            Message::Frame(_) => "raw",
+                        },
+                        len = frame.len(),
+                        "WS attach: frame received",
+                    );
+
                     match frame {
                         Message::Binary(bytes) => {
                             if let Some((channel, payload)) = bytes.split_first() {
                                 let text = String::from_utf8_lossy(payload).into_owned();
                                 match *channel {
                                     0x01 => {
+                                        tracing::trace!(len = text.len(), "WS attach: stdout frame");
                                         let _ = stdout_tx.send(text);
                                     }
                                     0x02 => {
+                                        tracing::trace!(len = text.len(), "WS attach: stderr frame");
                                         let _ = stderr_tx.send(text);
                                     }
                                     other => {
@@ -650,6 +669,7 @@ async fn attach_ws_pump(
                         }
                         Message::Text(text) => match parse_control_frame(&text) {
                             ControlFrame::Exit { exit_code } => {
+                                tracing::debug!(exit_code, "WS attach: exit control frame");
                                 let _ = result_tx.send(ExecResult {
                                     exit_code,
                                     error_message: None,
@@ -681,6 +701,10 @@ async fn attach_ws_pump(
         // distinguish "exec really finished" from "transient WS drop".
         match probe_execution_status(client, box_id, execution_id).await {
             ProbeResult::Terminal(result) => {
+                tracing::debug!(
+                    cause = %disconnect_cause,
+                    "WS attach: disconnected without an exit frame — taking exit code from status probe (any unstreamed stdout/stderr is lost)"
+                );
                 let _ = result_tx.send(result);
                 return;
             }
@@ -826,6 +850,7 @@ async fn emit_or_fallback(
     result_tx: &mpsc::UnboundedSender<ExecResult>,
     cause: String,
 ) {
+    tracing::debug!(cause = %cause, "WS attach: emit_or_fallback — recovering exit code from status probe (stdout/stderr not streamed)");
     let status_path = format!("/boxes/{}/executions/{}", box_id, execution_id);
     let status_probe = tokio::time::timeout(
         std::time::Duration::from_secs(5),
@@ -834,9 +859,16 @@ async fn emit_or_fallback(
     if let Ok(Ok(info)) = status_probe.await {
         match info.status.as_str() {
             "completed" | "killed" | "timed_out" => {
+                // The exec finished server-side, but we only reach
+                // emit_or_fallback when the output stream failed (never
+                // connected, or dropped and couldn't reconnect), so stdout/
+                // stderr were not delivered to the caller. Surface the cause as
+                // an error_message rather than reporting a clean exit: a caller
+                // otherwise can't tell "command produced no output" from "we
+                // lost the output". The real exit code is still preserved.
                 let _ = result_tx.send(ExecResult {
                     exit_code: info.exit_code.unwrap_or(-1),
-                    error_message: None,
+                    error_message: Some(cause.clone()),
                 });
                 return;
             }
diff --git a/src/cli/src/terminal/mod.rs b/src/cli/src/terminal/mod.rs
@@ -191,7 +191,7 @@ impl<'a> StreamManager<'a> {
                                 h.abort();
                             }
                             if io_done {
-                                break exit_status.unwrap().exit_code;
+                                break exit_status.as_ref().unwrap().exit_code;
                             }
                         }
                         Err(e) => {
@@ -232,6 +232,15 @@ impl<'a> StreamManager<'a> {
             }
         };
 
+        // The exec carried a diagnostic (its output stream failed and the exit
+        // code was recovered out-of-band, or the container init died): surface
+        // it as an error instead of returning a silent, possibly-misleading
+        // exit code. Without this the CLI reports success while stdout/stderr
+        // were never delivered.
+        if let Some(message) = exit_status.as_ref().and_then(|s| s.error_message.clone()) {
+            anyhow::bail!("exec did not complete normally: {message}");
+        }
+
         Ok(exit_code)
     }
 }
@@ -460,4 +469,46 @@ mod tests {
             elapsed,
         );
     }
+
+    /// A result that carries an `error_message` (the exec output stream failed
+    /// and the exit code was recovered out-of-band, so stdout/stderr were never
+    /// delivered) must surface as an `Err`, not a silent exit code. Guards the
+    /// regression where the CLI reported a clean `exit 0` for a cloud exec whose
+    /// WS attach was rejected 401 and produced no output.
+    #[test]
+    fn stream_manager_surfaces_exec_error_message_as_error() {
+        let rt = tokio::runtime::Builder::new_multi_thread()
+            .worker_threads(2)
+            .enable_all()
+            .build()
+            .expect("build runtime");
+
+        let (mut exec, stdout_tx, stderr_tx, _stdin_rx, result_tx) =
+            boxlite::Execution::stub("stream-mgr-error-message");
+
+        let result = rt.block_on(async {
+            tokio::spawn(async move {
+                let _ = result_tx.send(boxlite::ExecResult {
+                    exit_code: 0,
+                    error_message: Some(
+                        "WS connect failed: WS auth rejected (401 Unauthorized)".to_string(),
+                    ),
+                });
+                drop(stdout_tx);
+                drop(stderr_tx);
+            });
+            StreamManager::new(&mut exec, /*interactive*/ false, /*tty*/ false)
+                .start()
+                .await
+        });
+
+        let err = result.expect_err(
+            "a result carrying error_message must surface as Err, not a silent exit code",
+        );
+        let rendered = format!("{err:#}");
+        assert!(
+            rendered.contains("401 Unauthorized"),
+            "surfaced error must include the stream-failure cause, got: {rendered}"
+        );
+    }
 }
