Skip to content

fix(gateway): connect all messaging platforms concurrently with per-platform timeout - #13602

Closed
IamBaoMouMou wants to merge 1 commit into
NousResearch:mainfrom
IamBaoMouMou:fix/concurrent-platform-connect
Closed

fix(gateway): connect all messaging platforms concurrently with per-platform timeout#13602
IamBaoMouMou wants to merge 1 commit into
NousResearch:mainfrom
IamBaoMouMou:fix/concurrent-platform-connect

Conversation

@IamBaoMouMou

Copy link
Copy Markdown

Problem

Gateway connected to messaging platforms sequentially. If one platform (e.g. Discord in China, blocked by GFW) times out (~60s), all other platforms (Telegram, DingTalk, WeChat) were blocked from starting, causing cron jobs to fail silently.

Fix

Connect all platforms concurrently via asyncio.gather(), each with an independent 30s timeout.

Three phases:

  1. Phase 1 (sync): Create all adapters and set up handlers — fast, no I/O
  2. Phase 2 (async, concurrent): Connect all platforms simultaneously with asyncio.wait_for(adapter.connect(), timeout=30.0)
  3. Phase 3 (sync): Process results — successful platforms go live immediately, failed/timeout platforms enter the background reconnect queue

Why this is safe

  • The asyncio.gather(return_exceptions=True) pattern ensures one platform timeout does not cancel others
  • Failed platforms still enter the existing _failed_platforms reconnect queue
  • The asyncio.TimeoutError path calls _safe_adapter_disconnect() for clean resource cleanup
  • Platforms that connect instantly (Telegram, DingTalk, WeChat) are no longer penalized by a single slow platform

Testing

# Verify patch applies cleanly
git apply concurrent-platform-connect.patch
python3 -m py_compile gateway/run.py  # should exit 0

…latform timeout

Problem: Gateway connected to messaging platforms sequentially. If one
platform (e.g. Discord in China, blocked by GFW) times out (~60s), ALL
other platforms (Telegram, DingTalk, WeChat) were blocked from starting,
causing cron jobs to fail silently.

Fix: Connect all platforms concurrently via asyncio.gather(), each with
an independent 30s timeout. A slow or failed platform no longer blocks
other platforms that can connect instantly.

- Add _PLATFORM_CONNECT_TIMEOUT (30s) per-platform connection timeout
- Phase 1: create all adapters synchronously (fast)
- Phase 2: connect all platforms concurrently with asyncio.gather()
- Phase 3: process results — successful platforms go live immediately,
  failed/timeout platforms enter background reconnect queue

Fixes: cron jobs failing when Discord is unreachable in China.
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for targeting a real remaining startup bottleneck. Current main is still serial at gateway/run.py:7019-7060, and the cross-referenced #45397 discussion correctly identifies this PR as the canonical concurrency proposal.

Problems

  • The patch's direct fixed asyncio.wait_for(..., 30) bypasses current gateway/run.py:3430-3466, which now owns gateway.platform_connect_timeout, its env override, and reconnect-call semantics.
  • asyncio.gather() completes before Phase 3 installs any success into self.adapters, so a fast adapter is not marked connected until the slowest task finishes.
  • Current gateway/run.py:7021,7061-7062 aborts startup on restart; tests/gateway/test_startup_restart_race.py:152-186 requires Slack not to connect after Telegram triggers restart. Concurrent tasks need cancellation and teardown handling.
  • The old Phase 1 misses current wiring at gateway/run.py:7047-7049 (topic recovery, authorization check, busy-text mode), and the PR has no concurrency regression test.

Suggested changes

  • Rework the concurrent path against current main, preserving all adapter wiring and _connect_adapter_with_timeout().
  • Add event-based overlap, timeout/queueing, and restart-cancellation tests.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data 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 12, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Closing as superseded: parallel platform connects at gateway startup landed on main via #86673 (salvage of #83809), which keeps the existing per-platform timeout path and error handling and adds a capped Telegram cold-start budget. This PR pioneered the concurrent-startup idea back in April — thank you — but it is 4 months stale against a heavily rewritten startup loop (per-platform runtime status, multiplexing, abort-aware shutdown fencing) and the merged implementation covers its goal.

@teknium1 teknium1 closed this Aug 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery 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 sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants