diff --git a/hive-c0re/src/lifecycle/mod.rs b/hive-c0re/src/lifecycle/mod.rs index d2c72040..252adf94 100644 --- a/hive-c0re/src/lifecycle/mod.rs +++ b/hive-c0re/src/lifecycle/mod.rs @@ -806,7 +806,7 @@ pub async fn prebuild_toplevel(name: &str, flake_ref: &str, node_id: Option let pump_stderr = tokio::spawn(async move { let mut lines = BufReader::new(stderr).lines(); while let Ok(Some(line)) = lines.next_line().await { - if is_real_nix_warning(&line) { + if is_actionable_nix_line(&line) { tracing::warn!(target: "nix-prebuild", cmdline = %stderr_cmdline, "{line}"); } else { tracing::info!(target: "nix-prebuild", cmdline = %stderr_cmdline, "{line}"); @@ -893,19 +893,26 @@ pub fn ensure_agent_runtime_dir(name: &str) -> Result<()> { .with_context(|| format!("create agent runtime dir {}", dir.display())) } -/// Whether a `nix`/`nixos-container` stderr line is an actual warning -/// (nix's own convention: a genuine warning starts with `warning:`) rather -/// than routine progress chatter (`copying N paths…`, `fetching git -/// input…`, `structuredAttrs is enabled`, …) — nix writes the latter to -/// stderr too, and it dwarfs the former in volume. -fn is_real_nix_warning(line: &str) -> bool { - line.trim_start().starts_with("warning:") +/// Whether a `nix`/`nixos-container` stderr line is worth WARN (nix's own +/// warnings and errors) rather than routine progress chatter (`copying N +/// paths…`, `fetching git input…`, `structuredAttrs is enabled`, …) — nix +/// writes the latter to stderr too, and it dwarfs the former in volume. +/// Matches nix's own prefixes (`warning:`, `evaluation warning:`, +/// `error:`) rather than only `warning:`, so a real build failure +/// (`error: builder for '...' failed`, `error: 1 dependencies ... failed +/// to build`) never gets buried at INFO alongside the chatter. +fn is_actionable_nix_line(line: &str) -> bool { + let trimmed = line.trim_start(); + trimmed.starts_with("warning:") + || trimmed.starts_with("evaluation warning:") + || trimmed.starts_with("error:") } /// Build the per-line callback for `create_container_streaming` / /// `update_container_streaming`. Both ops share identical dispatch logic -/// (stdout → info + `append_stdout`, stderr → warn + `append_stderr`); this -/// helper avoids duplicating that match body across the two call sites. +/// (stdout → info + `append_stdout`, stderr → warn for a nix warning/error +/// and info otherwise + `append_stderr`); this helper avoids duplicating +/// that match body across the two call sites. fn make_log_callback( logs: Option>, log_id: Option, @@ -920,7 +927,7 @@ fn make_log_callback( } } PrivStream::Stderr => { - if is_real_nix_warning(line) { + if is_actionable_nix_line(line) { tracing::warn!(target: "nixos-container", cmdline = %cmdline, "{line}"); } else { tracing::info!(target: "nixos-container", cmdline = %cmdline, "{line}"); diff --git a/hive-c0re/src/lifecycle/tests.rs b/hive-c0re/src/lifecycle/tests.rs index 8fffe65d..bf5f2d3f 100644 --- a/hive-c0re/src/lifecycle/tests.rs +++ b/hive-c0re/src/lifecycle/tests.rs @@ -421,3 +421,31 @@ fn ensure_state_dir_creates_the_work_dir() { assert!(notes_dir.is_dir()); assert!(root.path().join(crate::paths::WORK_SUBDIR).is_dir()); } + +/// Real build failures (`error:`) and both of nix's own warning spellings +/// must stay WARN; routine progress chatter is the only thing demoted. +/// Regression guard for a first-pass predicate that only matched +/// `warning:` and silently buried `error:` lines at INFO. +#[test] +fn is_actionable_nix_line_keeps_warnings_and_errors_demotes_chatter() { + let actionable = [ + "warning: Lock file entry is unlocked", + " warning: leading whitespace", + "evaluation warning: this will be an error in a future release", + "error: builder for '/nix/store/foo.drv' failed with exit code 1", + "error: 1 dependencies of derivation '/nix/store/bar.drv' failed to build", + ]; + for line in actionable { + assert!(is_actionable_nix_line(line), "{line}"); + } + + let chatter = [ + "copying 3 paths", + "these 2 derivations will be built:", + "fetching git input 'git+file:///var/lib/hyperhive'", + "structuredAttrs is enabled", + ]; + for line in chatter { + assert!(!is_actionable_nix_line(line), "{line}"); + } +} diff --git a/hive-c0re/src/socket_server/mod.rs b/hive-c0re/src/socket_server/mod.rs index 1df9e322..057423d0 100644 --- a/hive-c0re/src/socket_server/mod.rs +++ b/hive-c0re/src/socket_server/mod.rs @@ -209,9 +209,11 @@ async fn read_request_line( }) } -/// Whether `e`'s root cause is the peer closing the connection — `serve`'s -/// own `?`s convert a raw `std::io::Error` straight to `anyhow::Error` with -/// no intervening `.context()`, so a direct downcast reaches it. +/// Whether `e`'s root cause is the peer closing the connection. +/// `downcast_ref` searches the whole error chain (anyhow's own +/// `object_downcast`, not just the outermost type), so this finds a +/// wrapped `std::io::Error` whether or not something added `.context()` +/// on top of it. fn is_broken_pipe(e: &anyhow::Error) -> bool { e.downcast_ref::().is_some_and(|io_err| { matches!( @@ -731,4 +733,31 @@ mod tests { assert_eq!(served, 1, "connection after EMFILE not served"); assert_eq!(err.raw_os_error(), Some(libc::EBADF)); } + + /// `is_broken_pipe` must recognise both peer-closed kinds whether the + /// `io::Error` reaches it bare (how `serve`'s own `?`s produce it) or + /// wrapped in `.context()` (anyhow's `downcast_ref` walks the whole + /// chain, so wrapping doesn't hide it — confirmed here rather than + /// assumed), and must not misclassify an unrelated `io::Error` or a + /// non-io `anyhow::Error`. + #[test] + fn is_broken_pipe_matches_peer_closed_kinds_only() { + let broken_pipe = std::io::Error::from(std::io::ErrorKind::BrokenPipe); + let conn_reset = std::io::Error::from(std::io::ErrorKind::ConnectionReset); + let permission_denied = std::io::Error::from(std::io::ErrorKind::PermissionDenied); + + assert!(is_broken_pipe(&anyhow::Error::new(broken_pipe))); + assert!(is_broken_pipe(&anyhow::Error::new(conn_reset))); + assert!(!is_broken_pipe(&anyhow::Error::new(permission_denied))); + + let contextual = anyhow::Error::new(std::io::Error::from(std::io::ErrorKind::BrokenPipe)) + .context("write response"); + assert!( + is_broken_pipe(&contextual), + "anyhow's downcast_ref searches the whole chain, so a \ + .context()-wrapped io::Error must still be found" + ); + + assert!(!is_broken_pipe(&anyhow::anyhow!("not an io error at all"))); + } }