Skip to content

fix(app): wait for local server readiness before bootstrap fan-out - #43370

Open
weiconghe wants to merge 2 commits into
anomalyco:devfrom
weiconghe:fix/app-bootstrap-server-ready
Open

fix(app): wait for local server readiness before bootstrap fan-out#43370
weiconghe wants to merge 2 commits into
anomalyco:devfrom
weiconghe:fix/app-bootstrap-server-ready

Conversation

@weiconghe

@weicongheweiconghe commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Issue for this PR

Closes#32379

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

On desktop startup the render process fires a fan-out of get/list requests through the v2 client (throwOnError: true) as soon as the bootstrap query mounts. The local sidecar server is started in parallel, so the first requests can hit a socket that is not listening yet, and the bootstrap query dies with an unhandled TypeError: Failed to fetch (call stack: render main-*.jsfetch → request → Object.get).

Two source defects contribute:

  1. isTransientError in packages/core/src/util/retry.ts classifies transient errors purely by message text (TRANSIENT_MESSAGES contains "failed to fetch"). That message is locale-dependent — on non-English systems the browser localizes it (e.g. ネットワークエラー), so the error is treated as non-transient and never retried. The cold-start race then surfaces as an unhandled exception instead of recovering on the next attempt.
  2. There is no readiness gate before the bootstrap fan-out: nothing waits for the local server to actually start listening.

Changes:

  • packages/core/src/util/retry.ts — add an explicit retryOnTypeError option (default off). fetch rejects with a bare TypeError when the target is unreachable and its message is locale-dependent, so it cannot be matched by text; callbacks whose failures are exclusively network requests opt in, while any other TypeError (a programming bug inside the callback) keeps failing fast after one attempt instead of being silently re-run.
  • packages/app/src/context/global-sync/bootstrap.ts, packages/app/src/context/server-session.ts — every retried network call site opts in with { retryOnTypeError: true }.
  • packages/app/src/utils/server-health.ts — new waitForServerReady(): bounded polling of the existing health check (defaults: 10s deadline / 250ms interval), reusing checkServerHealth; resolves false on timeout or abort instead of throwing. Each attempt is bounded by both its own budget and the time left before the overall deadline (min(pollMs*4, 1s, deadline - now)), so total wall clock stays within timeoutMs; and checkServerHealth now combines the caller's abort signal with its per-attempt timeout signal instead of choosing one or the other, so an aborted caller cuts off the in-flight request.
  • packages/app/src/context/server-sync.tsx — gate the global bootstrap queryFn and the per-directory fan-out (bootstrapInstance) on waitForServerReady() via a shared ensureServerReady() helper, guarded by ServerConnection.local(serverSDK.server) so remote server connections get no extra startup delay. When readiness fails, both gates now fail fast with a localized Could not reach <url> error (reuses the existing app.server.unreachable key, rendered via formatServerError) instead of silently firing the fan-out into a dead server; directory-bootstrap failures surface through the same toast used by sibling per-directory load failures.

Relation to earlier attempts: #41650 / #32468 retried MCP queries only, #28792 / #28707 are older and closed. This PR fixes the retry classification itself and adds the startup gate, in different files — not a duplicate of those.

How did you verify your code works?

  • packages/core/src/util/retry.test.ts: 6 cases — TypeError retried when opted in; localized TypeError message retried when opted in; programming TypeError fails after one attempt by default; message-matched transient errors still retried without opting in; attempts exhausted rethrows; custom retryIf takes precedence. Ran locally: bun test src/util/retry.test.ts (in packages/core) → 6 pass, 0 fail.
  • packages/app/src/utils/server-health.test.ts: 16 pass, 0 fail (bun test --conditions=solid --preload ./happydom.ts ./src/utils/server-health.test.ts). Includes the original 4 waitForServerReady cases plus 3 new ones: never-ready gating pins the wiring contract used by both gates (readiness failure ⇒ no data requests fired into the dead server and the caller gets an error); a hung fetch cannot push total wall clock past timeoutMs (old behavior overshot to ~timeoutMs + pollMs*4 + pollMs); a mid-poll caller abort cuts off the in-flight health request. One pre-existing checkServerHealth case was updated from asserting signal identity to asserting abort propagation, because the request now correctly receives a derived signal combining the caller's signal with the per-attempt timeout.
  • Full app suite: bun test --conditions=solid --preload ./happydom.ts → 729 pass, 0 fail.

Screenshots / recordings

N/A

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

- retry: treat network-layer TypeError as transient regardless of message
locale, so a cold-starting local server recovers on the next attempt
- app: add waitForServerReady() bounded health polling and gate the
bootstrap query + per-directory fan-out on it (local servers only)
Closesanomalyco#32379
@Enough1122

Copy link
Copy Markdown

AI code review — automated review for reference; please use your judgment.

  • packages/app/src/context/server-sync.tsx:324-329 — waitForServerReady resolves false when the sidecar never comes up, but the return value is ignored and bootstrapGlobal runs anyway — after burning the full 10s budget the fan-out still fires into a dead server and throws the exact Failed to fetch this PR sets out to prevent. On false, either throw a typed error (so the query lands in error state and formatServerError can render it) or surface an explicit "server unreachable" state.
  • packages/app/src/context/server-sync.tsx:486-490 — same gap in the directory-open path: a post-restart open that never becomes healthy silently proceeds after the timeout instead of failing fast with a meaningful error; propagate the negative result to the caller.
  • packages/core/src/util/retry.ts:23 — error instanceof TypeError classifies every TypeError as transient network noise for all callers of retry, so genuine programming bugs inside retried callbacks (calling an undefined method, bad argument types) are now silently re-executed attempts times, masking root causes and adding delay. Consider scoping this to the fetch layer (a wrapped fetch that tags network failures) or an explicit retryOnTypeError opt-in rather than a global rule.
  • packages/app/src/utils/server-health.ts:157-166 — the loop only checks Date.now() < deadline between iterations, and each attempt can take up to ${pollMs * 4}ms of fetch timeout before the poll wait, so total wall time can overshoot timeoutMs by seconds (default: up to ~1.25s per iteration); also the in-flight checkServerHealth call is never aborted when opts.signal fires. Pass remaining-time as the per-attempt timeout and forward the signal to the health request.
  • packages/app/src/utils/server-health.test.ts:180 — all four tests exercise the helper in isolation; nothing covers the actual behavioral fix (bootstrap/directory fan-out gated on readiness, including the never-ready case). An integration test pinning that bootstrapGlobal is skipped or errors when readiness fails would protect the change this PR exists for.

Address review feedback:
- server-sync: propagate readiness failure instead of ignoring it — the
bootstrap query and per-directory fan-out share ensureServerReady()
and throw a localized 'Could not reach <url>' error when the server
never becomes healthy; directory bootstrap failures surface through
the same toast as other per-directory load failures
- retry: make the TypeError rule an explicit retryOnTypeError opt-in so
programming errors inside retried callbacks fail fast; every network
call site in bootstrap.ts / server-session.ts opts in
- server-health: bound each poll attempt by the remaining deadline (no
wall-clock overshoot past timeoutMs) and forward the caller's abort
signal into checkServerHealth, which now combines it with its
per-attempt timeout signal instead of choosing one or the other
- tests: cover never-ready gating, wall-clock bound, mid-flight abort,
and the retry opt-in semantics
@weiconghe

Copy link
Copy Markdown
ContributorAuthor

Thanks for the careful review — all five points were real gaps in the PR as submitted. Addressed in ce12683:

  1. Bootstrap gate ignored the false result (server-sync.tsx) — both gates now go through a shared ensureServerReady() helper that throws a localized Could not reach <url> error (reusing the existing app.server.unreachable key) when readiness fails, so the query lands in error state and formatServerError can render it, instead of fanning out into a dead server after burning the 10s budget.
  2. Directory-open path — same helper; and the rejection now surfaces through the same toast the sibling per-directory loaders use (toast.project.reloadFailed.title + formatServerError), so a never-ready server produces a visible failure instead of either a silent proceed or an unhandled rejection. This also keeps the refresh-queue drain alive: bootstrapInstance no longer rejects into Promise.all mid-drain.
  3. Global TypeError rule — replaced with an explicit retryOnTypeError option (default off), enabled at each network call site in bootstrap.ts / server-session.ts. A programming TypeError inside a retried callback now fails after one attempt; message-based transient matching is unchanged, and an explicit retryIf still takes precedence.
  4. Wall-clock overshoot + abort forwarding (server-health.ts) — each attempt's timeout is now clamped by the remaining budget: min(pollMs * 4, 1s, deadline - now). checkServerHealth now combines the caller's signal with its per-attempt timeout signal (via AbortSignal.any, with a manual fallback) instead of choosing one or the other, so firing opts.signal cuts off the in-flight health request while a hung request still respects its own timeout.
  5. Tests — added: a gated-fan-out contract test (never-ready ⇒ zero data requests fired, caller gets the error); a hung-fetch test pinning total wall clock near timeoutMs; a mid-poll abort test asserting the in-flight request is actually cut off; and retry tests for both opt-in semantics and the new default (programming TypeError fails after one attempt). One pre-existing checkServerHealth case was updated from asserting signal identity to asserting abort propagation, since the request now receives a derived combined signal.

Verified locally on Windows: packages/core retry suite 6/6; app suite 729/729 (bun test --conditions=solid --preload ./happydom.ts). Local typecheck remains blocked by the pre-existing @opencode-ai/session-ui environment issue on this machine, same as before this change.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: Built-in tools (context7, grep_app, ast_grep) fail to register on startup due to bootstrap network race

2 participants

@weiconghe@Enough1122