fix: increase agent execute and chat completions timeouts to ≥600s#141
fix: increase agent execute and chat completions timeouts to ≥600s#141dineshreddy91 wants to merge 1 commit into
Conversation
- Agent completions: enforce min 600000ms (600s) timeout for OpenAI client - Executions: increase HTTP request timeout from 120000ms to 600000ms - Executions.wait: increase default polling timeout from 300s to 600s - Predictions.wait: increase default polling timeout from 60s to 600s - Update agent tests to match new timeout behavior Orion agent executions and chat completions can take several minutes, especially for complex tasks. Previous timeouts caused premature errors and retries. Co-Authored-By: dinesh@vlm.run <dinesh.andromeda@gmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
There was a problem hiding this comment.
Code Review
This pull request increases the default timeout values across the client SDK (including OpenAI client initialization, API requestors, and execution/prediction wait times) to 10 minutes (600,000 ms or 600 seconds). However, the feedback points out that unconditionally overriding or enforcing a minimum timeout of 10 minutes prevents users from specifying shorter custom timeouts. It is recommended to respect the user's explicitly configured timeout and only fall back to the 10-minute default when it is omitted. Additionally, the unit tests should be updated to correctly assert the custom timeout values rather than the overridden default.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| apiKey: this.client.apiKey, | ||
| baseURL: baseUrl, | ||
| timeout: this.client.timeout, | ||
| timeout: Math.max(this.client.timeout ?? 600000, 600000), |
There was a problem hiding this comment.
Overriding the user-specified timeout with a minimum of 10 minutes (600000 ms) prevents users from setting shorter timeouts (e.g., for fast-failing requests). Standard SDK practice is to respect the user's explicitly configured timeout if provided, and only fall back to the default when it is omitted.
Additionally, the unit test "should use client timeout and maxRetries when provided" (lines 415-432 in agent.test.ts) now has a mismatch where it configures a 30000 ms timeout but asserts that 600000 ms is used.
| timeout: Math.max(this.client.timeout ?? 600000, 600000), | |
| timeout: this.client.timeout ?? 600000, |
| constructor(client: Client) { | ||
| this.client = client; | ||
| this.requestor = new APIRequestor({ ...client, timeout: 120000 }); | ||
| this.requestor = new APIRequestor({ ...client, timeout: 600000 }); |
There was a problem hiding this comment.
Using { ...client, timeout: 600000 } unconditionally overrides any custom timeout configured by the user on the client instance. If a user explicitly configures a custom timeout (whether shorter or longer), it should be respected.
We should default to 600000 ms only if client.timeout is not specified.
| this.requestor = new APIRequestor({ ...client, timeout: 600000 }); | |
| this.requestor = new APIRequestor({ ...client, timeout: client.timeout ?? 600000 }); |
| apiKey: "test-api-key", | ||
| baseURL: "https://agent.vlm.run/v1/openai", | ||
| timeout: 30000, | ||
| timeout: 600000, |
There was a problem hiding this comment.
The test "should use client timeout and maxRetries when provided" configures a custom timeout of 30000 ms but asserts that the timeout is 600000 ms. This contradicts the test's description and intent. Once the implementation is updated to respect the user's custom timeout, this assertion should be updated to expect the configured 30000 ms.
| timeout: 600000, | |
| timeout: 30000, |
| constructor(client: Client) { | ||
| this.client = client; | ||
| this.requestor = new APIRequestor({ ...client, timeout: 120000 }); | ||
| this.requestor = new APIRequestor({ ...client, timeout: 600000 }); |
There was a problem hiding this comment.
🚩 Inconsistent timeout override patterns across client classes
The timeout override approach differs across SDK client classes:
Agent.completionsusesMath.max(client.timeout ?? 600000, 600000)— respects user timeouts above 600sExecutionsconstructor uses{ ...client, timeout: 600000 }— always hardcodes 600000ms, ignoring user config entirely (even if user wants >600s)Filesuses{ ...client, timeout: 0 }— always hardcodes 0 for unlimitedArtifactsusesthis.client.timeout ?? 120000— uses user config if set, 120000ms default otherwise
The Executions class at src/client/executions.ts:11 doesn't use the Math.max pattern, so a user who sets a timeout of e.g. 900000ms (15 minutes) would have it silently reduced to 600000ms for executions. This is a pre-existing pattern (previously hardcoded to 120000) but worth noting given that Agent now uses a more permissive approach.
Was this helpful? React with 👍 or 👎 to provide feedback.
Summary
Increase timeouts for Orion agent execute and chat completions to ≥600s to prevent premature errors and retries on long-running tasks.
Changes:
Agent.completionsOpenAI clientthis.client.timeout(120s default)Math.max(timeout ?? 600000, 600000)Executionsconstructor APIRequestorExecutions.wait()defaultPredictions.wait()defaultUpdated unit tests in
agent.test.tsto expect the new 600000ms minimum.Related PRs: vlm-run/vlmrun-python-sdk (same changes), vlm-run/vlm-lab (server-side timeouts).
Link to Devin session: https://app.devin.ai/sessions/977e2c561b8c4ef8b72d04517946a4dc
Requested by: @dineshreddy91