Skip to content

fix(dingtalk): monkey-patch DingTalkStreamClient.start for websockets >= 11 - #25024

Closed
eipiem1 wants to merge 1 commit into
NousResearch:mainfrom
eipiem1:fix/dingtalk-websockets11-compat
Closed

eipiem1 wants to merge 1 commit into
NousResearch:mainfrom
eipiem1:fix/dingtalk-websockets11-compat

Conversation

@eipiem1

@eipiem1 eipiem1 commented May 13, 2026

Copy link
Copy Markdown

Summary

dingtalk-stream (<=0.24.3) uses async with websockets.connect(uri) which breaks with websockets >= 11 because websockets.connect() became a coroutine function and must be awaited first.

This PR adds a monkey-patch in gateway/platforms/dingtalk.py that replaces DingTalkStreamClient.start with a fixed version using async with await websockets.connect(uri) — the only change from upstream.

Why not pin websockets?

dingtalk-stream requires websockets >= 11.0.2, but all versions >= 11 have connect() as a coroutine, so pinning alone cannot fix this. A runtime monkey-patch is the only self-contained fix.

How it works

The patch is applied at module import time in the DingTalk adapter. It can be removed cleanly once dingtalk-stream releases a fix upstream.

Testing

  • ✅ Gateway starts without the coroutine TypeError
  • ✅ DingTalk connects and stays connected (tested 40+ min after fix)
  • ✅ Other platforms (Weixin, Feishu, Yuanbao, Webhook) unaffected

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists platform/dingtalk DingTalk adapter comp/gateway Gateway runner, session dispatch, delivery labels May 13, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

Related to #24851 (reconnection storm caused by the same websockets.connect coroutine TypeError) and #17364 (monkey-patch isolation from the other direction). This PR fixes the root cause that #24851 works around with a circuit breaker.

@zhangwhite013-star

zhangwhite013-star commented May 14, 2026

Copy link
Copy Markdown

+1 — Confirmed hitting the same TypeError: 'coroutine' object does not support the asynchronous context manager protocol error with dingtalk-stream 0.24.3 + websockets 15.0.1.

Symptoms: DingTalk gateway completely broken — stream client fails to connect, gateway logs show the coroutine TypeError on every reconnect attempt. Other platforms (Feishu, Weixin) work fine.

Fix verified: Applied a similar monkey-patch locally in gateway/platforms/dingtalk.py (replacing async with websockets.connect(uri) with async with await websockets.connect(uri)) and DingTalk stream connection works perfectly after gateway restart. No more coroutine errors, connection stays stable.

This is a critical fix — DingTalk gateway is completely unusable without it for anyone on websockets >= 11. Would be great to get this merged soon! 🙏

@eipiem1
eipiem1 force-pushed the fix/dingtalk-websockets11-compat branch from f5a99b6 to e51f6bc Compare May 14, 2026 10:43
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for investigating the DingTalk reconnection failure. This is now covered on current main by fixing the shared Feishu wrapper that produced the coroutine/context-manager mismatch.

Automated hermes-sweeper review evidence:

  • plugins/platforms/feishu/adapter.py:1319-1324 uses a synchronous _connect_with_overrides() and returns original_connect(...) directly, preserving the async-context-manager contract.
  • Commit 71191b7e8e075037a814f77d37d4609e97f12029 made that root-cause fix; it is included in v2026.5.16.
  • The repository pins websockets==15.0.1 (pyproject.toml:107); direct inspection confirms websockets.connect() returns an async context manager, so dingtalk-stream 0.24.3's existing async with websockets.connect(...) remains valid.
  • This also matches the prior investigation in fix(dingtalk): isolate websockets.connect monkey-patch from DingTalk stream SDK #17364, which identified the Feishu wrapper as the source of the broken shared connector behavior.

@teknium1 teknium1 closed this Jul 13, 2026
@teknium1 teknium1 added sweeper:implemented-on-main Sweeper: behavior already present on current main sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages 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 13, 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 P2 Medium — degraded but workaround exists platform/dingtalk DingTalk adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:implemented-on-main Sweeper: behavior already present on current main 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

Development

Successfully merging this pull request may close these issues.

4 participants