fix: surface zccache daemon start failures

[zackees]

Summary

• propagate zccache start failures from the ESP32 build path instead of logging and continuing
• capture and include zccache stdout/stderr in the build error so messages like zccache[err][D]: cannot start daemon reach the CLI
• add focused tests for stderr/stdout/empty-output formatting

Notes

• Current source already pins managed zccache to 1.12.7, so this does not bump zccache. The older 1.12.4 seen in local global fbuild logs appears to come from an installed/stale fbuild daemon, not current main.
• I checked the soldr/zccache global JSONL/log files under C:\Users\niteris\.soldr\cache\zccache\private\soldr-dev-5c2d0a9ed30121ce; they show lifecycle events such as replaced-comm-error and cache-store warnings, but not the exact orphan-pipe message from the issue. The useful fix is still to surface process stderr directly.

Tests

• soldr cargo fmt --all --check
• soldr cargo test --locked -p fbuild-build zccache --lib
• git diff --check

Fixes #623

Summary by CodeRabbit

• Bug Fixes
  • Improved error handling for cache daemon startup failures, ensuring build errors are properly detected and reported instead of being silently ignored.
  • Enhanced error messages to include relevant diagnostic details when cache daemon startup fails, helping with troubleshooting.

[coderabbitai]

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

ensure_running in zccache.rs is updated to return Result<()>, capturing zccache start stdout/stderr via piped Stdio, returning Ok(()) on success or Err(FbuildError::BuildFailed(...)) with a message from a new format_zccache_start_failure helper on failure. The ESP32 orchestrator call site adds ? to propagate that error.

Changes

zccache startup error propagation

Layer / File(s)	Summary
ensure_running Result return and failure formatting crates/fbuild-build/src/zccache.rs	Imports extended with Stdio and FbuildError; ensure_running changed to return Result<()> using cmd.output() with piped streams, returning Ok(()) on success or Err(FbuildError::BuildFailed(...)) on spawn failure or non-zero exit; format_zccache_start_failure constructs messages with path, exit status, stderr/stdout, and a daemon-logs hint when both streams are empty; unit tests cover all three message cases.
ESP32 orchestrator call site crates/fbuild-build/src/esp32/orchestrator/build.rs	The ensure_running call now uses ? to abort the build on error instead of silently ignoring the return value.

Sequence Diagram(s)

sequenceDiagram
  participant ESP32Orchestrator as ESP32 Build Orchestrator
  participant ensure_running
  participant ZccacheProcess as zccache start (subprocess)
  participant format_zccache_start_failure

  ESP32Orchestrator->>ensure_running: ensure_running(zcc)?
  ensure_running->>ZccacheProcess: Command::new(zccache).arg("start").stdout(Piped).stderr(Piped).output()
  alt spawn/output error or non-zero exit
    ZccacheProcess-->>ensure_running: Err or Output{status!=0, stdout, stderr}
    ensure_running->>format_zccache_start_failure: (path, exit_status, stderr, stdout)
    format_zccache_start_failure-->>ensure_running: formatted error string
    ensure_running-->>ESP32Orchestrator: Err(FbuildError::BuildFailed(...))
    note over ESP32Orchestrator: build aborted via ?
  else zero exit
    ZccacheProcess-->>ensure_running: Output{status==0}
    ensure_running-->>ESP32Orchestrator: Ok(())
  end

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• FastLED/fbuild#349: Directly modifies the same ensure_running function in crates/fbuild-build/src/zccache.rs, adding a poll-based startup timeout to the same daemon-start code path.

Poem

🐇 No more silent fails at the pipe,
The zccache error is finally ripe!
With ? in place, the build will shout,
"The daemon's gone — there is no doubt!"
Stderr captured, stdout too,
Even hints for when both streams are few. 🌟

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix: surface zccache daemon start failures' clearly summarizes the main change: making zccache startup errors visible to users instead of being silently ignored.
Linked Issues check	✅ Passed	The PR implements the primary objective from issue #623: surfacing zccache daemon start failures to users by capturing and returning stdout/stderr in error messages, with comprehensive test coverage for error formatting.
Out of Scope Changes check	✅ Passed	All changes are directly related to propagating zccache startup failures: updating return types, capturing process output, formatting error messages, and adding targeted tests. No unrelated modifications detected.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/zccache-error-surfacing-623

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}