fix(startup): surface a "taking too long" notice when the daemon won't come up
F3: the daemon connect loop retried forever with only an animated spinner when the daemon was reachable-but-never-ready (stuck in RPC warmup / -28, or an external daemon that never finishes init) -- no error, no guidance, no escape. It now stamps connect_stall_since_ the moment the daemon first goes "reachable but not ready" (the warmup branch + applyDaemonInitStatus) and clears it on connect / disconnect / warmup-complete. A pure, unit-testable util::connectHasStalled() helper (new util/connect_stall.h, 45s default from ui.toml [screens.loading].stall-timeout-sec) drives a "Taking longer than expected" notice in renderLoadingOverlay(): a title, a reassuring body with elapsed seconds, and a full-node hint to Settings > Restart Daemon or the Console. The background retry keeps running underneath, so the notice self-clears the instant it connects. Guarded off while the daemon is in State::Error (that case is owned by the existing crash-count hint). The overlay is a pure draw-list layer with no interactive widgets, so this follows the existing crash-hint idiom (guidance text, not injected buttons); the stalled state is computed locally in the overlay, so the only new App member is connect_stall_since_. Adds testConnectHasStalled to test_phase4.cpp and three i18n keys to i18n.cpp (English source of truth; the res/lang/*.json back-fill is deferred to a single add_missing_translations.py run at the end of the batch). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -31,7 +31,7 @@ around a single shared helper. The connectivity-breaking security flip lands las
|
||||
| 3 | **F4** | `embedded_daemon.cpp` · `start()` | After F1/F2 so crash-count semantics are settled; its bail deliberately stays out of the crash path. | ☑ |
|
||||
| 4 | **F7** | `util/platform` · `connection.cpp` | Structural owner of the fs-error idiom + `ConnectionConfig` that F5/F6/F8 reuse. | ☑ |
|
||||
| 5 | **F6 + F5** | `app.cpp` · `verifySaplingParams()` | Same `startEmbeddedDaemon` / `verifySaplingParams` block; land together. | ☑ |
|
||||
| 6 | **F3** | `app.cpp` · `renderLoadingOverlay()` | After F1 — panel is guarded off during `State::Error` (owned by the crash-count hint). | ☐ |
|
||||
| 6 | **F3** | `app.cpp` · `renderLoadingOverlay()` | After F1 — panel is guarded off during `State::Error` (owned by the crash-count hint). | ☑ |
|
||||
| 7 | **F8** | `connection.cpp` · `tryConnect()` | Largest; only connectivity-breaking default flip — land last, with release notes. | ☐ |
|
||||
|
||||
---
|
||||
@@ -461,7 +461,16 @@ once). Wire `renderPlaintextRemoteRpcDialog` into the app modal-dispatch list.
|
||||
|
||||
## F3 — Unbounded connect spinner (deferred to step 6)
|
||||
|
||||
**Severity:** Medium · **Effort:** S (~3–5h) · **Status:** ☐
|
||||
**Severity:** Medium · **Effort:** S (~3–5h) · **Status:** ☑ landed & verified
|
||||
|
||||
> **As-built note.** `renderLoadingOverlay()` is a pure draw-list overlay with **no interactive
|
||||
> widgets** (the existing crash case at ~5289 already communicates via guidance *text*, relying on
|
||||
> the sidebar staying reachable). So rather than inject `ActionButton`s — which would fight the
|
||||
> non-interactive overlay — the stall notice follows that same idiom: a "Taking longer than
|
||||
> expected" title + a reassuring body (with elapsed seconds) + a full-node-gated hint ("Open
|
||||
> Settings → Restart Daemon, or check the Console"). This let me drop the planned
|
||||
> `WalletState::connect_stalled` flag too: the stalled state is computed locally in the overlay
|
||||
> from `connect_stall_since_`, so the only new member is `App::connect_stall_since_`.
|
||||
|
||||
The connect loop retries forever while `!state_.connected` (`app.cpp:1239`);
|
||||
`loading_timer_` only animates the spinner. Stamp `connect_stall_since_` when
|
||||
@@ -503,6 +512,7 @@ design record; see the shared-helper table above.
|
||||
|
||||
- **F1** — ☑ landed: `isRunning()` (POSIX) now reads the atomic `state_` (predicate `Running || Stopping`) instead of calling `waitpid`, leaving `monitorProcess()` the sole reaper. Clean build (all targets link); `ctest` 1/1 passing. Not unit-testable — needs the manual `kill -SEGV` repro before release.
|
||||
- **F2** — ☑ landed: `startProcess()` (POSIX) now creates a `FD_CLOEXEC` self-pipe before `fork()`; the child writes `errno` to it on `execv` failure, the parent reads EOF-vs-errno and, on failure, reaps the zombie + sets a precise `last_error_` ("not executable or wrong architecture") + returns `false` (so `start()` no longer reports `Running` for a daemon that never started). Parent-side `setpgid` is now best-effort with a `DEBUG_LOGF` on failure. Clean build; `ctest` 1/1 passing. Not unit-testable — needs the manual non-executable / wrong-arch-binary repro before release.
|
||||
- **F3** — ☑ landed: the connect loop now stamps `connect_stall_since_ = ImGui::GetTime()` the moment the daemon first goes "reachable but not ready" (warmup branch + `applyDaemonInitStatus`), and clears it in `onConnected` / `onDisconnected` / warmup-complete — all in `app_network.cpp`. The pure `util::connectHasStalled(stallSince, now, threshold)` helper (new `util/connect_stall.h`, default 45 s from `ui.toml`) drives a draw-list "Taking longer than expected" notice in `renderLoadingOverlay()` (title + elapsed-seconds body + full-node hint), guarded off while the daemon is in `State::Error`. Background retry continues, so the notice self-clears on connect. New `testConnectHasStalled` unit test (7 assertions). Clean build; `ctest` 1/1 passing. (Draw-list text, not buttons — see as-built note above.)
|
||||
- **F6** — ☑ landed: `verifySaplingParams()` now hash-verifies each Sapling param against its pinned canonical SHA-256 (from `build-lite-backend-artifact.sh`), replacing the existence-only check, so a truncated/corrupt-but-present param is rejected instead of failing later on a shielded op. A `<params_dir>/.sapling_verified` marker keyed on `size:mtime` skips re-hashing ~48 MB on every startup. Logic extracted to the injectable `verifySaplingParamsIn(dir, digests)`; new `testVerifySaplingParams` unit test (valid / marker fast-path / wrong-hash / truncated / missing). Clean build; `ctest` 1/1 passing.
|
||||
- **F5** — ☑ landed: `startEmbeddedDaemon()` now checks `extractEmbeddedResources()`'s return (abort with `sb_daemon_extract_failed` on failure) and the previously-dropped `copy_file` `error_code` in the daemon-binary fallback loop (abort with `sb_daemon_files_failed` incl. the dir), so a disk-full / truncated `dragonxd` write is surfaced up front instead of failing opaquely at spawn. An absent source file stays non-fatal. Two i18n keys added to `i18n.cpp`. Clean build; `ctest` 1/1 passing.
|
||||
- **F7** — ☑ landed: new non-throwing `Platform::ensureDirectory(dir, outError)` in `util/platform.{h,cpp}` with one consistent message. Replaces the unchecked/throwing directory-create sites at `main.cpp:730` (pre-init: now logs + `MessageBoxA` on Windows + `return 1`), `connection.cpp:216` (autoDetectConfig now uses the ec overload — **no more uncaught `filesystem_error`** — and sets the new `ConnectionConfig::dir_error`), and both `app.cpp` daemon-dir sites (surface via `daemon_status_` + `return false`). Primary connect path (`app_network.cpp:243`) checks `dir_error` and bails to the status line instead of mislabelling it "waiting for config". `embedded_resources.cpp:270` left as-is (already correct). New `testPlatformEnsureDirectory` unit test (existing-dir / fresh-nested / empty / parent-is-file). Clean build; `ctest` 1/1 passing.
|
||||
|
||||
Reference in New Issue
Block a user