diff --git a/extensions/feishu/src/comment-shared.ts b/extensions/feishu/src/comment-shared.ts index cbacb475bb8..1c9bd892f86 100644 --- a/extensions/feishu/src/comment-shared.ts +++ b/extensions/feishu/src/comment-shared.ts @@ -118,6 +118,23 @@ export function getFeishuSendRateLimitCode(error: unknown): number | undefined { return typeof code === "number" && FEISHU_SEND_RATE_LIMIT_CODES.has(code) ? code : undefined; } +/** + * Returns a retryable rate-limit code when a fulfilled (non-throwing) Feishu + * SDK response embeds it in the response body. The Feishu node SDK can resolve + * with `{ code: 11232, msg: "..." }` instead of throwing — see typing.ts + * (getBackoffCodeFromResponse) and issue #28157 for the same behavior on + * messageReaction.create. Without this classification, requestFeishuApi would + * `return` the rate-limited body and downstream `assertFeishuMessageApiSuccess` + * would fail once with no retry. + */ +export function getFeishuSendRateLimitCodeFromResponse(response: unknown): number | undefined { + if (!isRecord(response)) { + return undefined; + } + const code = (response as { code?: unknown }).code; + return typeof code === "number" && FEISHU_SEND_RATE_LIMIT_CODES.has(code) ? code : undefined; +} + export async function requestFeishuApi( request: () => Promise, errorPrefix: string, @@ -129,6 +146,7 @@ export async function requestFeishuApi( } = {}, ): Promise { const retryDelayMs = options.retryDelayMs ?? FEISHU_SEND_RETRY_BASE_MS; + let lastFulfilledRateLimit: { response: unknown; code: number } | undefined; for (let attempt = 0; attempt <= FEISHU_SEND_MAX_RETRIES; attempt++) { if (attempt > 0) { // Linear backoff: delay grows with each attempt to give the rate-limit window time to reset. @@ -137,7 +155,15 @@ export async function requestFeishuApi( }); } try { - return await request(); + const result = await request(); + // Feishu SDK may fulfill with a rate-limit body (e.g. { code: 11232, ... }) + // instead of throwing. Classify before returning so retry covers both shapes. + const fulfilledRateLimit = getFeishuSendRateLimitCodeFromResponse(result); + if (fulfilledRateLimit !== undefined && attempt < FEISHU_SEND_MAX_RETRIES) { + lastFulfilledRateLimit = { response: result, code: fulfilledRateLimit }; + continue; + } + return result; } catch (error) { const isRetryable = attempt < FEISHU_SEND_MAX_RETRIES && getFeishuSendRateLimitCode(error) !== undefined; @@ -147,6 +173,16 @@ export async function requestFeishuApi( // Rate-limit on a non-final attempt — loop continues to next retry. } } + // Exhausted retries while the SDK kept fulfilling rate-limit bodies. Surface + // the last response as an error so callers see the same wrapped shape they + // would have seen if the SDK had thrown. + if (lastFulfilledRateLimit) { + const synthetic = Object.assign( + new Error(`Request fulfilled with rate-limit code ${lastFulfilledRateLimit.code}`), + { response: { status: 200, data: lastFulfilledRateLimit.response } }, + ); + throw createFeishuApiError(synthetic, errorPrefix, options); + } // Unreachable: every iteration either returns or throws. Required for TypeScript exhaustiveness. throw createFeishuApiError(new Error("unreachable"), errorPrefix, options); } diff --git a/extensions/feishu/src/send.retry.test.ts b/extensions/feishu/src/send.retry.test.ts index 9a76733f433..65f6b7b9e38 100644 --- a/extensions/feishu/src/send.retry.test.ts +++ b/extensions/feishu/src/send.retry.test.ts @@ -6,7 +6,11 @@ */ import { describe, expect, it, vi } from "vitest"; -import { getFeishuSendRateLimitCode, requestFeishuApi } from "./comment-shared.js"; +import { + getFeishuSendRateLimitCode, + getFeishuSendRateLimitCodeFromResponse, + requestFeishuApi, +} from "./comment-shared.js"; /** Build an AxiosError-shaped object for a given Feishu body error code (HTTP 400). */ function axiosError(code: number) { @@ -219,3 +223,101 @@ describe("requestFeishuApi — retry on expanded rate-limit signals", () => { expect(request).toHaveBeenCalledTimes(3); }); }); + +// Feishu SDK can fulfill (no throw) with a rate-limit code in the body, e.g. +// `{ code: 11232, msg: "..." }`. Same shape the typing path already handles +// via getBackoffCodeFromResponse — see issue #28157. +describe("getFeishuSendRateLimitCodeFromResponse — fulfilled body classification", () => { + it("returns 230020 for a fulfilled per-chat rate-limit body", () => { + expect(getFeishuSendRateLimitCodeFromResponse({ code: 230020, msg: "rate limit" })).toBe( + 230020, + ); + }); + + it("returns 11232 for a fulfilled tenant-level rate-limit body", () => { + expect(getFeishuSendRateLimitCodeFromResponse({ code: 11232, msg: "rate limit" })).toBe(11232); + }); + + it("returns undefined for a successful body (code=0)", () => { + expect(getFeishuSendRateLimitCodeFromResponse({ code: 0, data: {} })).toBeUndefined(); + }); + + it("returns undefined for a non-rate-limit error body", () => { + expect(getFeishuSendRateLimitCodeFromResponse({ code: 230001 })).toBeUndefined(); + }); + + it("returns undefined for null / non-object", () => { + expect(getFeishuSendRateLimitCodeFromResponse(null)).toBeUndefined(); + expect(getFeishuSendRateLimitCodeFromResponse(undefined)).toBeUndefined(); + expect(getFeishuSendRateLimitCodeFromResponse("oops")).toBeUndefined(); + }); +}); + +describe("requestFeishuApi — retry on fulfilled rate-limit body (no throw)", () => { + // Mirrors the typing-path fix from #28157: SDK resolves with the rate-limit + // code instead of throwing, and the helper must classify before returning. + it("retries when SDK fulfills with code 11232 then succeeds", async () => { + const request = vi + .fn() + .mockResolvedValueOnce({ code: 11232, msg: "rate limit", data: null }) + .mockResolvedValueOnce({ code: 0, data: { message_id: "om_ok" } }); + + const result = await requestFeishuApi(request, "prefix", NO_DELAY); + expect(result).toEqual({ code: 0, data: { message_id: "om_ok" } }); + expect(request).toHaveBeenCalledTimes(2); + }); + + it("retries when SDK fulfills with code 230020 then succeeds", async () => { + const request = vi + .fn() + .mockResolvedValueOnce({ code: 230020, msg: "rate limit" }) + .mockResolvedValueOnce({ code: 0, data: { message_id: "om_ok2" } }); + + const result = await requestFeishuApi(request, "prefix", NO_DELAY); + expect((result as { data: { message_id: string } }).data.message_id).toBe("om_ok2"); + expect(request).toHaveBeenCalledTimes(2); + }); + + it("exhausts retries when SDK keeps fulfilling 11232 and throws wrapped error", async () => { + const request = vi.fn().mockResolvedValue({ code: 11232, msg: "rate limit" }); + + const err = await requestFeishuApi(request, "Feishu send failed", NO_DELAY).catch( + (e: unknown) => e, + ); + expect(err).toBeInstanceOf(Error); + expect((err as Error).message).toMatch(/Feishu send failed/); + expect((err as Error).message).toMatch(/11232/); + // 1 initial attempt + 2 retries + expect(request).toHaveBeenCalledTimes(3); + }); + + it("does not retry when SDK fulfills with a successful response (code=0)", async () => { + const request = vi.fn().mockResolvedValue({ code: 0, data: { message_id: "om_first" } }); + + const result = await requestFeishuApi(request, "prefix", NO_DELAY); + expect((result as { data: { message_id: string } }).data.message_id).toBe("om_first"); + expect(request).toHaveBeenCalledTimes(1); + }); + + it("does not retry when SDK fulfills with a non-rate-limit error code", async () => { + // Non-retryable error codes pass through to assertFeishuMessageApiSuccess upstream. + const fulfilled = { code: 230001, msg: "permission error" }; + const request = vi.fn().mockResolvedValue(fulfilled); + + const result = await requestFeishuApi(request, "prefix", NO_DELAY); + expect(result).toBe(fulfilled); + expect(request).toHaveBeenCalledTimes(1); + }); + + it("recovers across thrown then fulfilled rate-limits (catch → try → ok)", async () => { + const request = vi + .fn() + .mockRejectedValueOnce(axiosError(230020)) + .mockResolvedValueOnce({ code: 11232, msg: "rate limit" }) + .mockResolvedValueOnce({ code: 0, data: { message_id: "om_recovered" } }); + + const result = await requestFeishuApi(request, "prefix", NO_DELAY); + expect((result as { data: { message_id: string } }).data.message_id).toBe("om_recovered"); + expect(request).toHaveBeenCalledTimes(3); + }); +});