Skip to content

fix(daemon): register test-emu endpoint OperationGuard and propagate exit code to CLI#140

Merged
zackees merged 1 commit intomainfrom
fix/test-emu-daemon-endpoint
Apr 19, 2026
Merged

fix(daemon): register test-emu endpoint OperationGuard and propagate exit code to CLI#140
zackees merged 1 commit intomainfrom
fix/test-emu-daemon-endpoint

Conversation

@zackees
Copy link
Copy Markdown
Member

@zackees zackees commented Apr 19, 2026

Fixes #130.

Summary

On Windows 10 users saw error: daemon error: request failed: error sending request for url (http://127.0.0.1:8765/api/test-emu) when running fbuild test-emu against a healthy daemon. A second bug made the CLI exit with code 0 even though it had just printed error:.

Root cause 1 — daemon tore down the connection mid-request

POST /api/test-emu did not construct an OperationGuard, so the daemon's 30 s self-eviction loop classified itself as "empty" during long (>30 s) ESP32/QEMU builds and triggered graceful shutdown. Graceful shutdown closed the in-flight HTTP connection, and reqwest surfaced that as error sending request for url.

Root cause 2 — CLI exited 0 on daemon failure

run_test_emu called std::process::exit(resp.exit_code) when resp.success == false. If the daemon (or an intermediate proxy) returned exit_code=0 alongside success=false, the CLI silently exited 0 and CI / shell scripts treated the failure as success.

Fix

  • handlers/operations.rs — expose OperationGuard as pub(crate).
  • handlers/emulator.rs::test_emu — construct an OperationGuard at the top of the handler (before any early-returns) so the self-eviction loop leaves the daemon alone for the entire build + emulate lifecycle.
  • fbuild-cli/src/main.rs::run_test_emu — clamp exit code to ≥1 when success=false, so daemon failures always surface as non-zero shell exits.

Tests

  • crates/fbuild-daemon/tests/test_emu_endpoint.rs — asserts the route is registered (structured 400 on missing project) and proves the OperationGuard is installed via touch_activity() side-effect. Reverting the emulator.rs fix causes this test to fail with idle_duration is 60.0001571s.
  • crates/fbuild-cli/tests/test_emu_exit_code.rs — spawns a mock HTTP daemon that returns success=false, exit_code=0 and asserts the CLI exits non-zero.

Manual repro (Windows 10)

Before:

$ fbuild test-emu tests/platform/uno -e uno --emulator simavr --timeout 5
error: daemon error: request failed: error sending request for url (...)
$ echo $?
0

After:

$ fbuild test-emu tests/platform/uno -e uno --emulator simavr --timeout 5
FBUILD_DEV_MODE=1 (dev mode: port 8865, ~/.fbuild/dev/)
emulator error: deploy failed: simavr is required but 'simavr.exe' was not found on PATH. ...
$ echo $?
1

fbuild build tests/platform/uno -e uno also builds + exits cleanly after the fix.

Test plan

  • uv run cargo clippy --workspace --all-targets -- -D warnings
  • uv run cargo fmt --all --check
  • uv run cargo test -p fbuild-daemon (2 new tests pass)
  • uv run cargo test -p fbuild-cli (1 new test passes)
  • uv run cargo test --workspace — 0 failures across 34 test binaries
  • Manual smoke: fbuild test-emu tests/platform/uno -e uno --emulator simavr --timeout 5 → non-zero exit + explicit diagnostic

🤖 Generated with Claude Code

…e to CLI

Fixes #130.

Root cause (daemon): the `POST /api/test-emu` handler did not construct
an `OperationGuard`, so the daemon's 30 s self-eviction loop classified
the daemon as "empty" during long (>30 s) ESP32/QEMU builds and forced
graceful shutdown mid-request. That tore down the in-flight HTTP
connection and surfaced on the CLI as
`error sending request for url (.../api/test-emu)` even though the
daemon was nominally "healthy" a moment earlier.

Root cause (CLI): `run_test_emu` called `std::process::exit(resp.exit_code)`
whenever the daemon reported `success=false`. If the daemon (or a proxy)
returned `exit_code=0` alongside `success=false`, the CLI silently
exited 0 — shells and CI treated the failure as success.

Changes:
- `handlers/operations.rs` — expose `OperationGuard` as `pub(crate)` so
  sibling handlers can register operations.
- `handlers/emulator.rs::test_emu` — construct an `OperationGuard` at
  the top of the handler so the self-eviction loop leaves the daemon
  alone for the full build + emulate lifecycle.
- `fbuild-cli/src/main.rs::run_test_emu` — clamp the exit code to ≥1
  whenever `success=false`, so daemon failures always surface as
  non-zero shell exits.

Tests:
- `crates/fbuild-daemon/tests/test_emu_endpoint.rs` — verifies the
  route is registered (structured 400 on missing project) and proves
  the `OperationGuard` is installed via `touch_activity` side-effect.
  Reverting the emulator.rs fix causes this test to fail with
  `idle_duration is 60.0001571s`.
- `crates/fbuild-cli/tests/test_emu_exit_code.rs` — spawns a mock
  HTTP daemon that returns `success=false, exit_code=0` and asserts
  the CLI exits non-zero.

Manual repro (Windows):
  before: CLI printed `error: daemon error: request failed: error
          sending request for url (http://127.0.0.1:8765/api/test-emu)`
          with exit 0 after long builds.
  after:  `fbuild test-emu tests/platform/uno -e uno --emulator simavr \
          --timeout 5` → "emulator error: ..." + exit 1.
          `fbuild build tests/platform/uno -e uno` → builds + exits 0.

Quality gates:
- `uv run cargo clippy --workspace --all-targets -- -D warnings` clean
- `uv run cargo fmt --all --check` clean
- `uv run cargo test --workspace` 0 failures across 34 test binaries

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 19, 2026

Warning

Rate limit exceeded

@zackees has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 0 minutes and 5 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 0 minutes and 5 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: a6eae5d9-79dc-464b-a1e2-dfe538926bbe

📥 Commits

Reviewing files that changed from the base of the PR and between 6e03e9b and 16ad548.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (7)
  • crates/fbuild-cli/src/main.rs
  • crates/fbuild-cli/tests/README.md
  • crates/fbuild-cli/tests/test_emu_exit_code.rs
  • crates/fbuild-daemon/Cargo.toml
  • crates/fbuild-daemon/src/handlers/emulator.rs
  • crates/fbuild-daemon/src/handlers/operations.rs
  • crates/fbuild-daemon/tests/test_emu_endpoint.rs
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/test-emu-daemon-endpoint

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

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

@zackees zackees merged commit a221f56 into main Apr 19, 2026
3 of 77 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fbuild test-emu: 'daemon error: request failed' on Windows despite healthy daemon

1 participant