Skip to content

fix(weixin): broaden stale-session detection and strip context_token on rate-limit fallback - #22661

Open
rj-chenlinfeng wants to merge 2 commits into
NousResearch:mainfrom
rj-chenlinfeng:fix/weixin-stale-session-detection
Open

fix(weixin): broaden stale-session detection and strip context_token on rate-limit fallback#22661
rj-chenlinfeng wants to merge 2 commits into
NousResearch:mainfrom
rj-chenlinfeng:fix/weixin-stale-session-detection

Conversation

@rj-chenlinfeng

Copy link
Copy Markdown

Summary

Two bugs caused iLink errcode=-2 to be misidentified as genuine rate limiting when it was actually a stale/expired session, leading to infinite retries that always fail.

Bug 1: _is_stale_session_ret() only recognized "unknown error"

iLink uses errcode=-2 for both rate limiting and session expiry. The function only matched errmsg == "unknown error", but iLink also returns empty strings, "session expired", "token expired", and other locale-dependent variants — all misclassified as rate-limit errors.

Fix: Match empty errmsg, "unknown error", and any string containing "expire".

Bug 2: Rate-limit retry branch never cleared context_token

When errcode=-2 slipped through _is_stale_session_ret(), every retry carried the same expired token and always got -2 again — a dead loop until the retry budget was exhausted.

Fix: Strip context_token on the first rate-limit hit (with retried_without_token guard), same as the explicit session-expired branch. This provides a second safety net: even if the errmsg doesn't match any known pattern, the degraded tokenless retry can still succeed.

Files Changed

  • gateway/platforms/weixin.py — broaden _is_stale_session_ret() + strip context_token in rate-limit fallback

Test Plan

  • Verified existing tests still pass
  • Tested with real iLink session expiry scenario (errmsg variants)

chenlinfeng added 2 commits May 9, 2026 23:37
…on rate-limit fallback

Two bugs caused iLink errcode=-2 to be misidentified as genuine rate
limiting when it was actually a stale/expired session, leading to
infinite retries that always fail.

Bug 1: _is_stale_session_ret() only recognized errmsg "unknown error"
as a session-expiry signal, but iLink also returns empty strings,
"session expired", "token expired", etc. with errcode=-2.  These
were all misclassified as rate-limit errors.

  Before: (errmsg or "").lower() == "unknown error"
  After:  also match empty errmsg, "unknown error", and any string
          containing "expire" (covers locale-dependent variants)

Bug 2: The rate-limit retry branch in _send_text_chunk() never cleared
context_token, so if errcode=-2 was actually a stale session that
slipped through _is_stale_session_ret, every retry would carry the
same expired token and always get -2 again — a dead loop until the
retry budget was exhausted.

  Fix: strip context_token on the first rate-limit hit (with
  retried_without_token guard), same as the explicit session-expired
  branch.  This provides a second safety net: even if the errmsg
  doesn't match any known pattern, the degraded tokenless retry can
  still succeed.

Together these changes ensure that stale iLink sessions are recovered
reliably regardless of the exact errmsg wording returned by the server.
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for investigating the ambiguous iLink -2 responses. The requested behavior is not present on current main: gateway/platforms/weixin.py:99-107 still recognizes only "unknown error", and the unmatched -2 send path retains the token at gateway/platforms/weixin.py:1794-1813.

Problems

  • Commit db8bfc7a0e14 removes 12 unrelated CI/release workflows and must not be included in a focused salvage.
  • The blank-error behavior conflicts with current coverage: tests/gateway/test_weixin.py:926-928 asserts blank errmsg is not stale, but this PR changes no tests.
  • Main has since added a default-threshold-one rate-limit circuit (gateway/platforms/weixin.py:1179-1185). It breaks immediately after _record_rate_limit_event() (gateway/platforms/weixin.py:1802-1804), before this PR's proposed fallback location, so the fallback needs reordering and coverage on current main.

Suggested changes

  • Keep only the Weixin work, add protocol-backed variant tests, and place any tokenless fallback before the rate-limit circuit can suppress it.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-automation Sweeper risk: may affect CI, automerge, label sync, or maintainer automation sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit area/sessions Session lifecycle, resume, persistence, history labels Jul 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/sessions Session lifecycle, resume, persistence, history comp/gateway Gateway runner, session dispatch, delivery P3 Low — cosmetic, nice to have platform/wecom WeCom / WeChat Work adapter sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-automation Sweeper risk: may affect CI, automerge, label sync, or maintainer automation 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.

3 participants