Skip to content

fix: add default 120s request timeout when provider timeoutSeconds is not configured#79770

Closed
Andrew199012 wants to merge 1 commit into
openclaw:mainfrom
Andrew199012:fix/model-call-timeout-default
Closed

fix: add default 120s request timeout when provider timeoutSeconds is not configured#79770
Andrew199012 wants to merge 1 commit into
openclaw:mainfrom
Andrew199012:fix/model-call-timeout-default

Conversation

@Andrew199012

@Andrew199012 Andrew199012 commented May 9, 2026

Copy link
Copy Markdown

Problem

When models.providers.<id>.timeoutSeconds is not configured, resolveProviderRequestTimeoutMs() returns undefined. This cascades through the entire timeout chain:

  • buildTimeoutAbortSignal({timeoutMs: undefined}) returns {signal: undefined}
  • fetch() called without any abort signal
  • TCP connection hangs indefinitely when upstream API (e.g. DeepSeek) is slow

This is a P0 defect: a slow API response blocks the session forever, causing event loop congestion (CPU 68%+) and system unresponsiveness until Gateway restart.

Fix

Return DEFAULT_REQUEST_TIMEOUT_MS (120 seconds) when timeoutSeconds is not configured/invalid, instead of undefined.

The code already has full timeout infrastructure (buildTimeoutAbortSignal, AbortController, fetchWithSsrFGuard), it just lacked a safe default fallback.

Testing

  • npx tsc --noEmit passes
  • Only affects the fallback branch of resolveProviderRequestTimeoutMs

Related

Fixes a P0 production incident where slow DeepSeek API responses caused session-level deadlock in OpenClaw Gateway (see investigation report).

@Andrew199012
fix: add default 120s request timeout when provider timeoutSeconds is…
77f1e8c 
… not configured

When models.providers.<id>.timeoutSeconds is not set in config.yaml,
resolveProviderRequestTimeoutMs() returns undefined, which cascades
through buildTimeoutAbortSignal({timeoutMs: undefined}) → no AbortSignal
→ fetch() hangs indefinitely on TCP connections.

This is a P0 defect causing session stalls when upstream API (e.g.
DeepSeek) takes long to respond, leading to event loop congestion and
system unresponsiveness.

Fix: return DEFAULT_REQUEST_TIMEOUT_MS (120s) as fallback instead of
undefined, matching the common expectation that all HTTP requests
should have a bounded timeout.
@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: XS triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 9, 2026
@clawsweeper

clawsweeper Bot commented May 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge.

Summary
The PR changes provider model resolution so missing, invalid, or non-positive models.providers.<id>.timeoutSeconds returns a hard-coded 120000 ms requestTimeoutMs.

Reproducibility: yes. for the PR-introduced regression: source inspection shows the new fallback would set modelRequestTimeoutMs before resolveLlmIdleTimeoutMs reaches the local-provider exemption. The original DeepSeek hang is not high-confidence reproduced from the PR because the body reports only npx tsc --noEmit.

Real behavior proof
Needs real behavior proof before merge: The PR body reports only npx tsc --noEmit; it needs redacted logs, terminal output, live output, screenshots, recordings, or artifacts showing after-fix Gateway/provider behavior, with private details redacted, before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, ask a maintainer to comment @clawsweeper re-review.

Next step before merge
The PR needs contributor real behavior proof and a code revision for the local-provider timeout regression; automation cannot supply the contributor's claimed DeepSeek production proof.

Security
Cleared: The diff only changes TypeScript timeout resolution in agent model metadata and does not add dependency, CI, secret, package, or code-execution surface changes.

Review findings

  • [P2] Preserve local provider timeout exemption — src/agents/pi-embedded-runner/model.ts:347
Review details

Best possible solution:

Bound slow cloud/provider HTTP requests without assigning implicit requestTimeoutMs to local/self-hosted models that rely on current no-default-watchdog behavior, then add focused regression tests and redacted real setup proof.

Do we have a high-confidence way to reproduce the issue?

Yes for the PR-introduced regression: source inspection shows the new fallback would set modelRequestTimeoutMs before resolveLlmIdleTimeoutMs reaches the local-provider exemption. The original DeepSeek hang is not high-confidence reproduced from the PR because the body reports only npx tsc --noEmit.

Is this the best way to solve the issue?

No. A global fallback inside resolveProviderRequestTimeoutMs is broader than the existing timeout contract; the safer fix should bound the intended cloud/provider HTTP path while preserving local-provider opt-out behavior.

Full review comments:

  • [P2] Preserve local provider timeout exemption — src/agents/pi-embedded-runner/model.ts:347
    Returning 120_000 for missing provider timeoutSeconds makes configured local providers carry requestTimeoutMs. resolveLlmIdleTimeoutMs treats that value as authoritative before reaching the local baseUrl exemption, so long local Ollama/llama.cpp requests that currently rely on the no-default-watchdog behavior would be aborted at 120s unless users explicitly configure a longer provider timeout.
    Confidence: 0.91

Overall correctness: patch is incorrect
Overall confidence: 0.9

Acceptance criteria:

  • node scripts/run-vitest.mjs src/agents/pi-embedded-runner/model.test.ts src/agents/pi-embedded-runner/run/llm-idle-timeout.test.ts src/agents/provider-transport-fetch.test.ts

What I checked:

  • PR diff adds global fallback timeout: The PR adds DEFAULT_REQUEST_TIMEOUT_MS = 120_000, changes resolveProviderRequestTimeoutMs to return number, and returns the default for missing, invalid, or non-positive provider timeout values. (src/agents/pi-embedded-runner/model.ts:347, 77f1e8cc9017)
  • Current main leaves absent provider timeout unset: On current main, resolveProviderRequestTimeoutMs returns undefined for non-number, non-finite, or non-positive values, and callers only attach requestTimeoutMs when it is defined. (src/agents/pi-embedded-runner/model.ts:355, 862be9fb3d6c)
  • Local provider exemption is downstream: resolveLlmIdleTimeoutMs handles positive modelRequestTimeoutMs before the local-provider no-default-watchdog branch, so the PR's implicit timeout would make local providers hit the 120s path unless users configure a different provider timeout. (src/agents/pi-embedded-runner/run/llm-idle-timeout.ts:151, 862be9fb3d6c)
  • Tests document current timeout policy: Focused tests cover remote provider timeout capping, default idle-watchdog disablement for local base URLs, and honoring explicit local provider request timeouts. (src/agents/pi-embedded-runner/run/llm-idle-timeout.test.ts:46, 862be9fb3d6c)
  • HTTP transport consumes model request timeout metadata: resolveModelRequestTimeoutMs falls back to model.requestTimeoutMs, and buildGuardedModelFetch threads that value into guarded fetch, so the PR changes real provider fetch behavior rather than metadata only. (src/agents/provider-transport-fetch.ts:390, 862be9fb3d6c)
  • Docs describe provider timeout as explicit local/self-hosted tuning: The public timeout docs say provider timeoutSeconds extends slow local/self-hosted model HTTP handling and the idle watchdog when configured; they do not describe an implicit provider-level fallback timeout for all configured providers. Public docs: docs/concepts/agent-loop.md. (docs/concepts/agent-loop.md:169, 862be9fb3d6c)

Likely related people:

  • steipete: Recent history shows multiple timeout-policy and provider-timeout commits in the affected embedded runner and transport surfaces, including local model timeout consolidation and OpenAI SDK request timeout propagation. (role: recent area contributor; confidence: high; commits: e899b32e1d79, fa689295c649, 74211128983a; files: src/agents/pi-embedded-runner/run/llm-idle-timeout.ts, src/agents/pi-embedded-runner/model.ts, src/agents/provider-transport-fetch.ts)
  • liuy: GitHub commit history shows this author introduced the streaming LLM idle-timeout wrapper and resolver surface that now interacts with provider request timeouts. (role: introduced adjacent behavior; confidence: medium; commits: 84b72e66b918; files: src/agents/pi-embedded-runner/run/llm-idle-timeout.ts, src/agents/pi-embedded-runner/run/attempt.ts, src/agents/pi-embedded-runner/run/llm-idle-timeout.test.ts)
  • openperf: Recent Ollama/local-model work touched the same local-provider behavior area that this PR risks changing. (role: recent adjacent contributor; confidence: medium; commits: c1b9af277071; files: extensions/ollama/src/stream.ts, src/agents/pi-embedded-runner/run/attempt.ts, src/agents/pi-embedded-runner/run/llm-idle-timeout.ts)
  • DranboFieldston: Authored guarded dispatcher timeout propagation in the lower-level fetch path that consumes model request timeouts. (role: adjacent timeout contributor; confidence: medium; commits: 977a4b24afd0; files: src/agents/provider-transport-fetch.ts, src/agents/provider-transport-fetch.test.ts, src/infra/net/fetch-guard.ts)

Remaining risk / open question:

  • The claimed DeepSeek/Gateway production hang has no redacted after-fix runtime evidence in the PR; only a typecheck is reported.
  • The blanket fallback can shorten long local Ollama/llama.cpp requests that current main intentionally exempts from the default idle watchdog.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 862be9fb3d6c.

@clawsweeper clawsweeper Bot mentioned this pull request May 11, 2026
Closed
@shakkernerd

shakkernerd commented May 19, 2026

Copy link
Copy Markdown
Member

Closing this as superseded by #83979 and not the right fix shape for the current timeout contract.

Current main already applies the default ~120s model idle/request guard for unset cloud providers, while preserving the local/self-hosted behavior that avoids imposing the default watchdog unless the provider timeout is explicitly configured. The blanket fallback in this PR would set a provider request timeout earlier and risk regressing local/self-hosted providers back to the same class of failure fixed for #77744.

The landed path in #83979 keeps the safer split:

  • unset cloud provider: default ~120s idle/request guard
  • explicit models.providers.<id>.timeoutSeconds: honored as the provider request/idle ceiling
  • shorter run/agent timeout: still wins
  • unset local/self-hosted provider: no implicit 120s watchdog

Closing this PR rather than carrying a stale competing timeout implementation.

@clawsweeper clawsweeper Bot mentioned this pull request Jun 22, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

agents Agent runtime and tooling size: XS triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Andrew199012 @shakkernerd