diff --git a/hive-forge/src/verbs/ci_log.rs b/hive-forge/src/verbs/ci_log.rs index d131c78d..43ac7488 100644 --- a/hive-forge/src/verbs/ci_log.rs +++ b/hive-forge/src/verbs/ci_log.rs @@ -14,15 +14,17 @@ //! shows and `pr-status` surfaces as a CI context `target_url`). //! 2. **Durable persisted-log download** — when the streamer is pruned the //! step-log files still exist on disk (the web UI reads them fine after -//! the streamer 500s). We fetch them via the web download route -//! `///actions/runs//jobs//logs`, keyed -//! by the run's GLOBAL id (mirrors `artifact-get`: the REST runs list -//! translates run number → global id). This is a flat whole-job log with -//! no per-step framing, so `--step` is not honored on this path. +//! the streamer 500s). We fetch them via the web download route the run +//! page's "view raw logs" link uses, +//! `///actions/runs//jobs//attempt//logs`, +//! keyed by the per-repo run NUMBER (same value `--run` takes) with the +//! attempt segment. This is a flat whole-job log with no per-step framing, +//! so `--step` is not honored on this path. //! //! Result: live + recent runs get the rich streamed view; older / quickly- -//! pruned runs (the #1781 case argus + atlas both hit) still print via the -//! durable fallback instead of a bare "logs garbage-collected" error. +//! pruned runs (where `act_runner` has already pruned the live task record) +//! still print via the durable fallback instead of a bare +//! "logs garbage-collected" error. use anyhow::{Result, bail}; use clap::Args as ClapArgs; @@ -74,29 +76,46 @@ fn step_name(step: Option<&Value>) -> String { .to_owned() } -/// Try the live run-view streamer. Returns `Ok(true)` when it printed (or -/// emitted JSON for) actual log lines, `Ok(false)` when the streamer is -/// reachable but has no lines (pruned task), and `Err` on a transport / HTTP -/// failure (e.g. the `500 "resource does not exist"` a pruned run returns). -/// Both the `Ok(false)` and `Err` cases tell the caller to fall back to the -/// durable persisted-log download. -fn streamer_logs(client: &Client, path: &str, args: &Args) -> Result { +/// Why the live streamer didn't produce logs, so `run` can tell a real usage +/// error (which must surface) from a pruned/empty run (which falls back to the +/// durable persisted-log download). Avoids brittle string-matching on the +/// error message to make that distinction. +enum StreamerMiss { + /// A requested `--step` index is past the job's step count — a usage + /// error; `run` surfaces it instead of masking it with the fallback. + StepOutOfRange(String), + /// The streamer is reachable but has no log lines, OR a transport / HTTP + /// failure (e.g. the `500 "resource does not exist"` a pruned run + /// returns). Either way `run` falls back to the durable download. + Unavailable, +} + +/// Try the live run-view streamer. `Ok(())` means it printed (or emitted JSON +/// for) actual log lines. `Err(StreamerMiss::StepOutOfRange)` is a usage error +/// to surface; `Err(StreamerMiss::Unavailable)` tells the caller to fall back +/// to the durable persisted-log download. +fn streamer_logs(client: &Client, path: &str, args: &Args) -> Result<(), StreamerMiss> { // 1. Discover the job's steps (state is returned regardless of cursors). - let view = client.post_json_web(path, &json!({ "logCursors": [] }))?; + // A transport error / pruned-task 500 surfaces here → Unavailable. + let view = client + .post_json_web(path, &json!({ "logCursors": [] })) + .map_err(|_| StreamerMiss::Unavailable)?; let steps = steps_of(&view); if steps.is_empty() { - return Ok(false); + return Err(StreamerMiss::Unavailable); } // 2. Request the chosen step(s) from cursor 0, fully expanded. let want: Vec = match args.step { - Some(s) if s >= steps.len() => bail!( - "step {s} out of range — run #{} job {} has {} step(s) (0..{})", - args.run, - args.job, - steps.len(), - steps.len() - 1 - ), + Some(s) if s >= steps.len() => { + return Err(StreamerMiss::StepOutOfRange(format!( + "step {s} out of range — run #{} job {} has {} step(s) (0..{})", + args.run, + args.job, + steps.len(), + steps.len() - 1 + ))); + } Some(s) => vec![s], None => (0..steps.len()).collect(), }; @@ -104,7 +123,9 @@ fn streamer_logs(client: &Client, path: &str, args: &Args) -> Result { .iter() .map(|&s| json!({ "step": s, "cursor": 0, "expanded": true })) .collect(); - let resp = client.post_json_web(path, &json!({ "logCursors": cursors }))?; + let resp = client + .post_json_web(path, &json!({ "logCursors": cursors })) + .map_err(|_| StreamerMiss::Unavailable)?; let steps_log = resp .get("logs") @@ -113,12 +134,12 @@ fn streamer_logs(client: &Client, path: &str, args: &Args) -> Result { .cloned() .unwrap_or_default(); if steps_log.is_empty() { - return Ok(false); + return Err(StreamerMiss::Unavailable); } if client.json_mode() { - crate::verbs::print_json(&resp)?; - return Ok(true); + crate::verbs::print_json(&resp).map_err(|_| StreamerMiss::Unavailable)?; + return Ok(()); } // Human output: a header per step, then its lines in order. @@ -138,7 +159,7 @@ fn streamer_logs(client: &Client, path: &str, args: &Args) -> Result { } } } - Ok(true) + Ok(()) } /// Durable fallback: download the persisted whole-job log via the web route @@ -189,20 +210,18 @@ pub fn run(client: &Client, args: Args) -> Result<()> { let stream_path = format!("/{repo}/actions/runs/{}/jobs/{}", args.run, args.job); // 1. Live run-view streamer first — rich per-step framing for in-progress - // and recently-finished runs. A pruned run errors or returns no lines; - // both cases fall through to the durable download below. (An - // out-of-range `--step` is a real usage error and propagates.) + // and recently-finished runs. match streamer_logs(client, &stream_path, &args) { - Ok(true) => return Ok(()), + Ok(()) => return Ok(()), // An out-of-range `--step` is a real usage error, not a pruned run — // surface it instead of masking it with the fallback. - Err(e) if e.to_string().contains("out of range") => return Err(e), - // Pruned task (Err) or reachable-but-no-lines (Ok(false)) → fall back. - Ok(false) | Err(_) => {} + Err(StreamerMiss::StepOutOfRange(msg)) => bail!(msg), + // Pruned task or reachable-but-no-lines → fall through to the durable + // download (survives the act_runner task prune that 500s the streamer). + Err(StreamerMiss::Unavailable) => {} } - // 2. Durable persisted-log download (survives the act_runner task prune - // that 500s the streamer — the #1781 case). + // 2. Durable persisted-log download (the pruned-run case this verb fixes). persisted_logs(client, repo, &args) }