Skip to content
Closed
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
151 changes: 151 additions & 0 deletions src/api/providers/__tests__/anthropic-timeout.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
// npx vitest run api/providers/__tests__/anthropic-timeout.spec.ts

import { describe, it, expect, beforeEach, vitest } from "vitest"
import { ApiHandlerOptions } from "../../../shared/api"

// Mock the timeout config utility
vitest.mock("../utils/timeout-config", () => ({
getApiRequestTimeout: vitest.fn(),
}))

import { getApiRequestTimeout } from "../utils/timeout-config"

// Mock the Anthropic SDK
vitest.mock("@anthropic-ai/sdk", () => {
const mockAnthropicConstructor = vitest.fn().mockImplementation(() => ({
messages: {
create: vitest.fn(),
countTokens: vitest.fn(),
},
}))
return {
Anthropic: mockAnthropicConstructor,
}
})

// Import after mocks
import { Anthropic } from "@anthropic-ai/sdk"
import { AnthropicHandler } from "../anthropic"

const mockAnthropicConstructor = Anthropic as any

describe("AnthropicHandler timeout configuration", () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While the unit tests are thorough, have we considered adding an integration test that actually verifies timeout behavior with a real (or mocked) slow response? This could help catch regressions where the timeout isn't properly applied to the actual HTTP requests.

beforeEach(() => {
vitest.clearAllMocks()
})

it("should use default timeout of 600 seconds when no configuration is set", () => {
;(getApiRequestTimeout as any).mockReturnValue(600000)

const options: ApiHandlerOptions = {
apiKey: "test-api-key",
apiModelId: "claude-3-5-sonnet-20241022",
}

new AnthropicHandler(options)

expect(getApiRequestTimeout).toHaveBeenCalled()
expect(mockAnthropicConstructor).toHaveBeenCalledWith(
expect.objectContaining({
apiKey: "test-api-key",
timeout: 600000, // 600 seconds in milliseconds
}),
)
})

it("should use custom timeout when configuration is set", () => {
;(getApiRequestTimeout as any).mockReturnValue(1800000) // 30 minutes

const options: ApiHandlerOptions = {
apiKey: "test-api-key",
apiModelId: "claude-3-5-sonnet-20241022",
}

new AnthropicHandler(options)

expect(mockAnthropicConstructor).toHaveBeenCalledWith(
expect.objectContaining({
apiKey: "test-api-key",
timeout: 1800000, // 1800 seconds in milliseconds
}),
)
})

it("should handle zero timeout (no timeout)", () => {
;(getApiRequestTimeout as any).mockReturnValue(0)

const options: ApiHandlerOptions = {
apiKey: "test-api-key",
apiModelId: "claude-3-5-sonnet-20241022",
}

new AnthropicHandler(options)

expect(mockAnthropicConstructor).toHaveBeenCalledWith(
expect.objectContaining({
apiKey: "test-api-key",
timeout: 0, // No timeout
}),
)
})

it("should use custom base URL when provided", () => {
;(getApiRequestTimeout as any).mockReturnValue(600000)

const options: ApiHandlerOptions = {
apiKey: "test-api-key",
apiModelId: "claude-3-5-sonnet-20241022",
anthropicBaseUrl: "https://custom.anthropic.com",
}

new AnthropicHandler(options)

expect(mockAnthropicConstructor).toHaveBeenCalledWith(
expect.objectContaining({
baseURL: "https://custom.anthropic.com",
apiKey: "test-api-key",
timeout: 600000,
}),
)
})

it("should use authToken when anthropicUseAuthToken is set with custom base URL", () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice comprehensive test coverage! Consider grouping related tests together for better organization. For example, all auth-related tests (lines 112-150) could be grouped under a nested describe block:

describe('authentication handling', () => {
  it('should use authToken when anthropicUseAuthToken is set with custom base URL', ...)
  it('should use apiKey when anthropicUseAuthToken is set but no custom base URL', ...)
})

;(getApiRequestTimeout as any).mockReturnValue(900000) // 15 minutes

const options: ApiHandlerOptions = {
apiKey: "test-auth-token",
apiModelId: "claude-3-5-sonnet-20241022",
anthropicBaseUrl: "https://custom.anthropic.com",
anthropicUseAuthToken: true,
}

new AnthropicHandler(options)

expect(mockAnthropicConstructor).toHaveBeenCalledWith(
expect.objectContaining({
baseURL: "https://custom.anthropic.com",
authToken: "test-auth-token",
timeout: 900000,
}),
)
})

it("should use apiKey when anthropicUseAuthToken is set but no custom base URL", () => {
;(getApiRequestTimeout as any).mockReturnValue(600000)

const options: ApiHandlerOptions = {
apiKey: "test-api-key",
apiModelId: "claude-3-5-sonnet-20241022",
anthropicUseAuthToken: true,
}

new AnthropicHandler(options)

expect(mockAnthropicConstructor).toHaveBeenCalledWith(
expect.objectContaining({
apiKey: "test-api-key",
timeout: 600000,
}),
)
})
})
2 changes: 2 additions & 0 deletions src/api/providers/anthropic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { getModelParams } from "../transform/model-params"
import { BaseProvider } from "./base-provider"
import type { SingleCompletionHandler, ApiHandlerCreateMessageMetadata } from "../index"
import { calculateApiCostAnthropic } from "../../shared/cost"
import { getApiRequestTimeout } from "./utils/timeout-config"

export class AnthropicHandler extends BaseProvider implements SingleCompletionHandler {
private options: ApiHandlerOptions
Expand All @@ -33,6 +34,7 @@ export class AnthropicHandler extends BaseProvider implements SingleCompletionHa
this.client = new Anthropic({
baseURL: this.options.anthropicBaseUrl || undefined,
[apiKeyFieldName]: this.options.apiKey,
timeout: getApiRequestTimeout(),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding a comment here explaining why timeout configuration is important for large file processing. Something like:

Suggested change
timeout: getApiRequestTimeout(),
// Configure timeout to prevent hanging on large files (see issue #7744)
// Uses the VSCode setting 'roo-cline.apiRequestTimeout' (default: 600 seconds)
timeout: getApiRequestTimeout(),

This would help future maintainers understand the context.

})
}

Expand Down
Loading