Skip to content

fix(desktop): add 10s timeout to connection-ipc fetches#92

Merged
hqhq1025 merged 1 commit intomainfrom
wt/loop-fix-connection-fetch-timeout
Apr 19, 2026
Merged

fix(desktop): add 10s timeout to connection-ipc fetches#92
hqhq1025 merged 1 commit intomainfrom
wt/loop-fix-connection-fetch-timeout

Conversation

@hqhq1025
Copy link
Copy Markdown
Collaborator

Summary

  • Wrap all three connection IPC fetch calls (connection:v1:test, models:v1:list, connection:v1:test-active) in a fetchWithTimeout helper backed by AbortController (default 10s).
  • Surface AbortError as a clear NETWORK hint so the user sees a timeout message instead of a permanently spinning button.
  • Add a unit test that drives the helper with a hanging fake fetch and asserts the abort fires.

Why

Without an AbortSignal, an unreachable or slow baseUrl would pin the renderer's "Test connection" spinner indefinitely (no main-side timeout, no Node-default fetch timeout). 10s is well above the <1s normal /models latency but short enough that the UI fails fast.

PRINCIPLES §5b

  • Compatibility green — same response shape, same error codes
  • Upgradeability green — CONNECTION_FETCH_TIMEOUT_MS exported as a single source of truth
  • No bloat green — pure stdlib, no new deps
  • Elegance green — one helper, three call sites

Test plan

  • pnpm --filter @open-codesign/desktop exec vitest run src/main/connection-ipc.test.ts — 41/41 pass (incl. new fetchWithTimeout abort test)
  • pnpm typecheck — green
  • pnpm lint — 0 errors (pre-existing complexity warnings unrelated)

The three IPC handlers (`connection:v1:test`, `models:v1:list`,
`connection:v1:test-active`) called `fetch` with no AbortSignal, so an
unreachable or slow baseUrl would pin the renderer's "Test connection"
spinner indefinitely.

Wrap each call in a `fetchWithTimeout` helper backed by AbortController
(default 10s) and surface AbortError as a clear NETWORK hint. Adds a
unit test covering the abort path.
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings

  • No issues found in added/modified lines with >=80% confidence.

Summary

  • Review mode: initial
  • No blocker/major/minor/nit issues found in the latest diff.
  • Hard-constraint checks in this diff: no new dependencies (license/size impact not introduced), no direct provider SDK imports, no UI token hardcoding touched, and no silent fallback added (errors are surfaced with explicit code/message/hint).
  • docs/VISION.md and docs/PRINCIPLES.md: Not found in repo/docs.
  • Residual risk: timeout handling is unit-tested via fetchWithTimeout, but there is no integration-style IPC test in this diff proving timeout classification/hints across all three handlers.

Testing

  • Not run (automation)

open-codesign Bot

@hqhq1025 hqhq1025 merged commit f1e7bb8 into main Apr 19, 2026
5 of 6 checks passed
@hqhq1025 hqhq1025 deleted the wt/loop-fix-connection-fetch-timeout branch April 19, 2026 06:36
@hqhq1025 hqhq1025 mentioned this pull request Apr 19, 2026
4 tasks
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.

1 participant