From e4e07f54a815f187029ab67f0fbf5241851ffb0a Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 10 Sep 2026 17:44:21 +0000 Subject: [PATCH 1/3] feat(client): add retryExchange and a dev-only devtoolsExchange MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A single network blip was fatal: the operation failed, a toast appeared, and nothing recovered short of the user navigating again. `retryExchange` now retries on `error.networkError` only — never on GraphQL errors, which are answers rather than failures and will not change on a second attempt. Mutations are excluded by `retryExchange`'s default and stay that way; none of ours are idempotent. Placement matters more than it looks. `retryExchange` sits after `authExchange` and immediately before `fetchExchange`, so auth observes a single settled result rather than each attempt and a retry can never drive `didAuthError` or `refreshAuth`. Two tests pin the ordering and the `retryIf` predicate. `devtoolsExchange` goes first in the chain behind `import.meta.env.DEV`, so the urql browser devtools finally attach in development. A production build confirms it is tree-shaken out and that `retryExchange` is not. The chain also gains a comment marking where `cacheExchange` belongs — between the error handler and auth — for when normalized caching lands. Nothing occupies that slot today: passing an explicit `exchanges` array means urql installs no cache of its own. Extending `urql-client.test.ts` needed two things worth knowing: its `vi.mock` of `urql` is wholesale, so the new modules need their own mocks or the real ones load into the mocked graph; and the mock factories needed real call signatures, without which `.mock.calls[0][0]` types as an empty tuple and `tsc` rejects it while vitest passes. Step 8b of the urql quick-wins sequence, and the last of it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01TbrGL3NndzRiEwJkwHxnbm --- .../urql-retry-and-devtools-exchanges.md | 24 +++++++++ packages/client/package.json | 2 + .../client/src/__tests__/urql-client.test.ts | 52 ++++++++++++++++++- packages/client/src/providers/urql.tsx | 16 ++++++ yarn.lock | 28 +++++++++- 5 files changed, 119 insertions(+), 3 deletions(-) create mode 100644 .changeset/urql-retry-and-devtools-exchanges.md diff --git a/.changeset/urql-retry-and-devtools-exchanges.md b/.changeset/urql-retry-and-devtools-exchanges.md new file mode 100644 index 000000000..7060f14ae --- /dev/null +++ b/.changeset/urql-retry-and-devtools-exchanges.md @@ -0,0 +1,24 @@ +--- +'@accounter/client': patch +--- + +Add `retryExchange` for network failures, and the urql devtools exchange in development. + +A single network blip was fatal: the operation failed, a toast appeared, and nothing recovered short +of the user navigating again. `retryExchange` now retries on `error.networkError` only — never on +GraphQL errors, which are answers rather than failures and will not change on a second attempt. +Mutations are excluded by `retryExchange`'s default and stay that way, because none of ours are +idempotent. + +Placement matters more than it looks. `retryExchange` sits **after** `authExchange` and immediately +before `fetchExchange`, so `authExchange` observes a single settled result rather than each attempt +and a retry can never drive `didAuthError`/`refreshAuth`. Two tests pin the ordering and the +`retryIf` predicate. + +`devtoolsExchange` is added first in the chain, behind `import.meta.env.DEV`, so the urql browser +devtools finally attach during development. A production build was checked to confirm it is +tree-shaken out. + +The chain also gains a comment marking where `cacheExchange` belongs — between the error handler and +auth — for when normalized caching lands. Nothing occupies that slot today: passing an explicit +`exchanges` array means urql installs no cache of its own. diff --git a/packages/client/package.json b/packages/client/package.json index eb149888e..46b109acc 100644 --- a/packages/client/package.json +++ b/packages/client/package.json @@ -41,6 +41,7 @@ "@mui/x-charts": "8.29.3", "@tanstack/react-table": "9.2.4", "@urql/exchange-auth": "3.0.0", + "@urql/exchange-retry": "2.0.0", "chart.js": "4.5.1", "class-variance-authority": "0.7.1", "clsx": "2.1.1", @@ -87,6 +88,7 @@ "@types/deep-equal": "1.0.4", "@types/react": "19.3.0", "@types/react-dom": "19.3.0", + "@urql/devtools": "2.0.3", "@vitejs/plugin-react": "6.1.1", "autoprefixer": "10.5.5", "happy-dom": "20.14.3", diff --git a/packages/client/src/__tests__/urql-client.test.ts b/packages/client/src/__tests__/urql-client.test.ts index c0eaca6e9..807dfbef9 100644 --- a/packages/client/src/__tests__/urql-client.test.ts +++ b/packages/client/src/__tests__/urql-client.test.ts @@ -1,7 +1,13 @@ import { ROUTES } from '../router/routes.js'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -const { createClientMock, mapExchangeMock, authExchangeMock, appendHeadersMock } = vi.hoisted(() => { +const { + createClientMock, + mapExchangeMock, + authExchangeMock, + appendHeadersMock, + retryExchangeMock, +} = vi.hoisted(() => { const appendHeaders = vi.fn((operation: any, headers: Record) => ({ ...operation, context: { @@ -17,10 +23,17 @@ const { createClientMock, mapExchangeMock, authExchangeMock, appendHeadersMock } }, })); return { - createClientMock: vi.fn(() => ({ mockClient: true })), + createClientMock: vi.fn((_options: { url: string; exchanges: unknown[] }) => ({ + mockClient: true, + })), mapExchangeMock: vi.fn(() => ({ mockMapExchange: true })), authExchangeMock: vi.fn(), appendHeadersMock: appendHeaders, + retryExchangeMock: vi.fn( + (_options: { + retryIf: (error: { networkError?: unknown; graphQLErrors?: unknown[] }) => boolean; + }) => ({ mockRetryExchange: true }), + ), }; }); @@ -49,6 +62,14 @@ vi.mock('urql', () => ({ Provider: ({ children }: { children?: unknown }) => children, })); +vi.mock('@urql/exchange-retry', () => ({ + retryExchange: retryExchangeMock, +})); + +vi.mock('@urql/devtools', () => ({ + devtoolsExchange: { mockDevtoolsExchange: true }, +})); + vi.mock('@urql/exchange-auth', () => ({ authExchange: authExchangeMock.mockImplementation( ( @@ -365,6 +386,33 @@ describe('URQL auth exchange hardening', () => { ); }); + it('places retryExchange after authExchange and before fetchExchange', async () => { + // Order is the point: authExchange must see one settled result rather than + // each retry attempt, so retries cannot interact with didAuthError / + // refreshAuth. fetchExchange stays last. + await initializeAuth(async () => 'token-123'); + + const { exchanges } = createClientMock.mock.calls[0][0]; + const authIndex = exchanges.findIndex((e: unknown) => e === authExchangeMock.mock.results[0].value); + const retryIndex = exchanges.findIndex((e: unknown) => e === retryExchangeMock.mock.results[0].value); + const fetchIndex = exchanges.findIndex( + (e: unknown) => (e as { mockFetchExchange?: boolean })?.mockFetchExchange === true, + ); + + expect(authIndex).toBeGreaterThanOrEqual(0); + expect(retryIndex).toBeGreaterThan(authIndex); + expect(fetchIndex).toBeGreaterThan(retryIndex); + }); + + it('retries network errors but never GraphQL errors', async () => { + await initializeAuth(async () => 'token-123'); + + const { retryIf } = retryExchangeMock.mock.calls[0][0]; + + expect(retryIf({ networkError: new Error('offline') })).toBe(true); + expect(retryIf({ graphQLErrors: [{ message: 'nope' }] })).toBe(false); + }); + it('uses VITE_GRAPHQL_URL for the client endpoint when set', async () => { vi.stubEnv('VITE_GRAPHQL_URL', 'https://example.test/graphql'); diff --git a/packages/client/src/providers/urql.tsx b/packages/client/src/providers/urql.tsx index 582eac512..66802b830 100644 --- a/packages/client/src/providers/urql.tsx +++ b/packages/client/src/providers/urql.tsx @@ -9,7 +9,9 @@ import { type Operation, type OperationContext, } from 'urql'; +import { devtoolsExchange } from '@urql/devtools'; import { authExchange } from '@urql/exchange-auth'; +import { retryExchange } from '@urql/exchange-retry'; import { requestInteractiveReauth } from '../lib/reauth-coordinator.js'; import { ROUTES } from '../router/routes.js'; import { handleUrqlError } from './urql-error-handler.js'; @@ -240,11 +242,18 @@ export function getUrqlClient(): Client { globalClient = createClient({ url, exchanges: [ + // Dev only, and first so it observes every operation and result. Tree-shaken + // from production builds by the constant condition. + ...(import.meta.env.DEV ? [devtoolsExchange] : []), mapExchange({ onResult(result) { handleUrqlError(result); }, }), + // `cacheExchange` belongs here, between the error handler and auth, when + // normalized caching lands. Nothing occupies the slot today: passing an + // explicit `exchanges` array means urql installs no cache of its own. + authExchange(async utils => { if (!isDevAuthEnabled) { const initialToken = await getAccessToken(); @@ -323,6 +332,13 @@ export function getUrqlClient(): Client { }, }; }), + // After `authExchange`, deliberately: auth then sees a single settled result + // rather than every retry attempt, so a retry can never drive + // `didAuthError`/`refreshAuth`. Mutations are excluded by default and stay + // that way — none of ours are idempotent. + retryExchange({ + retryIf: error => !!error.networkError, + }), fetchExchange, ], }); diff --git a/yarn.lock b/yarn.lock index e50854209..524e9d54f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -88,7 +88,9 @@ __metadata: "@types/deep-equal": "npm:1.0.4" "@types/react": "npm:19.3.0" "@types/react-dom": "npm:19.3.0" + "@urql/devtools": "npm:2.0.3" "@urql/exchange-auth": "npm:3.0.0" + "@urql/exchange-retry": "npm:2.0.0" "@vitejs/plugin-react": "npm:6.1.1" autoprefixer: "npm:10.5.5" chart.js: "npm:4.5.1" @@ -11208,6 +11210,18 @@ __metadata: languageName: node linkType: hard +"@urql/devtools@npm:2.0.3": + version: 2.0.3 + resolution: "@urql/devtools@npm:2.0.3" + dependencies: + wonka: "npm:>= 4.0.9" + peerDependencies: + "@urql/core": ">= 1.14.0" + graphql: ">= 0.11.0" + checksum: 10c0/130d2f91b36b2586e68a90732831cf58dcc0765fc860a6e5b6c58ab5ab4b26856c1b5f92ef84539b4ad0348d6dd1bde209416ad2a9b4731387b064ab4d5ed704 + languageName: node + linkType: hard + "@urql/exchange-auth@npm:3.0.0": version: 3.0.0 resolution: "@urql/exchange-auth@npm:3.0.0" @@ -11220,6 +11234,18 @@ __metadata: languageName: node linkType: hard +"@urql/exchange-retry@npm:2.0.0": + version: 2.0.0 + resolution: "@urql/exchange-retry@npm:2.0.0" + dependencies: + "@urql/core": "npm:^6.0.0" + wonka: "npm:^6.3.2" + peerDependencies: + "@urql/core": ^6.0.0 + checksum: 10c0/37a4dbea6b158c73521fc58e33c2b73ad4568e221e3e2fce373a5cadb21b93f345b4a3e574646b2b1e3f598db4d6fcf62cb6d21d5abd11a3ca88d39c248894fd + languageName: node + linkType: hard + "@vercel/oidc@npm:3.2.0": version: 3.2.0 resolution: "@vercel/oidc@npm:3.2.0" @@ -26985,7 +27011,7 @@ __metadata: languageName: node linkType: hard -"wonka@npm:^6.3.2": +"wonka@npm:>= 4.0.9, wonka@npm:^6.3.2": version: 6.3.6 resolution: "wonka@npm:6.3.6" checksum: 10c0/a8887a7766cf9519b4f80b43842fe1b6575a0f5edf397c5a32c267185bd999af9d3c42d91d6d7cd86d3ec89fdc5f8909bb542004d184fcaad794d25e821ff70d From 601fd2155d44540c2e0f0c1297272698f85a0a32 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 12 Sep 2026 12:44:17 +0000 Subject: [PATCH 2/3] fix(client): never retry mutations on network errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I claimed in this PR that `@urql/exchange-retry` excludes mutations by default. It does not. Checking the source, the retry gate is `retryIf`'s return value alone: if (!res.error || (retryIf ? !retryIf(res.error, res.operation) : ...)) There is no `operation.kind` check anywhere in that decision — the one `kind` reference in the exchange is an unrelated teardown filter. So `retryIf: error => !!error.networkError` would have resubmitted any mutation that failed mid-flight, and none of ours are idempotent: a create or delete that timed out after the server had already applied it would have been sent twice. `retryIf` now takes the operation it is already handed and guards on `operation.kind !== 'mutation'`. Queries and subscriptions are unaffected. A test covers the mutation case directly, and the comment at the call site says why the guard exists so it is not mistaken for redundancy with a library default that does not exist. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01TbrGL3NndzRiEwJkwHxnbm --- .../urql-retry-and-devtools-exchanges.md | 7 +++++-- .../client/src/__tests__/urql-client.test.ts | 20 ++++++++++++++++--- packages/client/src/providers/urql.tsx | 12 ++++++++--- 3 files changed, 31 insertions(+), 8 deletions(-) diff --git a/.changeset/urql-retry-and-devtools-exchanges.md b/.changeset/urql-retry-and-devtools-exchanges.md index 7060f14ae..e92bc89a7 100644 --- a/.changeset/urql-retry-and-devtools-exchanges.md +++ b/.changeset/urql-retry-and-devtools-exchanges.md @@ -7,8 +7,11 @@ Add `retryExchange` for network failures, and the urql devtools exchange in deve A single network blip was fatal: the operation failed, a toast appeared, and nothing recovered short of the user navigating again. `retryExchange` now retries on `error.networkError` only — never on GraphQL errors, which are answers rather than failures and will not change on a second attempt. -Mutations are excluded by `retryExchange`'s default and stay that way, because none of ours are -idempotent. + +Mutations are never retried. That guard is explicit and load-bearing: +`@urql/exchange-retry` does not exclude mutations on its own, and `retryIf`'s return value is the +whole decision, so without `operation.kind !== 'mutation'` a write that failed mid-flight would be +resubmitted. None of our mutations are idempotent. Placement matters more than it looks. `retryExchange` sits **after** `authExchange` and immediately before `fetchExchange`, so `authExchange` observes a single settled result rather than each attempt diff --git a/packages/client/src/__tests__/urql-client.test.ts b/packages/client/src/__tests__/urql-client.test.ts index 807dfbef9..f467cffa9 100644 --- a/packages/client/src/__tests__/urql-client.test.ts +++ b/packages/client/src/__tests__/urql-client.test.ts @@ -31,7 +31,10 @@ const { appendHeadersMock: appendHeaders, retryExchangeMock: vi.fn( (_options: { - retryIf: (error: { networkError?: unknown; graphQLErrors?: unknown[] }) => boolean; + retryIf: ( + error: { networkError?: unknown; graphQLErrors?: unknown[] }, + operation: { kind: string }, + ) => boolean; }) => ({ mockRetryExchange: true }), ), }; @@ -409,8 +412,19 @@ describe('URQL auth exchange hardening', () => { const { retryIf } = retryExchangeMock.mock.calls[0][0]; - expect(retryIf({ networkError: new Error('offline') })).toBe(true); - expect(retryIf({ graphQLErrors: [{ message: 'nope' }] })).toBe(false); + expect(retryIf({ networkError: new Error('offline') }, { kind: 'query' })).toBe(true); + expect(retryIf({ graphQLErrors: [{ message: 'nope' }] }, { kind: 'query' })).toBe(false); + }); + + it('never retries a mutation, even on a network error', async () => { + // `@urql/exchange-retry` does NOT exclude mutations on its own — `retryIf` + // alone decides. Without this guard a write that failed mid-flight would be + // resubmitted, and none of our mutations are idempotent. + await initializeAuth(async () => 'token-123'); + + const { retryIf } = retryExchangeMock.mock.calls[0][0]; + + expect(retryIf({ networkError: new Error('offline') }, { kind: 'mutation' })).toBe(false); }); it('uses VITE_GRAPHQL_URL for the client endpoint when set', async () => { diff --git a/packages/client/src/providers/urql.tsx b/packages/client/src/providers/urql.tsx index 66802b830..f48c221bb 100644 --- a/packages/client/src/providers/urql.tsx +++ b/packages/client/src/providers/urql.tsx @@ -334,10 +334,16 @@ export function getUrqlClient(): Client { }), // After `authExchange`, deliberately: auth then sees a single settled result // rather than every retry attempt, so a retry can never drive - // `didAuthError`/`refreshAuth`. Mutations are excluded by default and stay - // that way — none of ours are idempotent. + // `didAuthError`/`refreshAuth`. + // + // The mutation guard is not belt-and-braces. `@urql/exchange-retry` does + // not exclude mutations on its own — `retryIf`'s return value is the whole + // decision — so without it a write that failed mid-flight would be + // resubmitted, and none of ours are idempotent. Queries and subscriptions + // retry on network errors only; a GraphQL error is an answer, not a + // failure, and will not change on a second attempt. retryExchange({ - retryIf: error => !!error.networkError, + retryIf: (error, operation) => operation.kind !== 'mutation' && !!error.networkError, }), fetchExchange, ], From f73c2a641c2efd4fb2aa0ea44d4424d0c9d8816d Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Thu, 10 Sep 2026 17:47:22 +0000 Subject: [PATCH 3/3] chore(dependencies): updated changesets for modified dependencies --- .changeset/@accounter_client-4450-dependencies.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/@accounter_client-4450-dependencies.md diff --git a/.changeset/@accounter_client-4450-dependencies.md b/.changeset/@accounter_client-4450-dependencies.md new file mode 100644 index 000000000..8ef8201b6 --- /dev/null +++ b/.changeset/@accounter_client-4450-dependencies.md @@ -0,0 +1,5 @@ +--- +"@accounter/client": patch +--- +dependencies updates: + - Added dependency [`@urql/exchange-retry@2.0.0` ↗︎](https://www.npmjs.com/package/@urql/exchange-retry/v/2.0.0) (to `dependencies`)