Skip to content

fix: handle zero timeout correctly for OpenAI-compatible providers #7367

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
+17 −11
Closed

fix: handle zero timeout correctly for OpenAI-compatible providers #7367

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions src/api/providers/__tests__/lm-studio-timeout.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ describe("LmStudioHandler timeout configuration", () => {
)
})

it("should handle zero timeout (no timeout)", () => {
it("should handle zero timeout (no timeout) by passing undefined to OpenAI client", () => {
;(getApiRequestTimeout as any).mockReturnValue(0)

const options: ApiHandlerOptions = {
Expand All @@ -84,7 +84,7 @@ describe("LmStudioHandler timeout configuration", () => {

expect(mockOpenAIConstructor).toHaveBeenCalledWith(
expect.objectContaining({
timeout: 0, // No timeout
timeout: undefined, // OpenAI SDK expects undefined for no timeout, not 0
}),
)
})
Expand Down
4 changes: 2 additions & 2 deletions src/api/providers/__tests__/ollama-timeout.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ describe("OllamaHandler timeout configuration", () => {
)
})

it("should handle zero timeout (no timeout)", () => {
it("should handle zero timeout (no timeout) by passing undefined to OpenAI client", () => {
;(getApiRequestTimeout as any).mockReturnValue(0)

const options: ApiHandlerOptions = {
Expand All @@ -84,7 +84,7 @@ describe("OllamaHandler timeout configuration", () => {

expect(mockOpenAIConstructor).toHaveBeenCalledWith(
expect.objectContaining({
timeout: 0, // No timeout
timeout: undefined, // OpenAI SDK expects undefined for no timeout, not 0
}),
)
})
Expand Down
4 changes: 2 additions & 2 deletions src/api/providers/__tests__/openai-timeout.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ describe("OpenAiHandler timeout configuration", () => {
)
})

it("should handle zero timeout (no timeout)", () => {
it("should handle zero timeout (no timeout) by passing undefined to OpenAI client", () => {
;(getApiRequestTimeout as any).mockReturnValue(0)

const options: ApiHandlerOptions = {
Expand All @@ -137,7 +137,7 @@ describe("OpenAiHandler timeout configuration", () => {

expect(mockOpenAIConstructor).toHaveBeenCalledWith(
expect.objectContaining({
timeout: 0, // No timeout
timeout: undefined, // OpenAI SDK expects undefined for no timeout, not 0
}),
)
})
Expand Down
4 changes: 3 additions & 1 deletion src/api/providers/lm-studio.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,12 @@ export class LmStudioHandler extends BaseProvider implements SingleCompletionHan
super()
this.options = options

const timeout = getApiRequestTimeout()
this.client = new OpenAI({
baseURL: (this.options.lmStudioBaseUrl || "http://localhost:1234") + "/v1",
apiKey: "noop",
timeout: getApiRequestTimeout(),
// OpenAI SDK expects undefined for no timeout, not 0
timeout: timeout === 0 ? undefined : timeout,
Copy link
Contributor Author

@roomote roomote bot Aug 24, 2025

Choose a reason for hiding this comment

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

Is this pattern intentional? I notice that here in LM Studio and Ollama, we're doing the conversion inline, but in the OpenAI provider, we're using an intermediate variable clientTimeout. Would it be worth standardizing on one approach across all three providers for consistency?

})
}

Expand Down
4 changes: 3 additions & 1 deletion src/api/providers/ollama.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,12 @@ export class OllamaHandler extends BaseProvider implements SingleCompletionHandl
super()
this.options = options

const timeout = getApiRequestTimeout()
this.client = new OpenAI({
baseURL: (this.options.ollamaBaseUrl || "http://localhost:11434") + "/v1",
apiKey: "ollama",
timeout: getApiRequestTimeout(),
// OpenAI SDK expects undefined for no timeout, not 0
timeout: timeout === 0 ? undefined : timeout,
Copy link
Contributor Author

@roomote roomote bot Aug 24, 2025

Choose a reason for hiding this comment

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

Same pattern as LM Studio - inline conversion. Could we consider using the same approach as the OpenAI provider with an intermediate variable for better readability across all providers?

})
}

Expand Down
8 changes: 5 additions & 3 deletions src/api/providers/openai.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ export class OpenAiHandler extends BaseProvider implements SingleCompletionHandl
}

const timeout = getApiRequestTimeout()
// OpenAI SDK expects undefined for no timeout, not 0
const clientTimeout = timeout === 0 ? undefined : timeout
Copy link
Contributor Author

@roomote roomote bot Aug 24, 2025

Choose a reason for hiding this comment

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

Nice use of an intermediate variable here! Though I'm wondering if we should consider extracting this conversion logic (0 to undefined) into a shared utility function since it's repeated in all three providers? Something like:

Suggested change
const clientTimeout = timeout === 0 ? undefined : timeout
const clientTimeout = normalizeOpenAITimeout(timeout)

Where normalizeOpenAITimeout could live in the utils folder and handle this conversion consistently.


if (isAzureAiInference) {
// Azure AI Inference Service (e.g., for DeepSeek) uses a different path structure
Expand All @@ -56,7 +58,7 @@ export class OpenAiHandler extends BaseProvider implements SingleCompletionHandl
apiKey,
defaultHeaders: headers,
defaultQuery: { "api-version": this.options.azureApiVersion || "2024-05-01-preview" },
timeout,
timeout: clientTimeout,
})
} else if (isAzureOpenAi) {
// Azure API shape slightly differs from the core API shape:
Expand All @@ -66,14 +68,14 @@ export class OpenAiHandler extends BaseProvider implements SingleCompletionHandl
apiKey,
apiVersion: this.options.azureApiVersion || azureOpenAiDefaultApiVersion,
defaultHeaders: headers,
timeout,
timeout: clientTimeout,
})
} else {
this.client = new OpenAI({
baseURL,
apiKey,
defaultHeaders: headers,
timeout,
timeout: clientTimeout,
})
}
}
Expand Down
Loading