Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 7 additions & 9 deletions cli/src/__tests__/integration/api-integration.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
import {
AuthenticationError,
NetworkError,
getUserInfoFromApiKey,
} from '@codebuff/sdk'
import { describe, test, expect, beforeEach, afterEach, mock } from 'bun:test'
Expand Down Expand Up @@ -143,7 +141,7 @@ describe('API Integration', () => {
fields: ['id'],
logger: testLogger,
}),
).rejects.toBeInstanceOf(AuthenticationError)
).rejects.toMatchObject({ statusCode: 401 })

// 401s are now logged as auth failures
expect(testLogger.error.mock.calls.length).toBeGreaterThan(0)
Expand All @@ -161,7 +159,7 @@ describe('API Integration', () => {
fields: ['id'],
logger: testLogger,
}),
).rejects.toBeInstanceOf(AuthenticationError)
).rejects.toMatchObject({ statusCode: 401 })

expect(testLogger.error.mock.calls.length).toBeGreaterThan(0)
})
Expand All @@ -180,7 +178,7 @@ describe('API Integration', () => {
fields: ['id'],
logger: testLogger,
}),
).rejects.toBeInstanceOf(NetworkError)
).rejects.toMatchObject({ statusCode: expect.any(Number) })

expect(testLogger.error.mock.calls.length).toBeGreaterThan(0)
})
Expand All @@ -197,7 +195,7 @@ describe('API Integration', () => {
fields: ['id'],
logger: testLogger,
}),
).rejects.toBeInstanceOf(NetworkError)
).rejects.toMatchObject({ statusCode: expect.any(Number) })

expect(
testLogger.error.mock.calls.some(([payload]) =>
Expand All @@ -218,7 +216,7 @@ describe('API Integration', () => {
fields: ['id'],
logger: testLogger,
}),
).rejects.toBeInstanceOf(NetworkError)
).rejects.toMatchObject({ statusCode: expect.any(Number) })

expect(testLogger.error.mock.calls.length).toBeGreaterThan(0)
})
Expand All @@ -239,7 +237,7 @@ describe('API Integration', () => {
fields: ['id'],
logger: testLogger,
}),
).rejects.toBeInstanceOf(NetworkError)
).rejects.toMatchObject({ statusCode: expect.any(Number) })

expect(fetchMock.mock.calls.length).toBe(1)
expect(
Expand All @@ -263,7 +261,7 @@ describe('API Integration', () => {
fields: ['id'],
logger: testLogger,
}),
).rejects.toBeInstanceOf(NetworkError)
).rejects.toMatchObject({ statusCode: expect.any(Number) })

expect(fetchMock.mock.calls.length).toBe(1)
expect(
Expand Down
21 changes: 9 additions & 12 deletions cli/src/app.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { NetworkError, RETRYABLE_ERROR_CODES } from '@codebuff/sdk'
import { isRetryableStatusCode, getErrorStatusCode } from '@codebuff/sdk'
import { useCallback, useEffect, useMemo, useRef, useState } from 'react'
import { useShallow } from 'zustand/react/shallow'

Expand Down Expand Up @@ -211,23 +211,20 @@ export const App = ({
)
}, [logoComponent, projectRoot, theme])

// Derive auth reachability + retrying state inline from authQuery error
// Derive auth reachability + retrying state from authQuery error
const authError = authQuery.error
const networkError =
authError && authError instanceof NetworkError ? authError : null
const isRetryableNetworkError = Boolean(
networkError && RETRYABLE_ERROR_CODES.has(networkError.code),
)
const authErrorStatusCode = authError ? getErrorStatusCode(authError) : undefined

let authStatus: AuthStatus = 'ok'
if (authQuery.isError) {
if (!networkError) {
authStatus = 'ok'
} else if (isRetryableNetworkError) {
if (authQuery.isError && authErrorStatusCode !== undefined) {
if (isRetryableStatusCode(authErrorStatusCode)) {
// Retryable errors (408 timeout, 429 rate limit, 5xx server errors)
authStatus = 'retrying'
} else {
} else if (authErrorStatusCode >= 500) {
// Non-retryable server errors (unlikely but possible future codes)
authStatus = 'unreachable'
}
// 4xx client errors (401, 403, etc.) keep 'ok' - network is fine, just auth failed
}

// Render login modal when not authenticated AND auth service is reachable
Expand Down
6 changes: 3 additions & 3 deletions cli/src/hooks/helpers/__tests__/send-message.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ const { setupStreamingContext, handleRunError } = await import(
const { createBatchedMessageUpdater } = await import(
'../../../utils/message-updater'
)
const { PaymentRequiredError } = await import('@codebuff/sdk')
import { createPaymentRequiredError } from '@codebuff/sdk'

const createMockTimerController = (): SendMessageTimerController & {
startCalls: string[]
Expand Down Expand Up @@ -375,7 +375,7 @@ describe('handleRunError', () => {
expect(mockInvalidateQueries).not.toHaveBeenCalled()
})

test('PaymentRequiredError uses setError, invalidates queries, and switches input mode', () => {
test('Payment required error (402) uses setError, invalidates queries, and switches input mode', () => {
let messages: ChatMessage[] = [
{
id: 'ai-1',
Expand All @@ -397,7 +397,7 @@ describe('handleRunError', () => {
setInputMode: setInputModeMock,
})

const paymentError = new PaymentRequiredError('Out of credits')
const paymentError = createPaymentRequiredError('Out of credits')

handleRunError({
error: paymentError,
Expand Down
37 changes: 15 additions & 22 deletions cli/src/hooks/helpers/send-message.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ import { useChatStore } from '../../state/chat-store'
import { processBashContext } from '../../utils/bash-context-processor'
import {
createErrorMessage,
createPaymentErrorMessage,
isOutOfCreditsError,
isPaymentRequiredError,
OUT_OF_CREDITS_MESSAGE,
} from '../../utils/error-handling'
import { usageQueryKeys } from '../use-usage-query'
import { formatElapsedTime } from '../../utils/format-elapsed-time'
import { processImagesForMessage } from '../../utils/image-processor'
import { logger } from '../../utils/logger'
Expand All @@ -17,7 +17,6 @@ import {
type BatchedMessageUpdater,
} from '../../utils/message-updater'
import { createModeDividerMessage } from '../../utils/send-message-helpers'
import { usageQueryKeys } from '../use-usage-query'

import type { PendingImage } from '../../state/chat-store'
import type { ChatMessage } from '../../types/chat'
Expand Down Expand Up @@ -216,23 +215,19 @@ export const handleRunCompletion = (params: {
}

if (isOutOfCreditsError(output)) {
const { message, showUsageBanner } = createPaymentErrorMessage(output)
updater.setError(message)

if (showUsageBanner) {
useChatStore.getState().setInputMode('usage')
queryClient.invalidateQueries({
queryKey: usageQueryKeys.current(),
})
}
} else {
const partial = createErrorMessage(
output.message ?? 'No output from agent run',
aiMessageId,
)
updater.setError(partial.content ?? '')
updater.setError(OUT_OF_CREDITS_MESSAGE)
useChatStore.getState().setInputMode('usage')
queryClient.invalidateQueries({ queryKey: usageQueryKeys.current() })
finalizeAfterError()
return
}

const partial = createErrorMessage(
output.message ?? 'No output from agent run',
aiMessageId,
)
updater.setError(partial.content ?? '')

finalizeAfterError()
return
}
Expand Down Expand Up @@ -302,10 +297,8 @@ export const handleRunError = (params: {
updateChainInProgress(false)
timerController.stop('error')

if (isPaymentRequiredError(error)) {
const { message } = createPaymentErrorMessage(error)

updater.setError(message)
if (isOutOfCreditsError(error)) {
updater.setError(OUT_OF_CREDITS_MESSAGE)
useChatStore.getState().setInputMode('usage')
queryClient.invalidateQueries({ queryKey: usageQueryKeys.current() })
return
Expand Down
46 changes: 26 additions & 20 deletions cli/src/hooks/use-auth-query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ import { createHash } from 'crypto'

import { getCiEnv } from '@codebuff/common/env-ci'
import {
AuthenticationError,
ErrorCodes,
getUserInfoFromApiKey as defaultGetUserInfoFromApiKey,
NetworkError,
RETRYABLE_ERROR_CODES,
isRetryableStatusCode,
getErrorStatusCode,
createAuthError,
createServerError,
MAX_RETRIES_PER_MESSAGE,
RETRY_BACKOFF_BASE_DELAY_MS,
} from '@codebuff/sdk'
Expand Down Expand Up @@ -47,6 +47,14 @@ type ValidatedUserInfo = {
email: string
}

/**
* Check if an error is an authentication error (401, 403)
*/
function isAuthenticationError(error: unknown): boolean {
const statusCode = getErrorStatusCode(error)
return statusCode === 401 || statusCode === 403
}

/**
* Validates an API key by calling the backend
*
Expand All @@ -69,42 +77,39 @@ export async function validateApiKey({

if (!authResult) {
logger.error('❌ API key validation failed - invalid credentials')
throw new AuthenticationError('Invalid API key', 401)
throw createAuthError('Invalid API key')
}

return authResult
} catch (error) {
if (error instanceof AuthenticationError) {
const statusCode = getErrorStatusCode(error)

if (isAuthenticationError(error)) {
logger.error('❌ API key validation failed - authentication error')
// Rethrow the original error to preserve error type for higher layers
// Rethrow the original error to preserve statusCode for higher layers
throw error
}

if (error instanceof NetworkError) {
if (statusCode !== undefined && isRetryableStatusCode(statusCode)) {
logger.error(
{
error: error.message,
code: error.code,
error: error instanceof Error ? error.message : String(error),
statusCode,
},
'❌ API key validation failed - network error',
)
// Rethrow the original error to preserve error type for higher layers
// Rethrow the original error to preserve statusCode for higher layers
throw error
}

// Unknown error - wrap in NetworkError for consistency
// Unknown error - wrap with statusCode for consistency
logger.error(
{
error: error instanceof Error ? error.message : String(error),
},
'❌ API key validation failed - unknown error',
)
throw new NetworkError(
'Authentication failed',
ErrorCodes.UNKNOWN_ERROR,
undefined,
error,
)
throw createServerError('Authentication failed')
}
}

Expand Down Expand Up @@ -139,12 +144,13 @@ export function useAuthQuery(deps: UseAuthQueryDeps = {}) {
// Retry only for retryable network errors (5xx, timeouts, etc.)
// Don't retry authentication errors (invalid credentials)
retry: (failureCount, error) => {
const statusCode = getErrorStatusCode(error)
// Don't retry authentication errors - user needs to update credentials
if (error instanceof AuthenticationError) {
if (isAuthenticationError(error)) {
return false
}
// Retry network errors if they're retryable and we haven't exceeded max retries
if (error instanceof NetworkError && RETRYABLE_ERROR_CODES.has(error.code)) {
if (statusCode !== undefined && isRetryableStatusCode(statusCode)) {
return failureCount < MAX_RETRIES_PER_MESSAGE
}
// Don't retry other errors
Expand Down
3 changes: 0 additions & 3 deletions cli/src/hooks/use-send-message.ts
Original file line number Diff line number Diff line change
Expand Up @@ -373,11 +373,8 @@ export const useSendMessage = ({
prompt: effectivePrompt,
content: messageContent,
previousRunState: previousRunStateRef.current,
abortController,
agentDefinitions,
eventHandlerState,
setIsRetrying,
setStreamStatus,
})

const runState = await client.run(runConfig)
Expand Down
44 changes: 0 additions & 44 deletions cli/src/utils/create-run-config.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,3 @@
import {
MAX_RETRIES_PER_MESSAGE,
RETRY_BACKOFF_BASE_DELAY_MS,
RETRY_BACKOFF_MAX_DELAY_MS,
} from '@codebuff/sdk'

import {
createEventHandler,
createStreamChunkHandler,
Expand All @@ -12,30 +6,15 @@ import {
import type { EventHandlerState } from './sdk-event-handlers'
import type { AgentDefinition, MessageContent, RunState } from '@codebuff/sdk'
import type { Logger } from '@codebuff/common/types/contracts/logger'
import type { StreamStatus } from '../hooks/use-message-queue'

export type CreateRunConfigParams = {
logger: Logger
agent: AgentDefinition | string
prompt: string
content: MessageContent[] | undefined
previousRunState: RunState | null
abortController: AbortController
agentDefinitions: AgentDefinition[]
eventHandlerState: EventHandlerState
setIsRetrying: (retrying: boolean) => void
setStreamStatus: (status: StreamStatus) => void
}

type RetryArgs = {
attempt: number
delayMs: number
errorCode?: string
}

type RetryExhaustedArgs = {
totalAttempts: number
errorCode?: string
}

export const createRunConfig = (params: CreateRunConfigParams) => {
Expand All @@ -45,11 +24,8 @@ export const createRunConfig = (params: CreateRunConfigParams) => {
prompt,
content,
previousRunState,
abortController,
agentDefinitions,
eventHandlerState,
setIsRetrying,
setStreamStatus,
} = params

return {
Expand All @@ -58,26 +34,6 @@ export const createRunConfig = (params: CreateRunConfigParams) => {
prompt,
content,
previousRun: previousRunState ?? undefined,
abortController,
retry: {
maxRetries: MAX_RETRIES_PER_MESSAGE,
backoffBaseMs: RETRY_BACKOFF_BASE_DELAY_MS,
backoffMaxMs: RETRY_BACKOFF_MAX_DELAY_MS,
onRetry: async ({ attempt, delayMs, errorCode }: RetryArgs) => {
logger.warn(
{ sdkAttempt: attempt, delayMs, errorCode },
'SDK retrying after error',
)
setIsRetrying(true)
setStreamStatus('waiting')
},
onRetryExhausted: async ({
totalAttempts,
errorCode,
}: RetryExhaustedArgs) => {
logger.warn({ totalAttempts, errorCode }, 'SDK exhausted all retries')
},
},
agentDefinitions,
maxAgentSteps: 100,
handleStreamChunk: createStreamChunkHandler(eventHandlerState),
Expand Down
Loading