Repository navigation
fix(transport): classify fetch TypeErrors as network errors - #208
Conversation
…owsers Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The classification change is minimal, correct, comprehensively unit-tested, and documented in the changelog with no decoupling, export-wiring, or publishing convention violations.
Review effort: Balanced
Findings: None
What changed in this PR
This PR adjusts createNetworkOrBackendError in app-kit's transport layer so that any TypeError thrown by fetch (except timeout cases) is classified as a network error (NETWORK_FAILED_TO_FETCH) rather than being attributed to the backend. The motivation is that a client can't distinguish a dropped keep-alive connection from a down backend, and previously such errors surfaced the "Service temporarily unavailable" internal-error popup (and weren't recognized at all on Safari/Firefox). This follows the GET/HEAD retry work from 9.3.1, ensuring the correct network popup appears across all browsers once retries are exhausted. As shared plumbing, this affects error-popup UX in both consuming frontends.
Changes:
- Collapse the message-specific
TypeErrorbranches so every non-timeoutTypeErrormaps toNETWORK_FAILED_TO_FETCH, independent of browser message or whether the URL is the backend. - Replace the per-message tests with a table over the four known browser/RN messages plus an unknown-message case, each asserted for backend and non-backend URLs.
- Add a
CHANGELOG.md[Unreleased]→ Fixed entry documenting the behavior change.
| File | Description |
|---|---|
| packages/app-kit/src/transport/app-errors/app-error-factory.ts | Maps all non-timeout TypeErrors to NETWORK_FAILED_TO_FETCH; adds explanatory comment. |
| packages/app-kit/src/transport/app-errors/app-error-factory.test.ts | Replaces message-specific assertions with a table covering the four browser messages plus an unknown message, for backend and non-backend URLs; removes the obsolete fall-through test. |
| CHANGELOG.md | Documents the transport error-classification fix under [Unreleased] → Fixed. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Summary
When the browser's connection drops (e.g.
ERR_CONNECTION_CLOSEDon a stale keep-alive to Cloudflare), the frontends showed the internal-error popup ("Service temporarily unavailable") instead of the network one, and on Safari/Firefox the error wasn't recognised at all. This follows up the GET/HEAD retry from 9.3.1: once retries are exhausted, the user now gets the right popup in every browser.Changes
createNetworkOrBackendError: anyTypeErrorthrown byfetchmaps toNETWORK_FAILED_TO_FETCH, regardless of the browser's message (Chrome "Failed to fetch", Safari "Load failed", Firefox "NetworkError when attempting to fetch resource.", RN "Network request failed") and regardless of whether the URL is the backend. A client can't distinguish a dropped connection from a down backend, so it no longer blames the backend (BACKEND_UNAVAILABLE).TypeErrors whose message containstimeoutkeep their timeout codes.[Unreleased]→ Fixed.Known trade-off:
TypeErrors thatfetchraises for a malformed request (e.g. invalid header value) are now also shown as a network error rather than an internal error.Test plan
pnpm lint && pnpm build && pnpm test— all test files pass🤖 Generated with Claude Code