fix(desktop): surface early daemon spawn failures - #1345
Open
mkdir700 wants to merge 2 commits into
Open
Conversation
`Command::spawn` returning Ok does not prove the daemon is running: on Windows `CreateProcess` succeeds even when the image loader later fails to resolve a dependency DLL (e.g. a missing vcruntime140.dll), so the process is created and then dies within milliseconds. A caller that only polls /health cannot tell that hard crash apart from a slow start until the (long) startup timeout elapses. Add the primitives to distinguish the two: - spawn_detached_daemon_returning_child hands back the Child handle so a caller can watch it; spawn_detached_daemon keeps its fire-and-forget contract as a thin drop wrapper. - poll_child_liveness / ChildLiveness give a transport-neutral view of whether the spawned child has already exited, with its exit code. - wait_for_daemon_health_watching_child mirrors wait_for_daemon_health but fails fast with DaemonBootstrapError::SpawnExitedEarly when the child exits before /health comes up. Probe-first ordering lets a genuinely healthy daemon win a race against a same-tick child exit. - SpawnExitedEarly renders the NTSTATUS exit code and, for known loader codes (STATUS_DLL_NOT_FOUND etc.), points at the likely fix (install the Visual C++ Redistributable). No existing caller behavior changes; the new API is additive. Refs #1259
On Windows a daemon binary that cannot launch (e.g. a missing vcruntime140.dll) lets CreateProcess/Command::spawn return Ok, so the GUI only noticed the failure via the 45s /health startup timeout and then showed a generic "couldn't reach the background service" — 45s of what looked like normal loading, followed by an unactionable message. Hold the spawned Child in spawn_external_and_wait_health and restart_local_daemon and wait via wait_for_daemon_health_watching_child, so a launch crash fails fast (~200-400ms) with the exit code and, for a missing-DLL NTSTATUS, an actionable hint. The failure flows through the existing DaemonBootstrapStatus -> get_daemon_bootstrap_failure path to the frontend error screen unchanged (classified Unavailable, with the actionable detail). Refs #1259
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughDaemon startup now retains spawned child handles, polls liveness during health checks, and reports immediate exits with optional exit-code details. Desktop bootstrap and restart flows use the new monitoring path. ChangesDaemon early-exit detection
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Desktop
participant Spawn
participant HealthWait
participant Child
Desktop->>Spawn: spawn daemon and retain Child
Spawn-->>Desktop: Child handle
Desktop->>HealthWait: wait for health with liveness callback
HealthWait->>Child: poll liveness
Child-->>HealthWait: Running or Exited with code
HealthWait-->>Desktop: readiness or bootstrap error
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Contributor
|
No React Doctor issues found. 🎉 Reviewed by React Doctor for commit |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/healthRoot cause
On Windows,
CreateProcesscan succeed before the image loader discovers a missing runtime DLL. The GUI therefore saw a successful spawn, dropped the child handle, and eventually reported only a generic health timeout. WatchingChild::try_wait()during health polling preserves the process exit code and distinguishes a hard launch failure from a slow startup.Fixes #1259.
Verification
cargo check -p uc-daemon-process --quietcargo test -p uc-daemon-process --lib --quiet(54 passed)cargo test -p uc-desktop daemon_probe --quiet(14 passed)Summary by CodeRabbit