Skip to content

HARDEN: desktop ingest retry + fetch timeouts + heartbeat backoff#160

Open
erikolsson wants to merge 3 commits into
mainfrom
harden/01-desktop-ingest-reliability
Open

HARDEN: desktop ingest retry + fetch timeouts + heartbeat backoff#160
erikolsson wants to merge 3 commits into
mainfrom
harden/01-desktop-ingest-reliability

Conversation

@erikolsson

@erikolsson erikolsson commented Apr 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Hardens the desktop ingest pipeline against the two failure modes that today silently degrade or lose data:

  1. Stalled fetches — bare fetch() could hang forever, pinning one of the uploader's 16 concurrent slots. Every fetch in apps/desktop/src/main/{backend,agentIngest}.ts is now wrapped in an AbortController with an explicit timeout (20s default JSON RPC, 45s ingest NDJSON, 10s refresh, 15s agent ingest). Caller-supplied signals are honored alongside the timeout, and the timer is cleared on the fast path so it doesn't pin the event loop.

  2. Discarded ingest chunks on transient failures/v1/ingest is now wrapped in bounded exponential backoff (4 attempts, 0.5s → 4s, capped). The server already dedups on (session, lineSeq, prefixHash) so retrying a 5xx or network blip is idempotent. 4xx (except 408/429) is treated as permanent and bubbles immediately so the uploader doesn't burn its budget against a permanent error.

  3. Heartbeat hammering a down serversetInterval(15s) is replaced with a self-rescheduling timer that exponentially backs off (15s → 5min cap) when every send in a pulse fails, and resets the moment a pulse succeeds (or partially succeeds).

The new primitives live in apps/desktop/src/main/httpRetry.ts (~90 lines) with 11 unit tests covering retry classification, backoff bounds, AbortSignal coordination, and pre-aborted-signal handling.

No behavior change for the happy path. No new external dependencies. No new product surface — pure hardening.

Test plan

  • bun run typecheck (apps/desktop) — clean
  • bun test (apps/desktop) — 80 pass, 0 fail
  • bun run lint (apps/desktop) — clean (one pre-existing warning in unrelated renderer file)
  • Manual: induce a 503 storm on /v1/ingest and confirm the desktop retries within budget, then surfaces a [http] warn line per attempt and gives up cleanly.
  • Manual: SIGSTOP the server, watch [heartbeat] send failed lines start backing off cadence beyond 15s, then resume the server and confirm cadence returns to 15s.
  • Manual: block the API at a firewall and confirm fetches reject within ~20–45s instead of hanging.

🤖 Generated with Claude Code


Note

Medium Risk
Touches core desktop-to-backend networking for ingest and heartbeat, so mis-tuned timeouts/retry classification could cause delayed uploads or increased request load during outages. Changes are bounded and tested but affect reliability-critical paths.

Overview
Hardens the desktop’s backend communication paths by adding shared HTTP timeout and retry utilities and wiring them into ingest, agent session ingest, and auth refresh.

All relevant fetch calls in backend.ts and agentIngest.ts now use fetchWithTimeout with endpoint-specific budgets, and /v1/ingest uploads gain bounded exponential retry via withRetry, treating 5xx/408/429 as transient and other 4xx as permanent.

The heartbeat loop replaces a fixed setInterval with a self-rescheduling timer that exponentially backs off (capped) when pulses fully fail and resets on success/partial success, reducing retry storms during backend outages. A new httpRetry.ts module plus unit tests cover timeout/AbortSignal behavior and retry semantics.

Reviewed by Cursor Bugbot for commit 850edc7. Bugbot is set up for automated code reviews on this repo. Configure here.

Summary by CodeRabbit

  • New Features

    • Enforced network timeouts per operation and added automatic retry with exponential backoff for improved request reliability and clearer error classification.
    • Heartbeat scheduler now uses adaptive backoff and better failure tracking to reduce unnecessary traffic and recover gracefully.
  • Tests

    • Added tests covering timeout and retry behaviors, transient vs permanent error handling, and abort scenarios.

- Wrap every desktop fetch in an AbortController-driven timeout so a stalled
  TCP connection can't pin an uploader slot indefinitely (max 16). Budgets:
  20s default JSON RPC, 45s ingest NDJSON, 10s refresh, 15s agent ingest.
- Wrap /v1/ingest in bounded exponential backoff (4 attempts, 0.5s→4s).
  Server already dedups on (session, lineSeq, prefixHash), so retrying a
  transient 5xx or network blip is idempotent. 4xx (except 408/429) is
  treated as permanent and bubbles immediately.
- Heartbeat: replace the 15s setInterval with a self-rescheduling timer that
  exponentially backs off (15s → 5min cap) when every send in a pulse fails,
  and resets on the next mixed/successful pulse.

New apps/desktop/src/main/httpRetry.ts holds the primitives + 11 unit tests.

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

coderabbitai Bot commented Apr 28, 2026

Copy link
Copy Markdown
ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Free

Run ID: 7f525f15-0aa7-4f2d-8e01-a8b9b722da77

📥 Commits

Reviewing files that changed from the base of the PR and between 2969a96 and 850edc7.

📒 Files selected for processing (1)
  • apps/desktop/src/main/heartbeat.ts

Walkthrough

This PR adds a new HTTP utilities module with timeout and retry primitives (fetchWithTimeout, withRetry, TransientHttpError, PermanentHttpError, and isTransientStatus) and updates several callers to use them. agentIngest.ts wraps session upsert and list requests with fetchWithTimeout (15s). backend.ts applies per-endpoint timeouts, refactors /v1/ingest to use withRetry with per-attempt timeouts and maps non-OK responses to transient or permanent errors. heartbeat.ts replaces fixed-interval heartbeats with a self-rescheduling loop using exponential backoff and adjusts consecutive-failure accounting. New tests exercise timeout and retry behaviors.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Additional notes

Changes introduce new error types and retry semantics that affect response classification and retry surfaces; verify error-class mapping, retry/backoff parameters, and heartbeat failure-count semantics against expected production behavior.


Note

🎁 Summarized by CodeRabbit Free

Your organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login.

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

Comment thread apps/desktop/src/main/heartbeat.ts Outdated
Cursor Bot on PR 160: the previous order — `scheduleNextPulse()` then
`await pulse()` — captured `consecutiveFailures` while still 0, so a
failed initial pulse couldn't bump the cadence beyond 15s for the first
follow-up. Reorder so the first scheduled timer reads the post-pulse
counter.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment thread apps/desktop/src/main/heartbeat.ts Outdated
…dic timer

Cursor Bot on PR 160 follow-up: the previous fix moved scheduleNextPulse()
to run after `await pulse()` so the first delay reflects the initial
pulse's outcome. But if the initial pulse throws (enumerateLive can
throw on a transient fs.readdir failure), scheduleNextPulse() is never
reached and the heartbeat permanently degrades to file-watcher-only
mode — start() is `void`-called from index.ts, so the rejection is
silently swallowed.

Wrap in try/finally so the schedule survives a thrown initial pulse
while still reading the post-pulse counter.

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

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 850edc7. Configure here.

consecutiveFailures++;
} else if (attempted > 0) {
consecutiveFailures = 0;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Watcher-triggered pulse doesn't reschedule backed-off periodic timer

Low Severity

When a file-watcher event triggers schedulePulse()pulse() and that pulse succeeds, consecutiveFailures is reset to 0 — but scheduleNextPulse() is never called, so the already-scheduled periodic timer keeps its backed-off delay (up to 5 minutes). The periodic heartbeat cadence won't recover to 15s until that stale timer finally fires. This contradicts the stated intent that cadence "resets the moment a pulse succeeds."

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 850edc7. Configure here.

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