Skip to content

fix(telegram): align standalone media upload timeout with gateway - #69984

Open
nateEc wants to merge 1 commit into
NousResearch:mainfrom
nateEc:codex/fix-62936-telegram-media-timeout
Open

fix(telegram): align standalone media upload timeout with gateway#69984
nateEc wants to merge 1 commit into
NousResearch:mainfrom
nateEc:codex/fix-62936-telegram-media-timeout

Conversation

@nateEc

@nateEc nateEc commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Summary

  • configure PTB's dedicated media_write_timeout for the standalone Telegram sender used by hermes send, cron, and other out-of-gateway callers
  • use the same fixed 60-second per-socket stall policy as the current gateway adapter
  • apply the timeout to both direct and TELEGRAM_PROXY request transports
  • leave the gateway implementation unchanged and do not add or restore a user-facing environment variable

Why this shape

Current main already fixes media uploads in the gateway and intentionally standardizes that path on a fixed 60-second stall timeout. The remaining gap is the standalone sender in tools/send_message_tool.py, where PTB otherwise retains its 20-second media-write default.

Verification

  • python -m pytest -q tests/tools/test_send_message_telegram_proxy.py — 2 passed
  • uvx ruff check tools/send_message_tool.py tests/tools/test_send_message_telegram_proxy.py
  • git diff --check

Fixes #62936

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/plugins Plugin system and bundled plugins platform/telegram Telegram bot adapter sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Jul 23, 2026
@nateEc nateEc added type/bug Something isn't working comp/plugins Plugin system and bundled plugins platform/telegram Telegram bot adapter P3 Low — cosmetic, nice to have sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Jul 23, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for addressing the real adapter-side PTB timeout split.

Problems

  • The same upload-timeout class remains in standalone delivery: tools/send_message_tool.py:1220-1229 constructs both proxy and direct PTB clients without media_write_timeout. The Telegram plugin delegates out-of-process sends to that function at plugins/platforms/telegram/adapter.py:9849-9858, so cron/standalone media delivery still retains PTB's default.
  • The test insertion point no longer exists on current main: tests/gateway/test_telegram_closewait_limits_31599.py now ends at line 161, while this diff anchors before tests removed after the PR base.

Suggested changes

  • Extend the timeout policy and regression coverage to _send_telegram's direct and proxy construction paths.
  • Rehome the adapter tests onto the current test surface during salvage.

Automated hermes-sweeper review.

@nateEc
nateEc force-pushed the codex/fix-62936-telegram-media-timeout branch from 759e33a to a812a05 Compare July 30, 2026 07:53
@nateEc

nateEc commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the sweeper findings in a812a059f after rebasing onto current main.

  • Added media_write_timeout to the gateway adapter and to standalone direct/proxy HTTPXRequest construction.
  • Rehomed the adapter regressions on the current test surface.
  • Added standalone coverage for both direct and proxy requests.

Validation: 67 focused and adjacent Telegram tests passed; Ruff and git diff --check are clean.

@nateEc
nateEc force-pushed the codex/fix-62936-telegram-media-timeout branch from a812a05 to a34616f Compare July 30, 2026 08:10
@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 30, 2026
@faisfamilytravel

Copy link
Copy Markdown
Contributor

Independent end-to-end validation from a macOS Hermes deployment:

  • Before this commit, a valid 10,591,468-byte H.264/AAC MP4 repeatedly failed with Timed out at approximately 20.009 seconds. Ordinary Telegram API/TCP connectivity was healthy.
  • I cherry-picked a34616f37395df522662482eafd2aa6e16df964e onto current origin/main. 506 Telegram-focused tests passed, plus 969 tests in the controlled release gate; Ruff and the deployment control-plane harness also passed.
  • After a controlled S-6 gateway restart onto that release, the exact same file completed through the standalone hermes send --profile s6 path in 75.470702 seconds and Telegram returned a native message ID.

This provides real-file E2E confirmation that configuring PTB’s upload-specific media_write_timeout resolves the exact 20-second large-media failure. Thank you for covering both the gateway adapter and standalone send path.

@alt-glitch alt-glitch added comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists blocked Waiting on external dependency or decision comp/tools Tool registry, model_tools, toolsets and removed comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have blocked Waiting on external dependency or decision labels Jul 30, 2026
- 让直连与代理发送都显式配置 60 秒媒体写入超时。
- 保持与网关现有策略一致,不新增或恢复用户配置项。
- 覆盖独立发送的直连和代理请求,并保留贡献者归属映射。
- 已通过目标 pytest、Ruff 与差异检查。
@nateEc
nateEc force-pushed the codex/fix-62936-telegram-media-timeout branch from a34616f to 10ccb12 Compare August 19, 2026 06:20
@nateEc nateEc changed the title fix(telegram): extend media upload timeout fix(telegram): align standalone media upload timeout with gateway Aug 19, 2026
@alt-glitch alt-glitch removed the comp/gateway Gateway runner, session dispatch, delivery label Aug 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tools Tool registry, model_tools, toolsets P2 Medium — degraded but workaround exists platform/telegram Telegram bot adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

4 participants