Skip to content

fix(gateway): surface delayed fetch timeouts#84836

Open
galiniliev wants to merge 2 commits into
openclaw:mainfrom
galiniliev:bug-031-fetch-timeout-starvation-diagnostic
Open

fix(gateway): surface delayed fetch timeouts#84836
galiniliev wants to merge 2 commits into
openclaw:mainfrom
galiniliev:bug-031-fetch-timeout-starvation-diagnostic

Conversation

@galiniliev

@galiniliev galiniliev commented May 21, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Problem: fetchWithTimeout detects late timeout timers, but event-loop starvation evidence is only attached to the warning log.
  • Solution: emit a structured fetch.timeout.delayed diagnostic event whenever the timeout fires late enough to produce the starvation hint.
  • What changed: the diagnostic event payload includes timeoutMs, elapsedMs, timerDelayMs, eventLoopDelayHint, and sanitized operation/url context; the existing timeout test now verifies the event.
  • What did NOT change (scope boundary): timeout behavior, abort reason, log text, URL redaction rules, and fetch call semantics are unchanged.

Motivation

  • A live gateway profile showed a 10s fetch timeout firing after 17.175s while the gateway was CPU-saturated. A structured diagnostic event makes this pressure signal queryable instead of leaving it only in a text log line.

Change Type (select all)

  • Bug fix
  • Feature
  • Refactor required for the fix
  • Docs
  • Security hardening
  • Chore/infra

Scope (select all touched areas)

  • Gateway / orchestration
  • Skills / tool execution
  • Auth / tokens
  • Memory / storage
  • Integrations
  • API / contracts
  • UI / DX
  • CI/CD / infra

Linked Issue/PR

Real behavior proof (required for external PRs)

  • Behavior addressed: delayed fetchWithTimeout timers now emit a structured diagnostic event instead of only a warning log field.
  • Real environment tested: local OpenClaw source checkout on Linux/WSL-style environment with Node v24.15.0, focused timeout test run after applying the patch.
  • Exact steps or command run after this patch: node scripts/run-vitest.mjs src/utils/fetch-timeout.test.ts
  • Evidence after fix (screenshot, recording, terminal capture, console output, redacted runtime log, linked artifact, or copied live output): terminal output captured from the patched branch:
RUN  v4.1.6 /home/galini/GitHub/worktrees/bug-031-fetch-timeout-starvation-diagnostic

Test Files  1 passed (1)
Tests  8 passed (8)
Start at  05:37:39
Duration  559ms (transform 209ms, setup 107ms, import 112ms, tests 51ms, environment 0ms)
  • Observed result after fix: the delayed-timeout regression test observed a fetch.timeout.delayed diagnostic event containing timeoutMs=25, elapsedMs=2025, timerDelayMs=2000, the starvation hint, operation, and sanitized URL.
  • What was not tested: no live saturated gateway/pidstat rerun was performed after this patch; the fix was seam-tested at the timeout helper where the delayed timer is detected.
  • Before evidence (optional but encouraged): redacted live log and profile excerpt from the reported bug:
avg_cpu=83.66
cpu_ge_100_count=42
max_cpu=190.0 at 05:07:47

2026-05-21T05:08:04.306+00:00 fetch-timeout timeoutMs=10000 elapsedMs=17175 timerDelayMs=7175 eventLoopDelayHint="timer delayed 7175ms, likely event-loop starvation" operation=fetchWithTimeout url=https://api.telegram.org/bot[REDACTED]/getMe message="fetch timeout reached; aborting operation"
2026-05-21T05:08:04.316+00:00 diagnostic heartbeat: webhooks=0/0/0 active=1 waiting=0 queued=3

Root Cause (if applicable)

  • Root cause: the timeout helper already computed timer lateness but only copied it into the warning log record.
  • Missing detection / guardrail: existing coverage asserted the log fields but did not assert any structured diagnostic event for downstream diagnostics consumers.
  • Contributing context (if known): timeout drift is a pressure signal from the event loop itself, so it is easy to miss if support tooling only reads diagnostic events/stability snapshots.

Regression Test Plan (if applicable)

  • Coverage level that should have caught this:
    • Unit test
    • Seam / integration test
    • End-to-end test
    • Existing coverage already sufficient
  • Target test or file: src/utils/fetch-timeout.test.ts
  • Scenario the test should lock in: when the timeout timer fires late enough to trigger the starvation hint, a fetch.timeout.delayed diagnostic event is emitted with sanitized context.
  • Why this is the smallest reliable guardrail: delayed timer detection and URL sanitization both happen inside buildTimeoutAbortSignal / abortDueToTimeout.
  • Existing test that already covers this (if any): the existing late-timeout test covered log fields only.
  • If no new test is added, why not: N/A

User-visible / Behavior Changes

Diagnostics consumers can observe fetch.timeout.delayed events for delayed timeout timers. Fetch timeout behavior and log output stay the same.

Diagram (if applicable)

Before:
timeout timer fires late -> warning log only

After:
timeout timer fires late -> warning log + fetch.timeout.delayed diagnostic event

Security Impact (required)

  • New permissions/capabilities? (Yes/No) No
  • Secrets/tokens handling changed? (Yes/No) No
  • New/changed network calls? (Yes/No) No
  • Command/tool execution surface changed? (Yes/No) No
  • Data access scope changed? (Yes/No) No
  • If any Yes, explain risk + mitigation: N/A

Repro + Verification

Environment

  • OS: Linux/WSL-style local source checkout
  • Runtime/container: Node v24.15.0
  • Model/provider: N/A for timeout helper seam test
  • Integration/channel (if any): Gateway fetch timeout helper
  • Relevant config (redacted): N/A

Steps

  1. Apply the patch.
  2. Run node scripts/run-vitest.mjs src/utils/fetch-timeout.test.ts.
  3. Inspect the delayed-timeout diagnostic event assertion.

Expected

  • Late timeout timers still log the existing warning fields.
  • Late timeout timers also emit fetch.timeout.delayed with sanitized operation/url context.

Actual

  • The focused test file passed with 8 tests.

Evidence

Attach at least one:

  • Failing test/log before + passing after
  • Trace/log snippets
  • Screenshot/recording
  • Perf numbers (if relevant)

Human Verification (required)

What you personally verified (not just CI), and how:

  • Verified scenarios: focused timeout test; git diff --check; Codex autoreview reported no accepted/actionable findings.
  • Edge cases checked: URL remains sanitized in the diagnostic event; normal abort behavior remains covered by existing tests.
  • What you did not verify: live gateway under CPU saturation after this patch.

Review Conversations

  • I replied to or resolved every bot review conversation I addressed in this PR.
  • I left unresolved only the conversations that still need reviewer or maintainer judgment.

Compatibility / Migration

  • Backward compatible? (Yes/No) Yes
  • Config/env changes? (Yes/No) No
  • Migration needed? (Yes/No) No
  • If yes, exact upgrade steps: N/A

Risks and Mitigations

  • Risk: the new diagnostic event could expose sensitive URL query strings if it bypassed existing redaction.
    • Mitigation: the event uses the same sanitizeTimeoutLogUrl output as the existing warning log, and the regression test asserts the sanitized URL.

@openclaw-barnacle openclaw-barnacle Bot added size: XS maintainer Maintainer-authored PR labels May 21, 2026
@clawsweeper

clawsweeper Bot commented May 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 1, 2026, 2:49 AM ET / 06:49 UTC.

Summary
The PR emits a new fetch.timeout.delayed diagnostic event from fetchWithTimeout, projects it into stability/CLI/diagnostics-otel, and adds regression tests.

PR surface: Source +105, Tests +182. Total +287 across 11 files.

Reproducibility: yes. for the source-level path: current main computes and logs delayed timeout drift, while the diagnostic event union has no fetch.timeout.delayed event. I did not rerun a live CPU-saturated gateway scenario in this read-only review.

Review metrics: 1 noteworthy metric.

  • Diagnostic Event Surface: 1 public event type added. DiagnosticEventPayload is exported through plugin SDK diagnostic APIs, so the new event shape and payload safety matter before merge.

Stored data model
Persistent data-model change detected: serialized state: src/logging/diagnostic-stability-bundle.test.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #84835
Summary: This PR is the open implementation candidate for the canonical missing structured delayed fetch-timeout diagnostic event; broader event-loop starvation reports overlap symptoms but do not own this exact event contract.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🧂 unranked krab
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Remove URL data from the raw delayed-timeout event or fully redact path-embedded secrets, with a Telegram Bot API token-path regression.
  • [P1] Add redacted after-fix proof from a real gateway, stability snapshot, OTel export, or support-log surface.
  • Refresh the branch against current main and preserve current timeout-delay and diagnostic-event infrastructure changes.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body provides copied output from a focused unit test only; it still needs redacted after-fix proof from a real gateway, stability snapshot, OTel export, or support-log surface, with private details such as IPs, API keys, phone numbers, endpoints, and bot tokens redacted before updating the PR body for re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • [P2] The raw fetch.timeout.delayed event can carry a URL whose path is preserved, and Telegram Bot API tokens are path-embedded; public diagnostic listeners receive the raw event before stability, CLI, or OTel projections drop URL data.
  • [P1] The PR adds a plugin-observable DiagnosticEventPayload variant, so maintainers need to accept the event shape, cardinality expectations, and compatibility impact before merge.
  • [P1] The contributor proof is a focused unit-test terminal run, not redacted after-fix proof from a real gateway, stability snapshot, OTel export, or support-log surface.
  • [P2] GitHub reports the branch as conflicting against main; the refresh needs to preserve current safe timeout delay handling and newer diagnostic event infrastructure.

Maintainer options:

  1. Ship A Listener-Safe Payload (recommended)
    Before merge, remove URL data from the raw fetch.timeout.delayed event or fully redact path-embedded secrets, then add a Telegram Bot API token-path regression.
  2. Accept The Diagnostic API Contract
    Maintainers can explicitly accept the new plugin-observable event variant and low-cardinality field contract after reviewing the final payload shape.
  3. Pause Until Proof And Refresh
    Keep the PR open but blocked until the branch is mergeable and the contributor adds redacted after-fix proof from a real diagnostic surface.

Next step before merge

  • [P1] Manual maintainer review is needed because the PR has a protected maintainer label, a public diagnostic-event contract decision, a concrete security payload issue, missing real behavior proof, and a merge conflict.

Security
Needs attention: Needs attention because the raw diagnostic event can carry path-preserved URL data to plugin-visible listeners.

Review findings

  • [P1] Drop URL data from delayed-timeout diagnostic events — src/utils/fetch-timeout.ts:99
Review details

Best possible solution:

Land a refreshed, listener-safe delayed-timeout diagnostic event that omits raw URL data or fully redacts path-embedded secrets, keeps low-cardinality projections, and has maintainer-accepted event shape plus real diagnostic proof.

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

Yes for the source-level path: current main computes and logs delayed timeout drift, while the diagnostic event union has no fetch.timeout.delayed event. I did not rerun a live CPU-saturated gateway scenario in this read-only review.

Is this the best way to solve the issue?

No as submitted. Emitting from the timeout helper is the right seam, but the raw event must be listener-safe, the public event shape needs maintainer acceptance, proof needs a real diagnostic surface, and the branch needs a refresh.

Full review comments:

  • [P1] Drop URL data from delayed-timeout diagnostic events — src/utils/fetch-timeout.ts:99
    sanitizeTimeoutLogUrl preserves URL path segments, and Telegram Bot API tokens are path-embedded. This event is delivered to public diagnostic listeners before stability, CLI, or OTel projections remove the URL, so delayed Telegram timeouts can expose bot tokens unless the raw event omits the URL or uses a sensitive redactor with a regression test.
    Confidence: 0.9

Overall correctness: patch is incorrect
Overall confidence: 0.9

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against b2355ef6a226.

Label changes

Label justifications:

  • P2: This is a focused gateway diagnostics improvement with a clear linked issue and limited runtime blast radius, but it is not an emergency outage fix by itself.
  • merge-risk: 🚨 compatibility: The PR adds a new plugin-observable diagnostic event variant, making the event shape a public compatibility contract.
  • merge-risk: 🚨 security-boundary: The new raw diagnostic event can carry URL path data that may include Telegram Bot API credentials before downstream projections remove it.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body provides copied output from a focused unit test only; it still needs redacted after-fix proof from a real gateway, stability snapshot, OTel export, or support-log surface, with private details such as IPs, API keys, phone numbers, endpoints, and bot tokens redacted before updating the PR body for re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +105, Tests +182. Total +287 across 11 files.

View PR surface stats
Area Files Added Removed Net
Source 6 105 0 +105
Tests 5 185 3 +182
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 11 290 3 +287

Security concerns:

  • [medium] Raw diagnostic event may expose URL path tokens — src/utils/fetch-timeout.ts:99
    The timeout log sanitizer preserves URL paths, while Telegram Bot API tokens are path-embedded and the raw diagnostic event is sent to public listeners before stability, CLI, or OTel projections remove URL data.
    Confidence: 0.9

What I checked:

  • Repository policy applied: Root AGENTS.md and the extensions scoped guide were read; their protected-label, plugin API compatibility, proof, and security-review guidance apply because the PR changes core diagnostics, CLI output, and a bundled plugin. (AGENTS.md:1, b2355ef6a226)
  • Current main is log-only: Current abortDueToTimeout computes timerDelayMs and eventLoopDelayHint, then writes them only to the warning log before aborting. (src/utils/fetch-timeout.ts:70, b2355ef6a226)
  • Current main lacks requested event: A source search for fetch.timeout.delayed in current src, extensions, packages, and docs returned no matches. (src/infra/diagnostic-events.ts:744, b2355ef6a226)
  • Latest release is also log-only: The v2026.6.11 tag still logs delayed timeout fields from abortDueToTimeout and has no fetch.timeout.delayed event in the diagnostic event union snippet inspected. (src/utils/fetch-timeout.ts:70, e085fa1a3ffd)
  • PR emits URL on the raw event: The PR adds url?: string to DiagnosticFetchTimeoutDelayedEvent and emits sanitizedUrl into the raw diagnostic event payload when the timeout timer fires late. (src/utils/fetch-timeout.ts:91, 00058145c8f7)
  • URL sanitizer preserves path segments: The timeout URL sanitizer strips credentials, query, and fragment, but keeps the parsed path, which is not sufficient for APIs with path-embedded tokens. (src/utils/fetch-timeout.ts:28, 00058145c8f7)

Likely related people:

  • quotentiroler: Git history shows this person consolidated fetchWithTimeout into the shared utility that this PR extends. (role: shared utility introducer; confidence: medium; commits: a26670a2fb87; files: src/utils/fetch-timeout.ts)
  • Kevin Lin: Git history shows this person added fetch timeout abort logging on the same helper surface. (role: timeout logging contributor; confidence: medium; commits: f35196117331; files: src/utils/fetch-timeout.ts, src/utils/fetch-timeout.test.ts)
  • Vincent Koc: Recent commits by this person changed diagnostic event and stability projection surfaces related to this PR's event contract. (role: recent diagnostics area contributor; confidence: high; commits: 6456d03868d0, 49e95c53083d; files: src/infra/diagnostic-events.ts, src/logging/diagnostic-stability.ts)
  • Galin Iliev: This person authored the open candidate PR and also has merged adjacent diagnostic event drain work in the same area. (role: linked fix author and adjacent diagnostics contributor; confidence: medium; commits: 00058145c8f7, 5d799c2d2052; files: src/utils/fetch-timeout.ts, src/infra/diagnostic-events.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. labels May 21, 2026
@clawsweeper

clawsweeper Bot commented May 21, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper PR egg

🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat.

Where did the egg go?
  • The egg game starts only after the PR passes the real-behavior proof check.
  • Before that, no creature or rarity is rolled. The treat waits for real proof.
  • This is still just collectible flavor: proof affects review readiness, not creature quality.

@clawsweeper clawsweeper Bot mentioned this pull request May 21, 2026
Open
@galiniliev galiniliev self-assigned this May 23, 2026
@openclaw-barnacle openclaw-barnacle Bot added extensions: diagnostics-otel Extension: diagnostics-otel cli CLI command changes size: M and removed size: XS labels May 23, 2026
@clawsweeper clawsweeper Bot added the merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. label May 23, 2026
@Malzbier

Malzbier commented Jun 7, 2026

Copy link
Copy Markdown

it looks like the test errors are unrealted the the actual cahnges. could it be that the test are touteded / relay on a older llm model api ?

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 20, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jun 21, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@galiniliev galiniliev

Labels

cli CLI command changes extensions: diagnostics-otel Extension: diagnostics-otel maintainer Maintainer-authored PR merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: M status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: event-loop starvation delays fetch timeouts during gateway load

2 participants

@galiniliev @Malzbier