From 70251e415d1608a7c8a4bc812e839eadfb458152 Mon Sep 17 00:00:00 2001 From: Chris Zhang Date: Wed, 29 Jul 2026 17:59:55 +0800 Subject: [PATCH] fix(feishu): cache inaccessible sender lookups (#111700) Co-authored-by: Chris <4436110+zqchris@users.noreply.github.com> Co-authored-by: Peter Steinberger --- extensions/feishu/src/bot-sender-name.test.ts | 70 +++++++++++++++++++ extensions/feishu/src/bot-sender-name.ts | 62 ++++++++++++---- 2 files changed, 119 insertions(+), 13 deletions(-) diff --git a/extensions/feishu/src/bot-sender-name.test.ts b/extensions/feishu/src/bot-sender-name.test.ts index 685cca848e23..59b5f70b59fb 100644 --- a/extensions/feishu/src/bot-sender-name.test.ts +++ b/extensions/feishu/src/bot-sender-name.test.ts @@ -21,6 +21,17 @@ const account = { config: FeishuConfigSchema.parse({}), } satisfies ResolvedFeishuAccount; +const secondaryAccount = { + ...account, + accountId: "secondary", +} satisfies ResolvedFeishuAccount; + +function feishuApiError(code: number, message: string): Error { + return Object.assign(new Error(message), { + response: { data: { code, msg: message } }, + }); +} + function mockUserNames(...names: string[]): ReturnType { const get = vi.fn(); for (const name of names) { @@ -66,6 +77,65 @@ describe("resolveFeishuSenderName", () => { expect(get).toHaveBeenCalledTimes(2); }); + it("caches 41050 misses per account without logging another failure", async () => { + const get = vi + .fn() + .mockRejectedValueOnce(feishuApiError(41050, "no user authority error")) + .mockResolvedValueOnce({ data: { user: { name: "Ada" } } }); + createFeishuClientMock.mockReturnValue({ contact: { user: { get } } }); + const log = vi.fn(); + + await expect( + resolveFeishuSenderName({ account, senderId: "ou_sender_hidden", log }), + ).resolves.toEqual({}); + await expect( + resolveFeishuSenderName({ account, senderId: "ou_sender_hidden", log }), + ).resolves.toEqual({}); + await expect( + resolveFeishuSenderName({ + account: secondaryAccount, + senderId: "ou_sender_hidden", + log, + }), + ).resolves.toEqual({ name: "Ada" }); + + expect(get).toHaveBeenCalledTimes(2); + expect(log).not.toHaveBeenCalled(); + }); + + it("retries a 41050 sender lookup after the negative cache expires", async () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-07-20T00:00:00Z")); + const get = vi + .fn() + .mockRejectedValueOnce(feishuApiError(41050, "no user authority error")) + .mockResolvedValueOnce({ data: { user: { name: "Ada" } } }); + createFeishuClientMock.mockReturnValue({ contact: { user: { get } } }); + + await expect( + resolveFeishuSenderName({ account, senderId: "ou_sender_retry", log: vi.fn() }), + ).resolves.toEqual({}); + vi.advanceTimersByTime(30 * 60 * 1000 + 1); + await expect( + resolveFeishuSenderName({ account, senderId: "ou_sender_retry", log: vi.fn() }), + ).resolves.toEqual({ name: "Ada" }); + + expect(get).toHaveBeenCalledTimes(2); + }); + + it("continues logging and retrying sender lookup failures other than 41050", async () => { + const error = feishuApiError(41051, "different lookup error"); + const get = vi.fn().mockRejectedValue(error); + createFeishuClientMock.mockReturnValue({ contact: { user: { get } } }); + const log = vi.fn(); + + await resolveFeishuSenderName({ account, senderId: "ou_sender_other_error", log }); + await resolveFeishuSenderName({ account, senderId: "ou_sender_other_error", log }); + + expect(get).toHaveBeenCalledTimes(2); + expect(log).toHaveBeenCalledTimes(2); + }); + it("evicts the oldest sender while retaining recent sender names", async () => { const get = vi.fn(async (params: { path: { user_id: string } }) => ({ data: { user: { name: `name-${params.path.user_id}` } }, diff --git a/extensions/feishu/src/bot-sender-name.ts b/extensions/feishu/src/bot-sender-name.ts index b3b3bf973ebf..f2b067676c99 100644 --- a/extensions/feishu/src/bot-sender-name.ts +++ b/extensions/feishu/src/bot-sender-name.ts @@ -25,13 +25,24 @@ type FeishuContactUserGetResponse = Awaited< type FeishuLogger = (...args: unknown[]) => void; +type FeishuApiError = { + code: number; + message: string; +}; + +type SenderNameCacheEntry = + | { kind: "resolved"; name: string; expireAt: number } + | { kind: "unavailable"; expireAt: number }; + const IGNORED_PERMISSION_SCOPE_TOKENS = ["contact:contact.base:readonly"]; const FEISHU_SCOPE_CORRECTIONS: Record = { "contact:contact.base:readonly": "contact:user.base:readonly", }; +const FEISHU_USER_LOOKUP_UNAUTHORIZED_CODE = 41050; const SENDER_NAME_TTL_MS = 10 * 60 * 1000; +const SENDER_NAME_NEGATIVE_TTL_MS = 30 * 60 * 1000; const SENDER_NAME_CACHE_MAX_SIZE = 500; -const senderNameCache = new Map(); +const senderNameCache = new Map(); function correctFeishuScopeInUrl(url: string): string { let corrected = url; @@ -47,7 +58,7 @@ function shouldSuppressPermissionErrorNotice(permissionError: FeishuPermissionEr return IGNORED_PERMISSION_SCOPE_TOKENS.some((token) => message.includes(token)); } -function extractPermissionError(err: unknown): FeishuPermissionError | null { +function extractFeishuApiError(err: unknown): FeishuApiError | null { if (!err || typeof err !== "object") { return null; } @@ -56,19 +67,34 @@ function extractPermissionError(err: unknown): FeishuPermissionError | null { if (!data || typeof data !== "object") { return null; } - const feishuErr = data as { code?: number; msg?: string }; - if (feishuErr.code !== 99991672) { + const feishuErr = data as { code?: unknown; msg?: unknown }; + if (typeof feishuErr.code !== "number") { return null; } - const msg = feishuErr.msg ?? ""; - const urlMatch = msg.match(/https:\/\/[^\s,]+\/app\/[^\s,]+/); return { code: feishuErr.code, - message: msg, + message: typeof feishuErr.msg === "string" ? feishuErr.msg : "", + }; +} + +function extractPermissionError(feishuErr: FeishuApiError | null): FeishuPermissionError | null { + if (feishuErr?.code !== 99991672) { + return null; + } + const urlMatch = feishuErr.message.match(/https:\/\/[^\s,]+\/app\/[^\s,]+/); + return { + code: feishuErr.code, + message: feishuErr.message, grantUrl: urlMatch?.[0] ? correctFeishuScopeInUrl(urlMatch[0]) : undefined, }; } +function writeSenderNameCache(key: string, entry: SenderNameCacheEntry): void { + senderNameCache.delete(key); + senderNameCache.set(key, entry); + pruneMapToMaxSize(senderNameCache, SENDER_NAME_CACHE_MAX_SIZE); +} + function resolveSenderLookupIdType(senderId: string): "open_id" | "user_id" | "union_id" { const trimmed = senderId.trim(); if (trimmed.startsWith("ou_")) { @@ -95,14 +121,15 @@ export async function resolveFeishuSenderName(params: { return {}; } - const cached = senderNameCache.get(normalizedSenderId); + const cacheKey = `${account.accountId}:${normalizedSenderId}`; + const cached = senderNameCache.get(cacheKey); const now = asDateTimestampMs(Date.now()); const cachedExpireAt = cached ? asDateTimestampMs(cached.expireAt) : undefined; if (cached && now !== undefined && cachedExpireAt !== undefined && cachedExpireAt > now) { - return { name: cached.name }; + return cached.kind === "resolved" ? { name: cached.name } : {}; } if (cached) { - senderNameCache.delete(normalizedSenderId); + senderNameCache.delete(cacheKey); } try { @@ -118,14 +145,14 @@ export async function resolveFeishuSenderName(params: { if (name) { const expireAt = resolveExpiresAtMsFromDurationMs(SENDER_NAME_TTL_MS); if (expireAt !== undefined) { - senderNameCache.set(normalizedSenderId, { name, expireAt }); - pruneMapToMaxSize(senderNameCache, SENDER_NAME_CACHE_MAX_SIZE); + writeSenderNameCache(cacheKey, { kind: "resolved", name, expireAt }); } return { name }; } return {}; } catch (err) { - const permErr = extractPermissionError(err); + const feishuErr = extractFeishuApiError(err); + const permErr = extractPermissionError(feishuErr); if (permErr) { if (shouldSuppressPermissionErrorNotice(permErr)) { log(`feishu: ignoring stale permission scope error: ${permErr.message}`); @@ -134,6 +161,15 @@ export async function resolveFeishuSenderName(params: { log(`feishu: permission error resolving sender name: code=${permErr.code}`); return { permissionError: permErr }; } + if (feishuErr?.code === FEISHU_USER_LOOKUP_UNAUTHORIZED_CODE) { + // 41050 means this app cannot see the user. Cache the account-scoped miss + // so later messages avoid repeating the same failing API request and SDK log. + const expireAt = resolveExpiresAtMsFromDurationMs(SENDER_NAME_NEGATIVE_TTL_MS); + if (expireAt !== undefined) { + writeSenderNameCache(cacheKey, { kind: "unavailable", expireAt }); + } + return {}; + } log(`feishu: failed to resolve sender name for ${normalizedSenderId}: ${String(err)}`); return {}; }