Skip to content

Commit 6b4c124

Browse files
committed
refactor: centralize retry constants and document error handling patterns
- Create sdk/src/retry-config.ts with all retry/connection timing constants - MAX_RETRIES_PER_MESSAGE, RETRY_BACKOFF_BASE_DELAY_MS, RETRY_BACKOFF_MAX_DELAY_MS - RECONNECTION_RETRY_DELAY_MS, RECONNECTION_MESSAGE_DURATION_MS - calculateBackoffDelay() helper and DEFAULT_RETRY_CONFIG type - Add comprehensive error handling documentation to sdk/src/errors.ts - Document throwing pattern as standard approach - Explain when to use try/catch vs ErrorOr pattern - Document error types, codes, and retryability - Update all imports across CLI and SDK to use centralized constants - cli/src/hooks/use-send-message.ts - cli/src/chat.tsx - cli/src/utils/ui-constants.ts - cli/src/__tests__/integration/reconnection-retry.test.ts - Export all retry config constants from sdk/src/index.ts Benefits: - Single source of truth for timing configuration - Consistent error handling patterns across codebase - Type-safe constants with clear documentation - Tests use same constants as production code
1 parent f88ddcf commit 6b4c124

File tree

7 files changed

+184
-29
lines changed

7 files changed

+184
-29
lines changed

cli/src/__tests__/integration/reconnection-retry.test.ts

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,9 @@
11
import { describe, test, expect, beforeEach, mock } from 'bun:test'
2+
import {
3+
MAX_RETRIES_PER_MESSAGE,
4+
RETRY_BACKOFF_BASE_DELAY_MS,
5+
RETRY_BACKOFF_MAX_DELAY_MS,
6+
} from '@codebuff/sdk'
27

38
/**
49
* Integration tests for reconnection and retry logic
@@ -153,7 +158,6 @@ describe('Reconnection and Retry Logic', () => {
153158
})
154159

155160
test('should respect max retry attempts', async () => {
156-
const MAX_RETRIES = 3
157161
const retryAttempts: Record<string, number> = {}
158162
const messageId = 'msg-1'
159163
let failedMessageMarked = false
@@ -164,7 +168,7 @@ describe('Reconnection and Retry Logic', () => {
164168

165169
const attemptRetry = () => {
166170
const attempts = retryAttempts[messageId] ?? 0
167-
if (attempts >= MAX_RETRIES) {
171+
if (attempts >= MAX_RETRIES_PER_MESSAGE) {
168172
markMessageFailed(messageId, 'Maximum retry attempts reached')
169173
return false
170174
}
@@ -218,7 +222,7 @@ describe('Reconnection and Retry Logic', () => {
218222
})
219223

220224
describe('Timeout Management', () => {
221-
test('should cleanup timeout on unmount', () => {
225+
test('should cleanup timeout on unmount', async () => {
222226
let timeoutRef: NodeJS.Timeout | null = null
223227
let cleanupCalled = false
224228

@@ -252,12 +256,11 @@ describe('Reconnection and Retry Logic', () => {
252256
expect(timeoutRef).toBeNull()
253257

254258
// Wait to ensure timeout doesn't fire
255-
setTimeout(() => {
256-
expect(timeoutFired).toBe(false)
257-
}, 150)
259+
await new Promise((resolve) => setTimeout(resolve, 150))
260+
expect(timeoutFired).toBe(false)
258261
})
259262

260-
test('should replace existing timeout when setting new one', () => {
263+
test('should replace existing timeout when setting new one', async () => {
261264
let timeoutRef: NodeJS.Timeout | null = null
262265
const callbacks: string[] = []
263266

@@ -279,10 +282,9 @@ describe('Reconnection and Retry Logic', () => {
279282
}, 50)
280283

281284
// Wait for second timeout to fire
282-
setTimeout(() => {
283-
expect(callbacks).toEqual(['second'])
284-
expect(callbacks).not.toContain('first')
285-
}, 120)
285+
await new Promise((resolve) => setTimeout(resolve, 120))
286+
expect(callbacks).toEqual(['second'])
287+
expect(callbacks).not.toContain('first')
286288
})
287289
})
288290

@@ -336,9 +338,6 @@ describe('Reconnection and Retry Logic', () => {
336338

337339
describe('Backoff Strategy', () => {
338340
test('should implement exponential backoff for retries', () => {
339-
const RETRY_BACKOFF_BASE_DELAY_MS = 1000
340-
const RETRY_BACKOFF_MAX_DELAY_MS = 30000
341-
342341
let backoffDelay = RETRY_BACKOFF_BASE_DELAY_MS
343342

344343
const calculateNextBackoff = (hasMorePending: boolean) => {
@@ -351,20 +350,18 @@ describe('Reconnection and Retry Logic', () => {
351350
}
352351

353352
// First retry - no backoff increase (no more pending)
354-
expect(calculateNextBackoff(false)).toBe(1000)
353+
expect(calculateNextBackoff(false)).toBe(RETRY_BACKOFF_BASE_DELAY_MS)
355354

356355
// Subsequent retries with pending messages
357356
expect(calculateNextBackoff(true)).toBe(2000)
358357
expect(calculateNextBackoff(true)).toBe(4000)
359-
expect(calculateNextBackoff(true)).toBe(8000)
360-
expect(calculateNextBackoff(true)).toBe(16000)
361-
expect(calculateNextBackoff(true)).toBe(30000) // Capped at max
358+
expect(calculateNextBackoff(true)).toBe(RETRY_BACKOFF_MAX_DELAY_MS) // Capped at max (8000)
362359

363360
// Should stay at max
364-
expect(calculateNextBackoff(true)).toBe(30000)
361+
expect(calculateNextBackoff(true)).toBe(RETRY_BACKOFF_MAX_DELAY_MS)
365362

366363
// Reset when no more pending
367-
expect(calculateNextBackoff(false)).toBe(1000)
364+
expect(calculateNextBackoff(false)).toBe(RETRY_BACKOFF_BASE_DELAY_MS)
368365
})
369366
})
370367
})

cli/src/chat.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,12 +50,12 @@ import { loadLocalAgents } from './utils/local-agent-registry'
5050
import { buildMessageTree } from './utils/message-tree-utils'
5151
import { computeInputLayoutMetrics } from './utils/text-layout'
5252
import { createMarkdownPalette } from './utils/theme-system'
53+
import { BORDER_CHARS } from './utils/ui-constants'
54+
import { formatRetryBannerMessage } from './utils/error-messages'
5355
import {
54-
BORDER_CHARS,
5556
RECONNECTION_MESSAGE_DURATION_MS,
5657
RECONNECTION_RETRY_DELAY_MS,
57-
} from './utils/ui-constants'
58-
import { formatRetryBannerMessage } from './utils/error-messages'
58+
} from '@codebuff/sdk'
5959

6060
import type { ChatMessage, ContentBlock } from './types/chat'
6161
import type { SendMessageFn } from './types/contracts/send-message'

cli/src/hooks/use-send-message.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ import {
2020
isNetworkError,
2121
ErrorCodes,
2222
RETRYABLE_ERROR_CODES,
23+
MAX_RETRIES_PER_MESSAGE,
24+
RETRY_BACKOFF_BASE_DELAY_MS,
25+
RETRY_BACKOFF_MAX_DELAY_MS,
2326
} from '@codebuff/sdk'
2427
import {
2528
loadMostRecentChatState,
@@ -42,9 +45,9 @@ const hiddenToolNames = new Set<ToolName | 'spawn_agent_inline'>([
4245
'spawn_agents',
4346
])
4447

45-
const MAX_RETRIES_PER_MESSAGE = 3
46-
const RETRY_BACKOFF_BASE_DELAY_MS = 1_000
47-
const RETRY_BACKOFF_MAX_DELAY_MS = 8_000
48+
// Message retry configuration - imported from @codebuff/sdk/retry-config
49+
// MAX_RETRIES_PER_MESSAGE, RETRY_BACKOFF_BASE_DELAY_MS, RETRY_BACKOFF_MAX_DELAY_MS
50+
4851
const MAX_FAILED_MESSAGES_TO_STORE = 50 // Limit memory usage for failed messages
4952
const FAILED_MESSAGE_TTL_MS = 5 * 60 * 1000 // 5 minutes - prune old failures
5053

cli/src/utils/ui-constants.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,8 @@ export const BORDER_CHARS: BorderCharacters = {
1414
cross: '┼',
1515
}
1616

17-
// Reconnection timing
18-
export const RECONNECTION_RETRY_DELAY_MS = 500
19-
export const RECONNECTION_MESSAGE_DURATION_MS = 2000
17+
// Reconnection timing constants moved to @codebuff/sdk/retry-config
18+
// Import RECONNECTION_RETRY_DELAY_MS and RECONNECTION_MESSAGE_DURATION_MS from @codebuff/sdk
2019

2120
// Shimmer animation
2221
export const SHIMMER_INTERVAL_MS = 160

sdk/src/errors.ts

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,62 @@
11
/**
22
* Custom error classes for the SDK
3+
*
4+
* ## Error Handling Philosophy
5+
*
6+
* This SDK uses a **consistent throwing pattern** for error handling:
7+
* - All SDK functions throw typed errors (AuthenticationError, NetworkError, etc.)
8+
* - Errors include a `code` property for programmatic error type checking
9+
* - Error codes indicate whether the error is retryable
10+
* - Optional ErrorOr wrappers available for functional programming style
11+
*
12+
* ## Error Types
13+
*
14+
* ### AuthenticationError (code: 'AUTH_FAILED')
15+
* - Thrown for 401/403 HTTP responses
16+
* - Indicates invalid credentials or expired tokens
17+
* - NOT retryable - requires user intervention
18+
*
19+
* ### NetworkError (code: 'NETWORK_ERROR')
20+
* - Thrown for connection failures, timeouts, 5xx errors
21+
* - Indicates transient network or server issues
22+
* - IS retryable - automatic retry is recommended
23+
*
24+
* ## Usage Patterns
25+
*
26+
* ### Pattern 1: Try/Catch with Error Code Checking (Recommended for most cases)
27+
* ```typescript
28+
* try {
29+
* const user = await getUserInfoFromApiKey({ apiKey, fields: ['id'], logger })
30+
* } catch (error) {
31+
* if (isAuthenticationError(error)) {
32+
* // Handle auth failure - show login prompt
33+
* } else if (isNetworkError(error)) {
34+
* // Handle network error - schedule retry
35+
* }
36+
* }
37+
* ```
38+
*
39+
* ### Pattern 2: ErrorOr Functional Style (Optional)
40+
* ```typescript
41+
* const result = await getUserInfoFromApiKeySafe({ apiKey, fields: ['id'], logger })
42+
* if (!result.success) {
43+
* // Handle error from result.error
44+
* }
45+
* // Use result.value
46+
* ```
47+
*
48+
* ## Error Code Checking
49+
*
50+
* Use the provided type guards for safe error type checking:
51+
* - `isAuthenticationError(error)` - Checks for AUTH_FAILED
52+
* - `isNetworkError(error)` - Checks for NETWORK_ERROR
53+
* - `isErrorWithCode(error)` - Checks if error has a code property
54+
*
55+
* Check `RETRYABLE_ERROR_CODES` set to determine if an error should trigger retry logic.
56+
*
57+
* ## Related
58+
* - See `retry-config.ts` for retry timing and backoff configuration
59+
* - See `ErrorOr` types from `@codebuff/common/util/error` for functional error handling
360
*/
461

562
/**

sdk/src/index.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,21 @@ export {
4646
} from './errors'
4747
export type { ValidationResult, ValidateAgentsOptions } from './validate-agents'
4848

49+
// Retry and connection configuration
50+
export {
51+
MAX_RETRIES_PER_MESSAGE,
52+
RETRY_BACKOFF_BASE_DELAY_MS,
53+
RETRY_BACKOFF_MAX_DELAY_MS,
54+
RECONNECTION_RETRY_DELAY_MS,
55+
RECONNECTION_MESSAGE_DURATION_MS,
56+
calculateBackoffDelay,
57+
DEFAULT_RETRY_CONFIG,
58+
type RetryConfig,
59+
} from './retry-config'
60+
61+
// Re-export promise utilities and constants from common
62+
export { INITIAL_RETRY_DELAY } from '@codebuff/common/util/promise'
63+
4964
// ErrorOr utilities
5065
export {
5166
failure,

sdk/src/retry-config.ts

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
/**
2+
* Centralized retry and connection configuration
3+
*
4+
* This file contains all constants related to retry logic, backoff strategies,
5+
* and connection management across the SDK and CLI.
6+
*/
7+
8+
/**
9+
* Maximum number of retry attempts per message before giving up
10+
* After this many failures, the message will be marked as failed
11+
*/
12+
export const MAX_RETRIES_PER_MESSAGE = 3
13+
14+
/**
15+
* Base delay for exponential backoff retry strategy (in milliseconds)
16+
* First retry starts at this delay, then doubles with each subsequent retry
17+
*
18+
* Example progression: 1s → 2s → 4s → 8s (capped at max)
19+
*/
20+
export const RETRY_BACKOFF_BASE_DELAY_MS = 1_000
21+
22+
/**
23+
* Maximum delay for exponential backoff retry strategy (in milliseconds)
24+
* Prevents backoff from growing unbounded during extended outages
25+
*/
26+
export const RETRY_BACKOFF_MAX_DELAY_MS = 8_000
27+
28+
/**
29+
* Delay before attempting to retry pending messages after reconnection (in milliseconds)
30+
* Short delay allows connection to stabilize before retrying
31+
*/
32+
export const RECONNECTION_RETRY_DELAY_MS = 500
33+
34+
/**
35+
* Duration to show the reconnection success message (in milliseconds)
36+
* Message is automatically hidden after this duration
37+
*/
38+
export const RECONNECTION_MESSAGE_DURATION_MS = 2_000
39+
40+
/**
41+
* Calculates the next backoff delay using exponential backoff strategy
42+
*
43+
* @param currentDelay - Current delay in milliseconds
44+
* @param hasMorePending - Whether there are more pending operations
45+
* @returns Next delay in milliseconds, capped at RETRY_BACKOFF_MAX_DELAY_MS
46+
*
47+
* @example
48+
* ```typescript
49+
* let delay = RETRY_BACKOFF_BASE_DELAY_MS
50+
* delay = calculateBackoffDelay(delay, true) // 2000ms
51+
* delay = calculateBackoffDelay(delay, true) // 4000ms
52+
* delay = calculateBackoffDelay(delay, false) // 1000ms (reset)
53+
* ```
54+
*/
55+
export function calculateBackoffDelay(
56+
currentDelay: number,
57+
hasMorePending: boolean,
58+
): number {
59+
if (!hasMorePending) {
60+
return RETRY_BACKOFF_BASE_DELAY_MS
61+
}
62+
return Math.min(currentDelay * 2, RETRY_BACKOFF_MAX_DELAY_MS)
63+
}
64+
65+
/**
66+
* Type representing retry configuration that can be customized
67+
*/
68+
export interface RetryConfig {
69+
maxRetries: number
70+
baseDelay: number
71+
maxDelay: number
72+
reconnectionDelay: number
73+
}
74+
75+
/**
76+
* Default retry configuration
77+
* Can be used to create custom configurations
78+
*/
79+
export const DEFAULT_RETRY_CONFIG: RetryConfig = {
80+
maxRetries: MAX_RETRIES_PER_MESSAGE,
81+
baseDelay: RETRY_BACKOFF_BASE_DELAY_MS,
82+
maxDelay: RETRY_BACKOFF_MAX_DELAY_MS,
83+
reconnectionDelay: RECONNECTION_RETRY_DELAY_MS,
84+
}

0 commit comments

Comments
 (0)