build: surface a panicked output-reader thread
run_command_capturing discarded the pump threads' join results: a reader that died mid-capture (UI sink or log writer failing) reported a successful build with truncated captured logs. A reader panic now fails the command; when the child itself failed first, its error keeps precedence and the reader panic is logged so the truncated output is not silently lost.
This commit is contained in:
+36
-3
@@ -815,7 +815,9 @@ struct CommandFailure {
|
|||||||
/// it (live view + tee log) while the stderr is additionally captured for
|
/// it (live view + tee log) while the stderr is additionally captured for
|
||||||
/// the failure classification; otherwise stdio is inherited from the
|
/// the failure classification; otherwise stdio is inherited from the
|
||||||
/// terminal. Returns an error (with the captured stderr) on non-zero exit
|
/// terminal. Returns an error (with the captured stderr) on non-zero exit
|
||||||
/// status.
|
/// status. A panicking stdout/stderr reader thread also yields an error
|
||||||
|
/// (the captured output would be incomplete), but only after the command's
|
||||||
|
/// own failure, which takes precedence.
|
||||||
fn run_command_capturing(
|
fn run_command_capturing(
|
||||||
cwd: &Path,
|
cwd: &Path,
|
||||||
program: &str,
|
program: &str,
|
||||||
@@ -837,6 +839,8 @@ fn run_command_capturing(
|
|||||||
// classification.
|
// classification.
|
||||||
let stderr_capture = Arc::new(std::sync::Mutex::new(String::new()));
|
let stderr_capture = Arc::new(std::sync::Mutex::new(String::new()));
|
||||||
|
|
||||||
|
// Printable panic message from a reader thread, if one died mid-pump.
|
||||||
|
let mut reader_panic = None;
|
||||||
let status = match sink {
|
let status = match sink {
|
||||||
None => cmd.status().map_err(|e| CommandFailure {
|
None => cmd.status().map_err(|e| CommandFailure {
|
||||||
error: format!("failed to run '{}': {}", program, e).into(),
|
error: format!("failed to run '{}': {}", program, e).into(),
|
||||||
@@ -873,8 +877,11 @@ fn run_command_capturing(
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
let _ = out_thread.join();
|
// The threads end on EOF, i.e. once the child exited and closed
|
||||||
let _ = err_thread.join();
|
// its streams; a panic from either means the captured output is
|
||||||
|
// incomplete.
|
||||||
|
reader_panic = reader_panic_message(out_thread.join())
|
||||||
|
.or_else(|| reader_panic_message(err_thread.join()));
|
||||||
|
|
||||||
child.wait().map_err(|e| CommandFailure {
|
child.wait().map_err(|e| CommandFailure {
|
||||||
error: format!("failed to wait for '{}': {}", program, e).into(),
|
error: format!("failed to wait for '{}': {}", program, e).into(),
|
||||||
@@ -884,6 +891,11 @@ fn run_command_capturing(
|
|||||||
};
|
};
|
||||||
|
|
||||||
if !status.success() {
|
if !status.success() {
|
||||||
|
// The command's own failure takes precedence over a dead reader; the
|
||||||
|
// panic is still logged so the truncated-log cause is not lost.
|
||||||
|
if let Some(message) = &reader_panic {
|
||||||
|
log::error!("the build output reader failed: {message}");
|
||||||
|
}
|
||||||
return Err(CommandFailure {
|
return Err(CommandFailure {
|
||||||
error: format!(
|
error: format!(
|
||||||
"'{} {}' failed with status: {}",
|
"'{} {}' failed with status: {}",
|
||||||
@@ -895,9 +907,30 @@ fn run_command_capturing(
|
|||||||
stderr: std::mem::take(&mut stderr_capture.lock().unwrap_or_else(|e| e.into_inner())),
|
stderr: std::mem::take(&mut stderr_capture.lock().unwrap_or_else(|e| e.into_inner())),
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// The command succeeded but a reader thread panicked: the captured output
|
||||||
|
// (live view + tee log) is incomplete, so this cannot pass as a success.
|
||||||
|
if let Some(message) = reader_panic {
|
||||||
|
return Err(CommandFailure {
|
||||||
|
error: format!("the build output reader failed: {message}").into(),
|
||||||
|
stderr: std::mem::take(&mut stderr_capture.lock().unwrap_or_else(|e| e.into_inner())),
|
||||||
|
});
|
||||||
|
}
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Extract a printable message from a reader-thread join result; `None` when
|
||||||
|
/// the thread finished normally.
|
||||||
|
fn reader_panic_message(join: std::thread::Result<()>) -> Option<String> {
|
||||||
|
join.err().map(|payload| {
|
||||||
|
payload
|
||||||
|
.downcast_ref::<&str>()
|
||||||
|
.map(|s| (*s).to_string())
|
||||||
|
.or_else(|| payload.downcast_ref::<String>().cloned())
|
||||||
|
.unwrap_or_else(|| "non-string panic payload".to_string())
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
/// Run a build command, discarding the captured stderr.
|
/// Run a build command, discarding the captured stderr.
|
||||||
fn run_command(
|
fn run_command(
|
||||||
cwd: &Path,
|
cwd: &Path,
|
||||||
|
|||||||
Reference in New Issue
Block a user