Skip to content

Commit bc174ef

Browse files
authored
fix: retry backoff for API calls and StatCard error state (#1568)
* fix: skip retry for client errors and add error state UI to StatCard Query client now classifies gRPC status codes into retryable (server errors, network failures) and non-retryable (client errors like InvalidArgument, NotFound, PermissionDenied). Client errors are never retried since they will not succeed. Server errors use exponential backoff capped at 3 attempts. StatCard displays an error icon with "Failed to load" message and an optional retry button instead of the previous uninformative dash. * test: align query-client retry count assertion with 3-attempt backoff The shouldRetry implementation uses failureCount < 3, allowing up to 3 retries. The existing integration test expected only 2 retries (< 2). Update to match the intended 3-attempt backoff behavior. * fix: skip link wrapper on StatCard error state with onRetry When both href and onRetry are set, the retry button was nested inside a Link element creating invalid nested interactive controls. Skip the link wrapper when rendering the error+retry state so the button receives click events correctly. --------- Co-authored-by: Ben Coombs <bjcoombs@users.noreply.github.com>
1 parent 7ecb38d commit bc174ef

7 files changed

Lines changed: 170 additions & 25 deletions

File tree

frontend/src/features/dashboard/pages/index.test.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -210,8 +210,8 @@ describe('DashboardPage', () => {
210210
expect(screen.getByText('7')).toBeInTheDocument()
211211
})
212212
expect(screen.getByText('3')).toBeInTheDocument()
213-
// Error state shows dash
214-
expect(screen.getByText('')).toBeInTheDocument()
213+
// Error state shows "Failed to load" with retry button
214+
expect(screen.getByText('Failed to load')).toBeInTheDocument()
215215
})
216216

217217
it('renders recent activity section', () => {

frontend/src/features/dashboard/pages/index.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,7 @@ export function DashboardPage() {
165165
value={paymentsCount?.count}
166166
isLoading={paymentsQuery.isLoading}
167167
error={paymentsQuery.isError}
168+
onRetry={() => void paymentsQuery.refetch()}
168169
showRecentQualifier={!!(paymentsCount?.isEstimate && !paymentsQuery.isLoading && !paymentsQuery.isError)}
169170
description="Active payment orders"
170171
icon={<CreditCard className="h-4 w-4" />}
@@ -175,6 +176,7 @@ export function DashboardPage() {
175176
value={bookingLogsCount?.count}
176177
isLoading={bookingLogsQuery.isLoading}
177178
error={bookingLogsQuery.isError}
179+
onRetry={() => void bookingLogsQuery.refetch()}
178180
showRecentQualifier={!!(bookingLogsCount?.isEstimate && !bookingLogsQuery.isLoading && !bookingLogsQuery.isError)}
179181
description="Financial booking logs"
180182
icon={<FileText className="h-4 w-4" />}
@@ -185,6 +187,7 @@ export function DashboardPage() {
185187
value={ledgerPostingsCount?.count}
186188
isLoading={ledgerPostingsQuery.isLoading}
187189
error={ledgerPostingsQuery.isError}
190+
onRetry={() => void ledgerPostingsQuery.refetch()}
188191
showRecentQualifier={!!(ledgerPostingsCount?.isEstimate && !ledgerPostingsQuery.isLoading && !ledgerPostingsQuery.isError)}
189192
description="Double-entry postings"
190193
icon={<BarChart3 className="h-4 w-4" />}

frontend/src/features/dashboard/pages/stat-card.test.tsx

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1-
import { describe, it, expect } from 'vitest'
1+
import { describe, it, expect, vi } from 'vitest'
22
import { render, screen } from '@testing-library/react'
3+
import userEvent from '@testing-library/user-event'
34
import { StatCard } from './stat-card'
45

56
describe('StatCard', () => {
@@ -42,10 +43,39 @@ describe('StatCard', () => {
4243
expect(screen.getByText('Active accounts')).toBeInTheDocument()
4344
})
4445

45-
it('renders error state when error is true', () => {
46+
it('renders error state with icon and message', () => {
4647
render(<StatCard title="Accounts" error />)
4748

48-
expect(screen.getByText('—')).toBeInTheDocument()
49+
expect(screen.getByTestId('stat-card-error')).toBeInTheDocument()
50+
expect(screen.getByText('Failed to load')).toBeInTheDocument()
51+
})
52+
53+
it('renders retry button in error state when onRetry is provided', () => {
54+
const onRetry = vi.fn()
55+
render(<StatCard title="Accounts" error onRetry={onRetry} />)
56+
57+
expect(screen.getByRole('button', { name: /retry accounts/i })).toBeInTheDocument()
58+
})
59+
60+
it('calls onRetry when retry button is clicked', async () => {
61+
const user = userEvent.setup()
62+
const onRetry = vi.fn()
63+
render(<StatCard title="Accounts" error onRetry={onRetry} />)
64+
65+
await user.click(screen.getByRole('button', { name: /retry accounts/i }))
66+
expect(onRetry).toHaveBeenCalledOnce()
67+
})
68+
69+
it('does not render retry button when onRetry is not provided', () => {
70+
render(<StatCard title="Accounts" error />)
71+
72+
expect(screen.queryByRole('button')).not.toBeInTheDocument()
73+
})
74+
75+
it('hides description when in error state', () => {
76+
render(<StatCard title="Accounts" error description="Active accounts" />)
77+
78+
expect(screen.queryByText('Active accounts')).not.toBeInTheDocument()
4979
})
5080

5181
it('renders icon when provided', () => {

frontend/src/features/dashboard/pages/stat-card.tsx

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import { type ReactNode } from 'react'
22
import { Link } from 'react-router-dom'
3+
import { AlertCircle, RefreshCw } from 'lucide-react'
34
import { Card, CardContent, CardHeader, CardTitle } from '@/components/ui/card'
5+
import { Button } from '@/components/ui/button'
46
import { cn } from '@/lib/utils'
57

68
interface StatCardProps {
@@ -10,6 +12,7 @@ interface StatCardProps {
1012
icon?: ReactNode
1113
isLoading?: boolean
1214
error?: boolean
15+
onRetry?: () => void
1316
showRecentQualifier?: boolean
1417
className?: string
1518
href?: string
@@ -22,6 +25,7 @@ export function StatCard({
2225
icon,
2326
isLoading,
2427
error,
28+
onRetry,
2529
showRecentQualifier,
2630
className,
2731
href,
@@ -38,28 +42,41 @@ export function StatCard({
3842
data-testid="stat-card-skeleton"
3943
className="h-8 w-24 animate-pulse rounded bg-muted"
4044
/>
45+
) : error ? (
46+
<div data-testid="stat-card-error" className="flex items-center gap-2">
47+
<AlertCircle className="h-4 w-4 text-destructive" />
48+
<span className="text-sm text-muted-foreground">Failed to load</span>
49+
{onRetry && (
50+
<Button
51+
variant="ghost"
52+
size="sm"
53+
className="ml-auto h-7 px-2"
54+
onClick={(e) => {
55+
e.preventDefault()
56+
onRetry()
57+
}}
58+
>
59+
<RefreshCw className="h-3 w-3" />
60+
<span className="sr-only">Retry {title}</span>
61+
</Button>
62+
)}
63+
</div>
4164
) : (
4265
<div className="text-2xl font-bold">
43-
{error ? (
44-
<span className="text-muted-foreground"></span>
45-
) : (
46-
<>
47-
{value ?? 0}
48-
{showRecentQualifier && (
49-
<span className="ml-1 text-xs font-normal text-muted-foreground">recent</span>
50-
)}
51-
</>
66+
{value ?? 0}
67+
{showRecentQualifier && (
68+
<span className="ml-1 text-xs font-normal text-muted-foreground">recent</span>
5269
)}
5370
</div>
5471
)}
55-
{description && (
72+
{description && !error && (
5673
<p className="mt-1 text-xs text-muted-foreground">{description}</p>
5774
)}
5875
</CardContent>
5976
</>
6077
)
6178

62-
if (href) {
79+
if (href && !(error && onRetry)) {
6380
return (
6481
<Link to={href} className="block">
6582
<Card
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
import { describe, it, expect } from 'vitest'
2+
import { ConnectError, Code } from '@connectrpc/connect'
3+
import { shouldRetry, retryDelay } from '../query-client'
4+
5+
describe('shouldRetry', () => {
6+
const nonRetryableCodes = [
7+
Code.InvalidArgument,
8+
Code.NotFound,
9+
Code.AlreadyExists,
10+
Code.PermissionDenied,
11+
Code.Unauthenticated,
12+
Code.FailedPrecondition,
13+
Code.OutOfRange,
14+
Code.Unimplemented,
15+
]
16+
17+
it.each(nonRetryableCodes)(
18+
'does not retry ConnectError with code %s',
19+
(code) => {
20+
const error = new ConnectError('client error', code)
21+
expect(shouldRetry(0, error)).toBe(false)
22+
},
23+
)
24+
25+
it('retries server errors (Internal)', () => {
26+
const error = new ConnectError('server error', Code.Internal)
27+
expect(shouldRetry(0, error)).toBe(true)
28+
expect(shouldRetry(1, error)).toBe(true)
29+
expect(shouldRetry(2, error)).toBe(true)
30+
})
31+
32+
it('retries Unavailable errors', () => {
33+
const error = new ConnectError('unavailable', Code.Unavailable)
34+
expect(shouldRetry(0, error)).toBe(true)
35+
})
36+
37+
it('stops retrying after 3 failures for server errors', () => {
38+
const error = new ConnectError('server error', Code.Internal)
39+
expect(shouldRetry(3, error)).toBe(false)
40+
})
41+
42+
it('retries generic network errors', () => {
43+
const error = new Error('Failed to fetch')
44+
expect(shouldRetry(0, error)).toBe(true)
45+
expect(shouldRetry(2, error)).toBe(true)
46+
})
47+
48+
it('stops retrying generic errors after 3 failures', () => {
49+
const error = new Error('Failed to fetch')
50+
expect(shouldRetry(3, error)).toBe(false)
51+
})
52+
})
53+
54+
describe('retryDelay', () => {
55+
it('uses exponential backoff', () => {
56+
expect(retryDelay(0)).toBe(1000)
57+
expect(retryDelay(1)).toBe(2000)
58+
expect(retryDelay(2)).toBe(4000)
59+
})
60+
61+
it('caps at 10 seconds', () => {
62+
expect(retryDelay(5)).toBe(10_000)
63+
expect(retryDelay(10)).toBe(10_000)
64+
})
65+
})

frontend/src/lib/query-client.ts

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,47 @@
11
import { QueryClient } from '@tanstack/react-query'
22
import { ConnectError, Code } from '@connectrpc/connect'
33

4+
/**
5+
* gRPC/Connect status codes that correspond to client errors (4xx equivalent).
6+
* These will not succeed on retry, so we skip them immediately.
7+
*/
8+
const NON_RETRYABLE_CODES: ReadonlySet<Code> = new Set([
9+
Code.InvalidArgument,
10+
Code.NotFound,
11+
Code.AlreadyExists,
12+
Code.PermissionDenied,
13+
Code.Unauthenticated,
14+
Code.FailedPrecondition,
15+
Code.OutOfRange,
16+
Code.Unimplemented,
17+
])
18+
19+
/**
20+
* Determines whether a failed query should be retried.
21+
* - Client errors (4xx-equivalent gRPC codes) are never retried.
22+
* - Server errors (5xx) and network failures retry up to 3 attempts.
23+
*/
24+
export function shouldRetry(failureCount: number, error: Error): boolean {
25+
if (error instanceof ConnectError && NON_RETRYABLE_CODES.has(error.code)) {
26+
return false
27+
}
28+
return failureCount < 3
29+
}
30+
31+
/**
32+
* Exponential backoff: 1s, 2s, 4s... capped at 10s.
33+
*/
34+
export function retryDelay(attempt: number): number {
35+
return Math.min(1000 * 2 ** attempt, 10_000)
36+
}
37+
438
export const queryClient = new QueryClient({
539
defaultOptions: {
640
queries: {
741
staleTime: 30_000,
842
gcTime: 5 * 60 * 1000,
9-
retry: (failureCount, error) => {
10-
if (error instanceof ConnectError && error.code === Code.Unauthenticated) {
11-
return false
12-
}
13-
return failureCount < 2
14-
},
15-
retryDelay: (attempt) => Math.min(1000 * 2 ** attempt, 10_000),
43+
retry: shouldRetry,
44+
retryDelay,
1645
refetchOnWindowFocus: true,
1746
refetchOnReconnect: true,
1847
},

frontend/src/test/query-client.test.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,11 @@ describe('queryClient', () => {
2222
const options = queryClient.getDefaultOptions()
2323
const retry = options.queries?.retry as (failureCount: number, error: Error) => boolean
2424
expect(typeof retry).toBe('function')
25-
// Regular errors retry up to 2 times
25+
// Regular errors retry up to 3 times
2626
expect(retry(0, new Error('network'))).toBe(true)
2727
expect(retry(1, new Error('network'))).toBe(true)
28-
expect(retry(2, new Error('network'))).toBe(false)
28+
expect(retry(2, new Error('network'))).toBe(true)
29+
expect(retry(3, new Error('network'))).toBe(false)
2930
// Unauthenticated errors should never retry
3031
expect(retry(0, new ConnectError('unauth', Code.Unauthenticated))).toBe(false)
3132
})

0 commit comments

Comments
 (0)