screenshare: surface pixelpass stderr on host startup failure
When a pixelpass host dies before emitting its ticket (e.g. a fresh Ubuntu box missing the GStreamer capture stack), we discarded its stderr — which is exactly where pixelpass prints the failing element plus an actionable "Install hint: sudo apt install ..." line. The user only saw a dead-end "pixelpass host exited before emitting a ticket". Capture stderr instead of nulling it, and on the early-exit path fold a short tail into our error via the pure `pixelpass_failure_detail` (drops the decorative banner, keeps the error + hint, caps at the last 12 lines). On the success path stderr is drained in the background so its pipe can't stall the host. +3 tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
+109
-4
@@ -332,7 +332,13 @@ pub async fn spawn_host(
|
||||
.args(host_args(audio_app))
|
||||
.stdin(Stdio::null())
|
||||
.stdout(Stdio::piped())
|
||||
.stderr(Stdio::null())
|
||||
// Capture stderr (not null): pixelpass prints its startup precondition
|
||||
// failures there — a missing GStreamer plugin / `pactl`, each with an
|
||||
// actionable "Install hint: sudo apt install ..." line. If the host dies
|
||||
// before its ticket we fold that tail into our error so the user sees
|
||||
// *what to install* instead of a dead-end "exited before a ticket". On
|
||||
// the success path we drain it in the background so the pipe can't fill.
|
||||
.stderr(Stdio::piped())
|
||||
.kill_on_drop(true)
|
||||
.spawn()?;
|
||||
|
||||
@@ -340,6 +346,7 @@ pub async fn spawn_host(
|
||||
.stdout
|
||||
.take()
|
||||
.ok_or_else(|| std::io::Error::other("pixelpass host stdout missing"))?;
|
||||
let stderr = child.stderr.take();
|
||||
let mut lines = BufReader::new(stdout).lines();
|
||||
|
||||
let ticket = match read_until(&mut lines, |e| match e {
|
||||
@@ -351,9 +358,10 @@ pub async fn spawn_host(
|
||||
Ok(Some(t)) => t,
|
||||
Ok(None) => {
|
||||
let _ = child.kill().await;
|
||||
return Err(std::io::Error::other(
|
||||
"pixelpass host exited before emitting a ticket",
|
||||
));
|
||||
let detail = read_stderr_tail(stderr).await;
|
||||
return Err(std::io::Error::other(format!(
|
||||
"pixelpass host exited before emitting a ticket{detail}"
|
||||
)));
|
||||
}
|
||||
Err(e) => {
|
||||
let _ = child.kill().await;
|
||||
@@ -361,10 +369,65 @@ pub async fn spawn_host(
|
||||
}
|
||||
};
|
||||
|
||||
if let Some(stderr) = stderr {
|
||||
drain_stderr_in_background(stderr);
|
||||
}
|
||||
drain_in_background(lines, "host", notices);
|
||||
Ok((child, ticket))
|
||||
}
|
||||
|
||||
/// Read a killed pixelpass child's stderr to EOF and reduce it to a short,
|
||||
/// user-facing diagnostic tail via [`pixelpass_failure_detail`]. Bounded: the
|
||||
/// caller kills the child first, so the pipe EOFs promptly. Returns an empty
|
||||
/// string when stderr was already taken or carried nothing useful.
|
||||
async fn read_stderr_tail(stderr: Option<tokio::process::ChildStderr>) -> String {
|
||||
use tokio::io::AsyncReadExt;
|
||||
let Some(mut stderr) = stderr else {
|
||||
return String::new();
|
||||
};
|
||||
let mut buf = Vec::new();
|
||||
let _ = stderr.read_to_end(&mut buf).await;
|
||||
pixelpass_failure_detail(&String::from_utf8_lossy(&buf))
|
||||
}
|
||||
|
||||
/// Discard a running pixelpass child's stderr in the background so its pipe
|
||||
/// can't fill and stall the host (mirrors [`drain_in_background`] for stdout).
|
||||
fn drain_stderr_in_background(mut stderr: tokio::process::ChildStderr) {
|
||||
use tokio::io::AsyncReadExt;
|
||||
tokio::spawn(async move {
|
||||
let mut buf = [0u8; 4096];
|
||||
while let Ok(n) = stderr.read(&mut buf).await {
|
||||
if n == 0 {
|
||||
break;
|
||||
}
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
/// Extract a human-useful tail from a failed pixelpass child's stderr to append
|
||||
/// to our error. pixelpass writes actionable startup errors there (a missing
|
||||
/// GStreamer element / `pactl` plus an `Install hint: sudo apt install ...`
|
||||
/// line), which is exactly what a freshly-installed host needs to see. The
|
||||
/// decorative host banner (box-drawing) is dropped — it only prints on the
|
||||
/// success path, but we filter it defensively. Pure: no I/O. Returns an empty
|
||||
/// string when there's nothing worth surfacing (so callers can append blindly).
|
||||
pub fn pixelpass_failure_detail(stderr: &str) -> String {
|
||||
let useful: Vec<&str> = stderr
|
||||
.lines()
|
||||
.map(str::trim_end)
|
||||
.filter(|l| !l.trim().is_empty())
|
||||
.filter(|l| !l.trim_start().starts_with(['│', '┌', '└', '├']))
|
||||
.collect();
|
||||
if useful.is_empty() {
|
||||
return String::new();
|
||||
}
|
||||
// The anyhow error and its install hint are the *last* lines printed, so
|
||||
// keep the tail rather than the head.
|
||||
const MAX_LINES: usize = 12;
|
||||
let start = useful.len().saturating_sub(MAX_LINES);
|
||||
format!("\n\npixelpass reported:\n{}", useful[start..].join("\n"))
|
||||
}
|
||||
|
||||
/// Spawn a pixelpass viewer for `ticket`, wait for it to connect, and open the
|
||||
/// stream in a local player (mpv, falling back to vlc). Returns the live viewer
|
||||
/// child so the caller can kill it on room-leave; it also self-exits when the
|
||||
@@ -633,6 +696,48 @@ mod tests {
|
||||
assert_eq!(parse_audio_apps(stdout.as_bytes()), vec!["mpv".to_string()]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn failure_detail_surfaces_install_hint_and_drops_banner() {
|
||||
// The real shape of a fresh-host failure: anyhow error + install hint on
|
||||
// stderr. We must keep those (so the user knows what to apt install) and
|
||||
// drop the decorative banner box-drawing lines.
|
||||
let stderr = "\
|
||||
┌─ PixelPass · host ─────────────────────────────────────────
|
||||
│ display server : Wayland
|
||||
└────────────────────────────────────────────────────────────
|
||||
Error: GStreamer element `vah264enc` not available.
|
||||
Install hint: sudo apt install gstreamer1.0-plugins-bad
|
||||
";
|
||||
let detail = pixelpass_failure_detail(stderr);
|
||||
assert!(detail.starts_with("\n\npixelpass reported:\n"));
|
||||
assert!(detail.contains("vah264enc` not available"));
|
||||
assert!(detail.contains("sudo apt install gstreamer1.0-plugins-bad"));
|
||||
assert!(!detail.contains('│'), "banner box-drawing must be dropped");
|
||||
assert!(!detail.contains('┌'));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn failure_detail_empty_when_nothing_useful() {
|
||||
// Blank / banner-only stderr yields an empty string so the caller can
|
||||
// append it to the base message unconditionally without trailing noise.
|
||||
assert_eq!(pixelpass_failure_detail(""), "");
|
||||
assert_eq!(pixelpass_failure_detail(" \n \n"), "");
|
||||
assert_eq!(
|
||||
pixelpass_failure_detail("│ display server : Wayland\n│ capture : x\n"),
|
||||
""
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn failure_detail_keeps_only_the_tail() {
|
||||
// A long stderr is truncated to its last lines (where the real error
|
||||
// and hint live), not its head.
|
||||
let body: String = (0..30).map(|i| format!("line {i}\n")).collect();
|
||||
let detail = pixelpass_failure_detail(&body);
|
||||
assert!(detail.contains("line 29"));
|
||||
assert!(!detail.contains("line 0\n"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn help_probe_detects_strict_audio_flag() {
|
||||
// A new pixelpass advertises the flag; an old one doesn't. The probe must
|
||||
|
||||
Reference in New Issue
Block a user