Skip to content

fix(transport): retry idempotent requests on network failures - #205

Merged
Andreas-Garcia merged 1 commit into
developfrom
feature/retry-idempotent-network-errors
Oct 6, 2026
Merged

Andreas-Garcia merged 1 commit into
developfrom
feature/retry-idempotent-network-errors

Conversation

@Andreas-Garcia

Copy link
Copy Markdown
Member

A dropped browser↔Cloudflare connection (ERR_CONNECTION_CLOSED on grow staging) surfaced straight to the connectivity popup: React Query retry can't fire since fetchWrapper swallows the error via handleError.

fetchWrapper now retries GET/HEAD up to twice (300ms, 900ms) only when fetch throws. Error responses, aborted signals, and other methods are never retried.

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 6, 2026 11:48
@Andreas-Garcia
Andreas-Garcia merged commit c2f18d8 into develop Oct 6, 2026
3 checks passed

Copilot AI 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.

Copilot review overview

🔵 Needs a closer look

It changes the retry/latency behavior of the core shared transport layer used by two production frontends, so a human should sign off on the operational trade-offs even though the implementation is clean and well-tested.

Review effort: Balanced
Findings: None

What changed in this PR

This PR hardens the shared transport layer so a dropped browser↔Cloudflare keep-alive connection (ERR_CONNECTION_CLOSED) no longer surfaces straight to the connectivity popup. Previously fetchWrapper swallowed the thrown fetch error via handleError, so React Query's own retry never got a chance to fire. The fix adds an internal, bounded retry around fetch for safe/idempotent methods only. This is a focused behavior change to @behindthemusictree/app-kit's transport module, which is consumed by both grow-the-music-tree-frontend and hear-the-music-tree-frontend.

Changes:

  • Added fetchWithNetworkRetry that retries GET/HEAD up to twice (300ms, then 900ms) only when fetch itself throws, skipping aborted signals, error responses, and non-idempotent methods.
  • Added tests covering recovery-after-retry, no-retry-for-POST/abort/error-response, and retry-exhaustion (3 total attempts), updating existing network-failure tests to drive fake timers.
  • Added a CHANGELOG.md entry under [Unreleased] → Fixed.
File Description
packages/​app-kit/​src/​transport/​fetch-wrapper.ts Introduces bounded network retry for idempotent methods around the fetch call.
packages/​app-kit/​src/​transport/​fetch-wrapper.test.ts Adds retry-behavior tests and adapts existing tests to fake timers.
CHANGELOG.md Documents the retry behavior under [Unreleased].

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@Andreas-Garcia
Andreas-Garcia deleted the feature/retry-idempotent-network-errors branch October 7, 2026 15:45
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.

2 participants