mirror of
https://github.com/openclaw/openclaw.git
synced 2026-03-12 07:20:45 +00:00
fix(feishu): avoid media regressions from global HTTP timeout (#36500)
* fix(feishu): avoid media regressions from global http timeout * fix(feishu): source HTTP timeout from config * fix(feishu): apply media timeout override to image uploads * fix(feishu): invalidate cached client when timeout changes * fix(feishu): clamp timeout values and cover image download
This commit is contained in:
@@ -43,12 +43,15 @@ import {
|
||||
createFeishuWSClient,
|
||||
clearClientCache,
|
||||
FEISHU_HTTP_TIMEOUT_MS,
|
||||
FEISHU_HTTP_TIMEOUT_MAX_MS,
|
||||
FEISHU_HTTP_TIMEOUT_ENV_VAR,
|
||||
} from "./client.js";
|
||||
|
||||
const proxyEnvKeys = ["https_proxy", "HTTPS_PROXY", "http_proxy", "HTTP_PROXY"] as const;
|
||||
type ProxyEnvKey = (typeof proxyEnvKeys)[number];
|
||||
|
||||
let priorProxyEnv: Partial<Record<ProxyEnvKey, string | undefined>> = {};
|
||||
let priorFeishuTimeoutEnv: string | undefined;
|
||||
|
||||
const baseAccount: ResolvedFeishuAccount = {
|
||||
accountId: "main",
|
||||
@@ -68,6 +71,8 @@ function firstWsClientOptions(): { agent?: unknown } {
|
||||
|
||||
beforeEach(() => {
|
||||
priorProxyEnv = {};
|
||||
priorFeishuTimeoutEnv = process.env[FEISHU_HTTP_TIMEOUT_ENV_VAR];
|
||||
delete process.env[FEISHU_HTTP_TIMEOUT_ENV_VAR];
|
||||
for (const key of proxyEnvKeys) {
|
||||
priorProxyEnv[key] = process.env[key];
|
||||
delete process.env[key];
|
||||
@@ -84,6 +89,11 @@ afterEach(() => {
|
||||
process.env[key] = value;
|
||||
}
|
||||
}
|
||||
if (priorFeishuTimeoutEnv === undefined) {
|
||||
delete process.env[FEISHU_HTTP_TIMEOUT_ENV_VAR];
|
||||
} else {
|
||||
process.env[FEISHU_HTTP_TIMEOUT_ENV_VAR] = priorFeishuTimeoutEnv;
|
||||
}
|
||||
});
|
||||
|
||||
describe("createFeishuClient HTTP timeout", () => {
|
||||
@@ -137,6 +147,121 @@ describe("createFeishuClient HTTP timeout", () => {
|
||||
expect.objectContaining({ timeout: 5_000 }),
|
||||
);
|
||||
});
|
||||
|
||||
it("uses config-configured default timeout when provided", async () => {
|
||||
createFeishuClient({
|
||||
appId: "app_4",
|
||||
appSecret: "secret_4",
|
||||
accountId: "timeout-config",
|
||||
config: { httpTimeoutMs: 45_000 },
|
||||
});
|
||||
|
||||
const calls = (LarkClient as unknown as ReturnType<typeof vi.fn>).mock.calls;
|
||||
const lastCall = calls[calls.length - 1][0] as {
|
||||
httpInstance: { get: (...args: unknown[]) => Promise<unknown> };
|
||||
};
|
||||
const httpInstance = lastCall.httpInstance;
|
||||
|
||||
await httpInstance.get("https://example.com/api");
|
||||
|
||||
expect(mockBaseHttpInstance.get).toHaveBeenCalledWith(
|
||||
"https://example.com/api",
|
||||
expect.objectContaining({ timeout: 45_000 }),
|
||||
);
|
||||
});
|
||||
|
||||
it("falls back to default timeout when configured timeout is invalid", async () => {
|
||||
createFeishuClient({
|
||||
appId: "app_5",
|
||||
appSecret: "secret_5",
|
||||
accountId: "timeout-config-invalid",
|
||||
config: { httpTimeoutMs: -1 },
|
||||
});
|
||||
|
||||
const calls = (LarkClient as unknown as ReturnType<typeof vi.fn>).mock.calls;
|
||||
const lastCall = calls[calls.length - 1][0] as {
|
||||
httpInstance: { get: (...args: unknown[]) => Promise<unknown> };
|
||||
};
|
||||
const httpInstance = lastCall.httpInstance;
|
||||
|
||||
await httpInstance.get("https://example.com/api");
|
||||
|
||||
expect(mockBaseHttpInstance.get).toHaveBeenCalledWith(
|
||||
"https://example.com/api",
|
||||
expect.objectContaining({ timeout: FEISHU_HTTP_TIMEOUT_MS }),
|
||||
);
|
||||
});
|
||||
|
||||
it("uses env timeout override when provided", async () => {
|
||||
process.env[FEISHU_HTTP_TIMEOUT_ENV_VAR] = "60000";
|
||||
|
||||
createFeishuClient({
|
||||
appId: "app_8",
|
||||
appSecret: "secret_8",
|
||||
accountId: "timeout-env-override",
|
||||
config: { httpTimeoutMs: 45_000 },
|
||||
});
|
||||
|
||||
const calls = (LarkClient as unknown as ReturnType<typeof vi.fn>).mock.calls;
|
||||
const lastCall = calls[calls.length - 1][0] as {
|
||||
httpInstance: { get: (...args: unknown[]) => Promise<unknown> };
|
||||
};
|
||||
await lastCall.httpInstance.get("https://example.com/api");
|
||||
|
||||
expect(mockBaseHttpInstance.get).toHaveBeenCalledWith(
|
||||
"https://example.com/api",
|
||||
expect.objectContaining({ timeout: 60_000 }),
|
||||
);
|
||||
});
|
||||
|
||||
it("clamps env timeout override to max bound", async () => {
|
||||
process.env[FEISHU_HTTP_TIMEOUT_ENV_VAR] = String(FEISHU_HTTP_TIMEOUT_MAX_MS + 123_456);
|
||||
|
||||
createFeishuClient({
|
||||
appId: "app_9",
|
||||
appSecret: "secret_9",
|
||||
accountId: "timeout-env-clamp",
|
||||
});
|
||||
|
||||
const calls = (LarkClient as unknown as ReturnType<typeof vi.fn>).mock.calls;
|
||||
const lastCall = calls[calls.length - 1][0] as {
|
||||
httpInstance: { get: (...args: unknown[]) => Promise<unknown> };
|
||||
};
|
||||
await lastCall.httpInstance.get("https://example.com/api");
|
||||
|
||||
expect(mockBaseHttpInstance.get).toHaveBeenCalledWith(
|
||||
"https://example.com/api",
|
||||
expect.objectContaining({ timeout: FEISHU_HTTP_TIMEOUT_MAX_MS }),
|
||||
);
|
||||
});
|
||||
|
||||
it("recreates cached client when configured timeout changes", async () => {
|
||||
createFeishuClient({
|
||||
appId: "app_6",
|
||||
appSecret: "secret_6",
|
||||
accountId: "timeout-cache-change",
|
||||
config: { httpTimeoutMs: 30_000 },
|
||||
});
|
||||
createFeishuClient({
|
||||
appId: "app_6",
|
||||
appSecret: "secret_6",
|
||||
accountId: "timeout-cache-change",
|
||||
config: { httpTimeoutMs: 45_000 },
|
||||
});
|
||||
|
||||
const calls = (LarkClient as unknown as ReturnType<typeof vi.fn>).mock.calls;
|
||||
expect(calls.length).toBe(2);
|
||||
|
||||
const lastCall = calls[calls.length - 1][0] as {
|
||||
httpInstance: { get: (...args: unknown[]) => Promise<unknown> };
|
||||
};
|
||||
await lastCall.httpInstance.get("https://example.com/api");
|
||||
|
||||
expect(mockBaseHttpInstance.get).toHaveBeenCalledWith(
|
||||
"https://example.com/api",
|
||||
expect.objectContaining({ timeout: 45_000 }),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("createFeishuWSClient proxy handling", () => {
|
||||
|
||||
@@ -1,9 +1,11 @@
|
||||
import * as Lark from "@larksuiteoapi/node-sdk";
|
||||
import { HttpsProxyAgent } from "https-proxy-agent";
|
||||
import type { FeishuDomain, ResolvedFeishuAccount } from "./types.js";
|
||||
import type { FeishuConfig, FeishuDomain, ResolvedFeishuAccount } from "./types.js";
|
||||
|
||||
/** Default HTTP timeout for Feishu API requests (30 seconds). */
|
||||
export const FEISHU_HTTP_TIMEOUT_MS = 30_000;
|
||||
export const FEISHU_HTTP_TIMEOUT_MAX_MS = 300_000;
|
||||
export const FEISHU_HTTP_TIMEOUT_ENV_VAR = "OPENCLAW_FEISHU_HTTP_TIMEOUT_MS";
|
||||
|
||||
function getWsProxyAgent(): HttpsProxyAgent<string> | undefined {
|
||||
const proxyUrl =
|
||||
@@ -20,7 +22,7 @@ const clientCache = new Map<
|
||||
string,
|
||||
{
|
||||
client: Lark.Client;
|
||||
config: { appId: string; appSecret: string; domain?: FeishuDomain };
|
||||
config: { appId: string; appSecret: string; domain?: FeishuDomain; httpTimeoutMs: number };
|
||||
}
|
||||
>();
|
||||
|
||||
@@ -39,11 +41,11 @@ function resolveDomain(domain: FeishuDomain | undefined): Lark.Domain | string {
|
||||
* but injects a default request timeout to prevent indefinite hangs
|
||||
* (e.g. when the Feishu API is slow, causing per-chat queue deadlocks).
|
||||
*/
|
||||
function createTimeoutHttpInstance(): Lark.HttpInstance {
|
||||
function createTimeoutHttpInstance(defaultTimeoutMs: number): Lark.HttpInstance {
|
||||
const base: Lark.HttpInstance = Lark.defaultHttpInstance as unknown as Lark.HttpInstance;
|
||||
|
||||
function injectTimeout<D>(opts?: Lark.HttpRequestOptions<D>): Lark.HttpRequestOptions<D> {
|
||||
return { timeout: FEISHU_HTTP_TIMEOUT_MS, ...opts } as Lark.HttpRequestOptions<D>;
|
||||
return { timeout: defaultTimeoutMs, ...opts } as Lark.HttpRequestOptions<D>;
|
||||
}
|
||||
|
||||
return {
|
||||
@@ -67,14 +69,40 @@ export type FeishuClientCredentials = {
|
||||
appId?: string;
|
||||
appSecret?: string;
|
||||
domain?: FeishuDomain;
|
||||
httpTimeoutMs?: number;
|
||||
config?: Pick<FeishuConfig, "httpTimeoutMs">;
|
||||
};
|
||||
|
||||
function resolveConfiguredHttpTimeoutMs(creds: FeishuClientCredentials): number {
|
||||
const clampTimeout = (value: number): number => {
|
||||
const rounded = Math.floor(value);
|
||||
return Math.min(Math.max(rounded, 1), FEISHU_HTTP_TIMEOUT_MAX_MS);
|
||||
};
|
||||
|
||||
const envRaw = process.env[FEISHU_HTTP_TIMEOUT_ENV_VAR];
|
||||
if (envRaw) {
|
||||
const envValue = Number(envRaw);
|
||||
if (Number.isFinite(envValue) && envValue > 0) {
|
||||
return clampTimeout(envValue);
|
||||
}
|
||||
}
|
||||
|
||||
const fromConfig = creds.config?.httpTimeoutMs;
|
||||
const fromDirectField = creds.httpTimeoutMs;
|
||||
const timeout = fromDirectField ?? fromConfig;
|
||||
if (typeof timeout !== "number" || !Number.isFinite(timeout) || timeout <= 0) {
|
||||
return FEISHU_HTTP_TIMEOUT_MS;
|
||||
}
|
||||
return clampTimeout(timeout);
|
||||
}
|
||||
|
||||
/**
|
||||
* Create or get a cached Feishu client for an account.
|
||||
* Accepts any object with appId, appSecret, and optional domain/accountId.
|
||||
*/
|
||||
export function createFeishuClient(creds: FeishuClientCredentials): Lark.Client {
|
||||
const { accountId = "default", appId, appSecret, domain } = creds;
|
||||
const defaultHttpTimeoutMs = resolveConfiguredHttpTimeoutMs(creds);
|
||||
|
||||
if (!appId || !appSecret) {
|
||||
throw new Error(`Feishu credentials not configured for account "${accountId}"`);
|
||||
@@ -86,7 +114,8 @@ export function createFeishuClient(creds: FeishuClientCredentials): Lark.Client
|
||||
cached &&
|
||||
cached.config.appId === appId &&
|
||||
cached.config.appSecret === appSecret &&
|
||||
cached.config.domain === domain
|
||||
cached.config.domain === domain &&
|
||||
cached.config.httpTimeoutMs === defaultHttpTimeoutMs
|
||||
) {
|
||||
return cached.client;
|
||||
}
|
||||
@@ -97,13 +126,13 @@ export function createFeishuClient(creds: FeishuClientCredentials): Lark.Client
|
||||
appSecret,
|
||||
appType: Lark.AppType.SelfBuild,
|
||||
domain: resolveDomain(domain),
|
||||
httpInstance: createTimeoutHttpInstance(),
|
||||
httpInstance: createTimeoutHttpInstance(defaultHttpTimeoutMs),
|
||||
});
|
||||
|
||||
// Cache it
|
||||
clientCache.set(accountId, {
|
||||
client,
|
||||
config: { appId, appSecret, domain },
|
||||
config: { appId, appSecret, domain, httpTimeoutMs: defaultHttpTimeoutMs },
|
||||
});
|
||||
|
||||
return client;
|
||||
|
||||
@@ -165,6 +165,7 @@ const FeishuSharedConfigShape = {
|
||||
chunkMode: z.enum(["length", "newline"]).optional(),
|
||||
blockStreamingCoalesce: BlockStreamingCoalesceSchema,
|
||||
mediaMaxMb: z.number().positive().optional(),
|
||||
httpTimeoutMs: z.number().int().positive().max(300_000).optional(),
|
||||
heartbeat: ChannelHeartbeatVisibilitySchema,
|
||||
renderMode: RenderModeSchema,
|
||||
streaming: StreamingModeSchema,
|
||||
|
||||
@@ -10,6 +10,7 @@ const resolveReceiveIdTypeMock = vi.hoisted(() => vi.fn());
|
||||
const loadWebMediaMock = vi.hoisted(() => vi.fn());
|
||||
|
||||
const fileCreateMock = vi.hoisted(() => vi.fn());
|
||||
const imageCreateMock = vi.hoisted(() => vi.fn());
|
||||
const imageGetMock = vi.hoisted(() => vi.fn());
|
||||
const messageCreateMock = vi.hoisted(() => vi.fn());
|
||||
const messageResourceGetMock = vi.hoisted(() => vi.fn());
|
||||
@@ -75,6 +76,7 @@ describe("sendMediaFeishu msg_type routing", () => {
|
||||
create: fileCreateMock,
|
||||
},
|
||||
image: {
|
||||
create: imageCreateMock,
|
||||
get: imageGetMock,
|
||||
},
|
||||
message: {
|
||||
@@ -91,6 +93,10 @@ describe("sendMediaFeishu msg_type routing", () => {
|
||||
code: 0,
|
||||
data: { file_key: "file_key_1" },
|
||||
});
|
||||
imageCreateMock.mockResolvedValue({
|
||||
code: 0,
|
||||
data: { image_key: "image_key_1" },
|
||||
});
|
||||
|
||||
messageCreateMock.mockResolvedValue({
|
||||
code: 0,
|
||||
@@ -176,6 +182,26 @@ describe("sendMediaFeishu msg_type routing", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("uses image upload timeout override for image media", async () => {
|
||||
await sendMediaFeishu({
|
||||
cfg: {} as any,
|
||||
to: "user:ou_target",
|
||||
mediaBuffer: Buffer.from("image"),
|
||||
fileName: "photo.png",
|
||||
});
|
||||
|
||||
expect(imageCreateMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
timeout: 120_000,
|
||||
}),
|
||||
);
|
||||
expect(messageCreateMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
data: expect.objectContaining({ msg_type: "image" }),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("uses msg_type=media when replying with mp4", async () => {
|
||||
await sendMediaFeishu({
|
||||
cfg: {} as any,
|
||||
@@ -291,6 +317,12 @@ describe("sendMediaFeishu msg_type routing", () => {
|
||||
imageKey,
|
||||
});
|
||||
|
||||
expect(imageGetMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
path: { image_key: imageKey },
|
||||
timeout: 120_000,
|
||||
}),
|
||||
);
|
||||
expect(result.buffer).toEqual(Buffer.from("image-data"));
|
||||
expect(capturedPath).toBeDefined();
|
||||
expectPathIsolatedToTmpRoot(capturedPath as string, imageKey);
|
||||
@@ -476,10 +508,13 @@ describe("downloadMessageResourceFeishu", () => {
|
||||
type: "file",
|
||||
});
|
||||
|
||||
expect(messageResourceGetMock).toHaveBeenCalledWith({
|
||||
path: { message_id: "om_audio_msg", file_key: "file_key_audio" },
|
||||
params: { type: "file" },
|
||||
});
|
||||
expect(messageResourceGetMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
path: { message_id: "om_audio_msg", file_key: "file_key_audio" },
|
||||
params: { type: "file" },
|
||||
timeout: 120_000,
|
||||
}),
|
||||
);
|
||||
expect(result.buffer).toBeInstanceOf(Buffer);
|
||||
});
|
||||
|
||||
@@ -493,10 +528,13 @@ describe("downloadMessageResourceFeishu", () => {
|
||||
type: "image",
|
||||
});
|
||||
|
||||
expect(messageResourceGetMock).toHaveBeenCalledWith({
|
||||
path: { message_id: "om_img_msg", file_key: "img_key_1" },
|
||||
params: { type: "image" },
|
||||
});
|
||||
expect(messageResourceGetMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
path: { message_id: "om_img_msg", file_key: "img_key_1" },
|
||||
params: { type: "image" },
|
||||
timeout: 120_000,
|
||||
}),
|
||||
);
|
||||
expect(result.buffer).toBeInstanceOf(Buffer);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -9,6 +9,8 @@ import { getFeishuRuntime } from "./runtime.js";
|
||||
import { assertFeishuMessageApiSuccess, toFeishuSendResult } from "./send-result.js";
|
||||
import { resolveFeishuSendTarget } from "./send-target.js";
|
||||
|
||||
const FEISHU_MEDIA_HTTP_TIMEOUT_MS = 120_000;
|
||||
|
||||
export type DownloadImageResult = {
|
||||
buffer: Buffer;
|
||||
contentType?: string;
|
||||
@@ -101,6 +103,7 @@ export async function downloadImageFeishu(params: {
|
||||
|
||||
const response = await client.im.image.get({
|
||||
path: { image_key: normalizedImageKey },
|
||||
timeout: FEISHU_MEDIA_HTTP_TIMEOUT_MS,
|
||||
});
|
||||
|
||||
const buffer = await readFeishuResponseBuffer({
|
||||
@@ -137,6 +140,7 @@ export async function downloadMessageResourceFeishu(params: {
|
||||
const response = await client.im.messageResource.get({
|
||||
path: { message_id: messageId, file_key: normalizedFileKey },
|
||||
params: { type },
|
||||
timeout: FEISHU_MEDIA_HTTP_TIMEOUT_MS,
|
||||
});
|
||||
|
||||
const buffer = await readFeishuResponseBuffer({
|
||||
@@ -189,6 +193,7 @@ export async function uploadImageFeishu(params: {
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- SDK accepts Buffer or ReadStream
|
||||
image: imageData as any,
|
||||
},
|
||||
timeout: FEISHU_MEDIA_HTTP_TIMEOUT_MS,
|
||||
});
|
||||
|
||||
// SDK v1.30+ returns data directly without code wrapper on success
|
||||
@@ -260,6 +265,7 @@ export async function uploadFileFeishu(params: {
|
||||
file: fileData as any,
|
||||
...(duration !== undefined && { duration }),
|
||||
},
|
||||
timeout: FEISHU_MEDIA_HTTP_TIMEOUT_MS,
|
||||
});
|
||||
|
||||
// SDK v1.30+ returns data directly without code wrapper on success
|
||||
|
||||
Reference in New Issue
Block a user