From 0ffee85d17fbaa76ff6047a6b824bfbf2a7f7531 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BE=90=E9=97=BB=E6=B6=B50668001344?= Date: Fri, 19 Jun 2026 13:23:44 +0800 Subject: [PATCH] fix(telegram): classify sendChatAction 401 by structured error_code, not bare substring match is401Error used message.includes("401") before the transient-error branch, so a structured 429 rate-limit whose grammY-rendered message contains the substring "401" (e.g. "retry after 401") was miscounted as a consecutive 401, eventually suspending all chat actions and logging a CRITICAL "token likely invalid, Telegram may DELETE the bot" alarm for a perfectly valid bot. Check the structured Telegram error_code field first: error_code === 401 is a 401; any other structured code (429/5xx/4xx) is not. Remove the bare "401" substring check entirely (it was the root cause). Keep "unauthorized" case-insensitive matching only as a fallback for non-Telegram errors that lack a structured error_code. This matches the classification pattern already used by sibling classifiers in network-errors.ts (hasTelegramErrorCode-based isTelegramRateLimitError, isTelegramServerError, etc.) and follows the fix-shape guidance: the right layer to classify a Telegram API error is the structured error_code, not the rendered message string. Closes #94787 --- .../src/sendchataction-401-backoff.test.ts | 90 +++++++++++++++++++ .../src/sendchataction-401-backoff.ts | 20 ++++- 2 files changed, 107 insertions(+), 3 deletions(-) diff --git a/extensions/telegram/src/sendchataction-401-backoff.test.ts b/extensions/telegram/src/sendchataction-401-backoff.test.ts index fe8f832c43da..f6687950602a 100644 --- a/extensions/telegram/src/sendchataction-401-backoff.test.ts +++ b/extensions/telegram/src/sendchataction-401-backoff.test.ts @@ -301,4 +301,94 @@ describe("createTelegramSendChatActionHandler", () => { await handler.sendChatAction(444, "typing"); expect(fn).toHaveBeenCalledTimes(3); }); + + it("treats a structured 429 with retry_after=401 as transient, not as a 401 suspension", async () => { + // grammY renders this as: "Call to 'sendChatAction' failed! (429: Too Many Requests: retry after 401)" + // The substring "401" in the message must NOT trigger the 401 suspension path. + const make429WithRetryAfter401 = () => + Object.assign( + new Error("Call to 'sendChatAction' failed! (429: Too Many Requests: retry after 401)"), + { error_code: 429, parameters: { retry_after: 401 } }, + ); + + let now = 0; + const fn = vi.fn().mockRejectedValue(make429WithRetryAfter401()); + const logger = vi.fn(); + const handler = createTelegramSendChatActionHandler({ + sendChatActionFn: fn, + logger, + maxConsecutive401: 3, + now: () => now, + }); + + // All calls should fail but as transient errors, NOT 401 errors + await expect(handler.sendChatAction(123, "typing")).rejects.toThrow("429"); + // Advance past the transient cooldown (retry_after=401 → 401000ms) + now += 402_000; + await expect(handler.sendChatAction(123, "typing")).rejects.toThrow("429"); + now += 402_000; + await expect(handler.sendChatAction(123, "typing")).rejects.toThrow("429"); + + // Handler must NOT be suspended — this is a transient 429, not a 401 + expect(handler.isSuspended()).toBe(false); + + // Must NOT have logged the CRITICAL token-deletion alarm + const criticalLogs = logger.mock.calls.filter(([msg]) => String(msg).includes("CRITICAL")); + expect(criticalLogs).toEqual([]); + }); + + it("detects a structured Telegram 401 error by error_code, not by message substring", async () => { + const makeStructured401 = () => Object.assign(new Error("Unauthorized"), { error_code: 401 }); + + const fn = vi.fn().mockRejectedValue(makeStructured401()); + const logger = vi.fn(); + const handler = createTelegramSendChatActionHandler({ + sendChatActionFn: fn, + logger, + maxConsecutive401: 2, + }); + + await expect(handler.sendChatAction(123, "typing")).rejects.toThrow("Unauthorized"); + await expect(handler.sendChatAction(123, "typing")).rejects.toThrow("Unauthorized"); + + expect(handler.isSuspended()).toBe(true); + expect(logger.mock.calls.at(-1)).toEqual([ + "CRITICAL: sendChatAction suspended after 2 consecutive 401 errors. Bot token is likely invalid. Telegram may DELETE the bot if requests continue. Replace the token and restart: openclaw channels restart telegram", + ]); + }); + + it("does not misclassify a plain error whose message contains '401' as a 401 error", async () => { + // A plain Error (no error_code) with "401" in its message should NOT + // trigger the 401 path, since bare substring matching was the root cause + // of #94787. Only "unauthorized" is the fallback for non-Telegram errors. + const fn = vi.fn().mockRejectedValue(new Error("401 Too Many Requests: retry after")); + const logger = vi.fn(); + const handler = createTelegramSendChatActionHandler({ + sendChatActionFn: fn, + logger, + maxConsecutive401: 3, + }); + + // This is neither a recognized 401 nor a recognized transient → falls to + // the "else" branch (clears transient cooldown, re-throws). + await expect(handler.sendChatAction(123, "typing")).rejects.toThrow("401"); + expect(handler.isSuspended()).toBe(false); + }); + + it("recognizes non-Telegram 401 via 'unauthorized' in message as a 401", async () => { + // A plain Error without error_code but with "unauthorized" should still + // be classified as 401 (this is the intentional fallback path). + const fn = vi.fn().mockRejectedValue(new Error("Unauthorized access")); + const logger = vi.fn(); + const handler = createTelegramSendChatActionHandler({ + sendChatActionFn: fn, + logger, + maxConsecutive401: 2, + }); + + await expect(handler.sendChatAction(123, "typing")).rejects.toThrow("Unauthorized"); + await expect(handler.sendChatAction(123, "typing")).rejects.toThrow("Unauthorized"); + + expect(handler.isSuspended()).toBe(true); + }); }); diff --git a/extensions/telegram/src/sendchataction-401-backoff.ts b/extensions/telegram/src/sendchataction-401-backoff.ts index 8ee1c9877b01..2a607e183e40 100644 --- a/extensions/telegram/src/sendchataction-401-backoff.ts +++ b/extensions/telegram/src/sendchataction-401-backoff.ts @@ -69,10 +69,24 @@ function is401Error(error: unknown): boolean { if (!error) { return false; } + // When a structured Telegram error_code is present, trust it exclusively. + // A 429 with retry_after=401 renders as "(429: Too Many Requests: retry after 401)" + // whose message contains the substring "401" — that must NOT trigger the 401 + // suspension path. The sibling classifiers in network-errors.ts also use + // error_code before message heuristics; see hasTelegramErrorCode. + if ( + typeof error === "object" && + error !== null && + "error_code" in error && + typeof (error as { error_code: unknown }).error_code === "number" + ) { + return (error as { error_code: number }).error_code === 401; + } + // Fallback for non-Telegram errors without a structured error_code: + // match "unauthorized" case-insensitively, but do NOT use bare "401" + // substring matching — that was the root cause of #94787. const message = error instanceof Error ? error.message : JSON.stringify(error); - return ( - message.includes("401") || normalizeLowercaseStringOrEmpty(message).includes("unauthorized") - ); + return normalizeLowercaseStringOrEmpty(message).includes("unauthorized"); } class TelegramSendChatActionTransientCooldownError extends Error {