Skip to content

Commit 634b583

Browse files
committed
feat: refactor network status handling and add message retry system
- Separate auth and validation status tracking for graceful degradation - Add retry mechanism with exponential backoff (max 3 attempts) - Improve error handling and user feedback in offline banner - Extract UI timing constants for better maintainability - Update tests to reflect new network status behavior
1 parent 4488da1 commit 634b583

File tree

8 files changed

+382
-167
lines changed

8 files changed

+382
-167
lines changed

cli/src/app.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ export const App = ({
183183
dense
184184
/>
185185
</box>
186-
{validationErrors.length > 0 && networkStatus.isOnline && (
186+
{validationErrors.length > 0 && networkStatus.validation.isReachable && (
187187
<box style={{ flexDirection: 'column', gap: 0 }}>
188188
{createValidationErrorBlocks({
189189
errors: validationErrors,
@@ -210,14 +210,14 @@ export const App = ({
210210
isAgentListCollapsed,
211211
validationErrors,
212212
separatorWidth,
213-
networkStatus.isOnline,
213+
networkStatus.validation.isReachable,
214214
])
215215

216216
// Only show login modal for actual authentication failures, not network errors
217217
if (
218218
requireAuth !== null &&
219219
isAuthenticated === false &&
220-
networkStatus.isOnline // Only require login when we're online
220+
networkStatus.auth.isReachable // Only require login when auth is reachable
221221
) {
222222
return (
223223
<LoginModal

cli/src/chat.tsx

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,11 @@ import { loadLocalAgents } from './utils/local-agent-registry'
4242
import { buildMessageTree } from './utils/message-tree-utils'
4343
import { computeInputLayoutMetrics } from './utils/text-layout'
4444
import { createMarkdownPalette } from './utils/theme-system'
45-
import { BORDER_CHARS } from './utils/ui-constants'
45+
import {
46+
BORDER_CHARS,
47+
RECONNECTION_MESSAGE_DURATION_MS,
48+
RECONNECTION_RETRY_DELAY_MS,
49+
} from './utils/ui-constants'
4650

4751
import type { ChatMessage, ContentBlock } from './types/chat'
4852
import type { SendMessageFn } from './types/contracts/send-message'
@@ -219,11 +223,11 @@ export const Chat = ({
219223
clearTimeout(reconnectionTimeoutRef.current)
220224
}
221225

222-
// Hide the message after 2 seconds
226+
// Hide the message after the configured duration
223227
reconnectionTimeoutRef.current = setTimeout(() => {
224228
setShowReconnectionMessage(false)
225229
reconnectionTimeoutRef.current = null
226-
}, 2000)
230+
}, RECONNECTION_MESSAGE_DURATION_MS)
227231
}
228232

229233
// Always trigger retry check (for both initial connection and reconnection)
@@ -560,6 +564,21 @@ export const Chat = ({
560564
continueChatId,
561565
})
562566

567+
const offlineBannerMessage = useMemo(() => {
568+
const baseMessage = `⚠️ Network Error: ${
569+
networkStatus.error?.message ?? 'Connection lost'
570+
}`
571+
const retryMessage =
572+
pendingRetryCount > 0
573+
? ` • ${pendingRetryCount} message${
574+
pendingRetryCount === 1 ? '' : 's'
575+
} will retry when the connection is restored`
576+
: ''
577+
const guidance =
578+
' • Verify your API key or network connection before retrying.'
579+
return `${baseMessage}${retryMessage}${guidance}`
580+
}, [networkStatus.error, pendingRetryCount])
581+
563582
sendMessageRef.current = sendMessage
564583
retryPendingMessagesRef.current = retryPendingMessages
565584
processFailedMessagesRef.current = processFailedMessages
@@ -569,13 +588,13 @@ export const Chat = ({
569588
if (connectionEstablished > 0 && pendingRetryCount > 0) {
570589
logger.info(
571590
{ pendingRetryCount, connectionEstablished },
572-
`[RETRY-EFFECT] CONDITIONS MET! Will retry ${pendingRetryCount} pending message(s) after 500ms delay...`
591+
`[RETRY-EFFECT] CONDITIONS MET! Will retry ${pendingRetryCount} pending message(s) after ${RECONNECTION_RETRY_DELAY_MS}ms delay...`
573592
)
574593
// Small delay to ensure the connection is fully established
575594
const timer = setTimeout(() => {
576595
logger.info('[RETRY-EFFECT] Calling retryPendingMessages()')
577596
retryPendingMessagesRef.current?.()
578-
}, 500)
597+
}, RECONNECTION_RETRY_DELAY_MS)
579598
return () => {
580599
clearTimeout(timer)
581600
}
@@ -864,7 +883,9 @@ export const Chat = ({
864883
)
865884

866885
const validationBanner = useValidationBanner({
867-
liveValidationErrors: networkStatus.isOnline ? validationErrors : [],
886+
liveValidationErrors: networkStatus.validation.isReachable
887+
? validationErrors
888+
: [],
868889
loadedAgentsData,
869890
theme,
870891
})
@@ -1063,11 +1084,16 @@ export const Chat = ({
10631084
paddingBottom: 1,
10641085
paddingLeft: 1,
10651086
paddingRight: 1,
1066-
backgroundColor: '#333333',
1087+
backgroundColor: theme.surface ?? theme.background ?? '#333333',
10671088
}}
10681089
>
1069-
<text style={{ fg: '#FFA500', wrapMode: 'word' }}>
1070-
⚠️ Network Error: {networkStatus.error.message}
1090+
<text
1091+
style={{
1092+
fg: theme.warning ?? theme.foreground,
1093+
wrapMode: 'word',
1094+
}}
1095+
>
1096+
{offlineBannerMessage}
10711097
</text>
10721098
</box>
10731099
)}

cli/src/components/shimmer-text.tsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { TextAttributes } from '@opentui/core'
22
import React, { useEffect, useMemo, useState } from 'react'
33

44
import { useTheme } from '../hooks/use-theme'
5+
import { SHIMMER_INTERVAL_MS } from '../utils/ui-constants'
56

67
const clamp = (value: number, min: number, max: number): number =>
78
Math.min(max, Math.max(min, value))
@@ -130,7 +131,7 @@ const generatePaletteFromPrimary = (
130131

131132
export const ShimmerText = ({
132133
text,
133-
interval = 180,
134+
interval = SHIMMER_INTERVAL_MS,
134135
colors,
135136
primaryColor,
136137
}: {
@@ -145,7 +146,7 @@ export const ShimmerText = ({
145146
const numChars = chars.length
146147

147148
if (numChars === 0) {
148-
return null
149+
return <></>
149150
}
150151

151152
useEffect(() => {

cli/src/hooks/__tests__/use-network-status.test.ts

Lines changed: 24 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,5 @@
1-
import { describe, expect, test, beforeEach, afterEach, spyOn } from 'bun:test'
2-
import { renderHook } from '@testing-library/react'
3-
import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
4-
import React from 'react'
5-
import { useNetworkStatus } from '../use-network-status'
1+
import { describe, expect, test, spyOn } from 'bun:test'
2+
import { useNetworkStatus, type NetworkStatus } from '../use-network-status'
63
import * as useAuthQueryModule from '../use-auth-query'
74

85
/**
@@ -60,7 +57,7 @@ describe('useNetworkStatus', () => {
6057
mockUseAuthQuery.mockRestore()
6158
})
6259

63-
test('detects validation network errors', () => {
60+
test('reports validation degradation without marking offline', () => {
6461
const mockAuthQuery = {
6562
error: null,
6663
isError: false,
@@ -75,10 +72,12 @@ describe('useNetworkStatus', () => {
7572
validationNetworkError: 'Failed to connect to validation API',
7673
})
7774

78-
expect(result.isOnline).toBe(false)
75+
expect(result.isOnline).toBe(true)
7976
expect(result.error).not.toBeNull()
8077
expect(result.error?.source).toBe('validation')
8178
expect(result.error?.message).toBe('Failed to connect to validation API')
79+
expect(result.validation.isReachable).toBe(false)
80+
expect(result.auth.isReachable).toBe(true)
8281

8382
// Restore original
8483
mockUseAuthQuery.mockRestore()
@@ -137,44 +136,31 @@ describe('useNetworkStatus', () => {
137136
})
138137

139138
describe('type definitions', () => {
140-
test('NetworkStatus type is correctly defined', () => {
141-
type NetworkStatus =
142-
| { isOnline: true; error: null }
143-
| { isOnline: false; error: { source: 'auth' | 'validation' | 'unknown'; message: string } }
144-
145-
const onlineStatus: NetworkStatus = {
139+
test('NetworkStatus shape includes auth and validation fields', () => {
140+
const status: NetworkStatus = {
146141
isOnline: true,
147142
error: null,
143+
auth: { isReachable: true, error: null },
144+
validation: { isReachable: false, error: 'Validation unavailable' },
148145
}
149146

150-
const offlineStatus: NetworkStatus = {
151-
isOnline: false,
152-
error: {
153-
source: 'auth',
154-
message: 'Network error',
155-
},
156-
}
157-
158-
expect(onlineStatus.isOnline).toBe(true)
159-
expect(onlineStatus.error).toBeNull()
160-
expect(offlineStatus.isOnline).toBe(false)
161-
expect(offlineStatus.error.source).toBe('auth')
147+
expect(status.isOnline).toBe(true)
148+
expect(status.error).toBeNull()
149+
expect(status.auth.isReachable).toBe(true)
150+
expect(status.validation.isReachable).toBe(false)
151+
expect(status.validation.error).toBe('Validation unavailable')
162152
})
163153

164154
test('error sources are exhaustive', () => {
165-
const sources = ['auth', 'validation', 'unknown']
166-
167-
sources.forEach(source => {
168-
const status = {
169-
isOnline: false,
170-
error: {
171-
source: source as 'auth' | 'validation' | 'unknown',
172-
message: 'Test error',
173-
},
174-
}
175-
176-
expect(['auth', 'validation', 'unknown']).toContain(status.error.source)
155+
const sources: Array<NetworkStatus['error']> = [
156+
{ source: 'auth', message: 'Auth down' },
157+
{ source: 'validation', message: 'Validation down' },
158+
{ source: 'unknown', message: 'Unknown' },
159+
]
160+
161+
sources.forEach((error) => {
162+
expect(['auth', 'validation', 'unknown']).toContain(error?.source)
177163
})
178164
})
179165
})
180-
})
166+
})
Lines changed: 47 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,51 +1,68 @@
1-
import { useState, useEffect, useCallback } from 'react'
21
import { useAuthQuery } from './use-auth-query'
32
import { isNetworkError } from '@codebuff/sdk'
43

5-
export type NetworkStatus =
6-
| { isOnline: true; error: null }
7-
| { isOnline: false; error: { source: 'auth' | 'validation' | 'unknown'; message: string } }
4+
export type NetworkStatusErrorSource = 'auth' | 'validation' | 'unknown'
5+
6+
export interface NetworkStatusDetails {
7+
isReachable: boolean
8+
error: string | null
9+
}
10+
11+
export interface NetworkStatus {
12+
isOnline: boolean
13+
error: { source: NetworkStatusErrorSource; message: string } | null
14+
auth: NetworkStatusDetails
15+
validation: NetworkStatusDetails
16+
}
817

918
interface UseNetworkStatusOptions {
1019
validationNetworkError?: string | null
1120
}
1221

1322
/**
1423
* Unified hook for network status detection.
15-
* Consolidates network error detection from auth and validation into a single source of truth.
24+
* Keeps login/auth logic responsive even if the validation service is degraded.
1625
*/
17-
export function useNetworkStatus(options: UseNetworkStatusOptions = {}): NetworkStatus {
26+
export function useNetworkStatus(
27+
options: UseNetworkStatusOptions = {},
28+
): NetworkStatus {
1829
const { validationNetworkError } = options
1930
const authQuery = useAuthQuery()
2031

21-
// Check auth query for network errors
22-
const authNetworkError = authQuery.error && isNetworkError(authQuery.error)
23-
? authQuery.error.message || 'Unable to reach server'
24-
: null
25-
26-
// Determine overall network status
27-
if (authNetworkError) {
28-
return {
29-
isOnline: false,
30-
error: {
31-
source: 'auth',
32-
message: authNetworkError,
33-
},
34-
}
32+
const authNetworkError =
33+
authQuery.error && isNetworkError(authQuery.error)
34+
? authQuery.error.message || 'Unable to reach server'
35+
: null
36+
37+
const authStatus: NetworkStatusDetails = {
38+
isReachable: authNetworkError == null,
39+
error: authNetworkError,
3540
}
3641

37-
if (validationNetworkError) {
38-
return {
39-
isOnline: false,
40-
error: {
41-
source: 'validation',
42-
message: validationNetworkError,
43-
},
42+
const validationStatus: NetworkStatusDetails = {
43+
isReachable: !validationNetworkError,
44+
error: validationNetworkError ?? null,
45+
}
46+
47+
let consolidatedError: NetworkStatus['error'] = null
48+
49+
if (!authStatus.isReachable) {
50+
consolidatedError = {
51+
source: 'auth',
52+
message: authStatus.error ?? 'Unable to reach server',
53+
}
54+
} else if (!validationStatus.isReachable) {
55+
consolidatedError = {
56+
source: 'validation',
57+
message:
58+
validationStatus.error ?? 'Validation service is temporarily unavailable',
4459
}
4560
}
4661

4762
return {
48-
isOnline: true,
49-
error: null,
63+
isOnline: authStatus.isReachable,
64+
error: consolidatedError,
65+
auth: authStatus,
66+
validation: validationStatus,
5067
}
51-
}
68+
}

0 commit comments

Comments
 (0)