mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-21 10:16:44 +00:00
fix(discord): add timeouts to directory lookup requests (#104290)
This commit is contained in:
@@ -303,8 +303,8 @@ describe("fetchDiscord", () => {
|
||||
});
|
||||
|
||||
it("caps oversized request timeouts before creating abort signals", async () => {
|
||||
const timeoutController = new AbortController();
|
||||
const timeoutSpy = vi.spyOn(AbortSignal, "timeout").mockReturnValue(timeoutController.signal);
|
||||
const setTimeoutSpy = vi.spyOn(globalThis, "setTimeout");
|
||||
const clearTimeoutSpy = vi.spyOn(globalThis, "clearTimeout");
|
||||
let request: RequestInit | undefined;
|
||||
const fetcher = withFetchPreconnect(async (_url, init) => {
|
||||
request = init;
|
||||
@@ -317,8 +317,9 @@ describe("fetchDiscord", () => {
|
||||
timeoutMs: Number.MAX_SAFE_INTEGER,
|
||||
});
|
||||
|
||||
expect(timeoutSpy).toHaveBeenCalledWith(MAX_TIMER_TIMEOUT_MS);
|
||||
expect(request?.signal).toBe(timeoutController.signal);
|
||||
expect(setTimeoutSpy).toHaveBeenCalledWith(expect.any(Function), MAX_TIMER_TIMEOUT_MS);
|
||||
expect(request?.signal).toBeInstanceOf(AbortSignal);
|
||||
expect(clearTimeoutSpy).toHaveBeenCalledWith(setTimeoutSpy.mock.results[0]?.value);
|
||||
});
|
||||
|
||||
it("throws DiscordApiError on malformed JSON success response body", async () => {
|
||||
|
||||
@@ -21,6 +21,7 @@ const DISCORD_API_RETRY_DEFAULTS = {
|
||||
const DISCORD_API_429_FALLBACK_RETRY_AFTER_SECONDS = 60;
|
||||
const DISCORD_API_ERROR_BODY_LIMIT_BYTES = 8 * 1024;
|
||||
const DISCORD_API_RESPONSE_BODY_LIMIT_BYTES = 4 * 1024 * 1024;
|
||||
export const DISCORD_DIRECTORY_LOOKUP_TIMEOUT_MS = 10_000;
|
||||
|
||||
type DiscordApiErrorPayload = {
|
||||
message?: string;
|
||||
@@ -120,6 +121,8 @@ function getDiscordApiRetryAfterMs(
|
||||
type DiscordFetchOptions = {
|
||||
retry?: RetryConfig;
|
||||
label?: string;
|
||||
signal?: AbortSignal;
|
||||
timeoutMs?: number;
|
||||
};
|
||||
|
||||
type DiscordApiRequestOptions = DiscordFetchOptions & {
|
||||
@@ -127,8 +130,6 @@ type DiscordApiRequestOptions = DiscordFetchOptions & {
|
||||
fetcher?: typeof fetch;
|
||||
headers?: Record<string, string>;
|
||||
method?: string;
|
||||
signal?: AbortSignal;
|
||||
timeoutMs?: number;
|
||||
};
|
||||
|
||||
function normalizeDiscordRequestBody(body: unknown, headers: Headers): BodyInit | null | undefined {
|
||||
@@ -148,11 +149,22 @@ function normalizeDiscordRequestBody(body: unknown, headers: Headers): BodyInit
|
||||
return JSON.stringify(body);
|
||||
}
|
||||
|
||||
function resolveDiscordRequestSignal(options: DiscordApiRequestOptions) {
|
||||
if (options.signal || typeof options.timeoutMs !== "number") {
|
||||
return options.signal;
|
||||
function createDiscordRequestSignal(options: DiscordApiRequestOptions): {
|
||||
signal?: AbortSignal;
|
||||
cleanup: () => void;
|
||||
} {
|
||||
if (typeof options.timeoutMs !== "number" || options.signal?.aborted) {
|
||||
return { signal: options.signal, cleanup: () => undefined };
|
||||
}
|
||||
return AbortSignal.timeout(resolveTimerTimeoutMs(options.timeoutMs, 1));
|
||||
const controller = new AbortController();
|
||||
const timeout = setTimeout(() => controller.abort(), resolveTimerTimeoutMs(options.timeoutMs, 1));
|
||||
timeout.unref?.();
|
||||
return {
|
||||
signal: options.signal
|
||||
? AbortSignal.any([options.signal, controller.signal])
|
||||
: controller.signal,
|
||||
cleanup: () => clearTimeout(timeout),
|
||||
};
|
||||
}
|
||||
|
||||
export async function requestDiscord<T>(
|
||||
@@ -171,42 +183,51 @@ export async function requestDiscord<T>(
|
||||
const headers = new Headers(options?.headers);
|
||||
headers.set("Authorization", `Bot ${token}`);
|
||||
const body = normalizeDiscordRequestBody(options?.body, headers);
|
||||
const res = await fetchImpl(`${DISCORD_API_BASE}${path}`, {
|
||||
method: options?.method ?? (body === undefined ? "GET" : "POST"),
|
||||
headers,
|
||||
body,
|
||||
signal: resolveDiscordRequestSignal(options ?? {}),
|
||||
});
|
||||
if (!res.ok) {
|
||||
const text = await readResponseTextLimited(res, DISCORD_API_ERROR_BODY_LIMIT_BYTES).catch(
|
||||
() => "",
|
||||
);
|
||||
const detail = formatDiscordApiErrorText(text, res);
|
||||
const suffix = detail ? `: ${detail}` : "";
|
||||
const retryAfter =
|
||||
res.status === 429
|
||||
? (parseRetryAfterSeconds(text, res) ?? DISCORD_API_429_FALLBACK_RETRY_AFTER_SECONDS)
|
||||
: undefined;
|
||||
throw new DiscordApiError(
|
||||
`Discord API ${path} failed (${res.status})${suffix}`,
|
||||
res.status,
|
||||
retryAfter,
|
||||
);
|
||||
}
|
||||
const responseBody = await readResponseWithLimit(res, DISCORD_API_RESPONSE_BODY_LIMIT_BYTES, {
|
||||
onOverflow: ({ size, maxBytes }) =>
|
||||
new Error(
|
||||
`Discord API ${path} response body too large: ${size} bytes (limit: ${maxBytes} bytes)`,
|
||||
),
|
||||
});
|
||||
const text = new TextDecoder().decode(responseBody);
|
||||
if (!text.trim()) {
|
||||
return undefined as T;
|
||||
}
|
||||
const requestSignal = createDiscordRequestSignal(options ?? {});
|
||||
try {
|
||||
return JSON.parse(text) as T;
|
||||
} catch {
|
||||
throw new DiscordApiError(`Discord API ${path} returned malformed JSON`, 0);
|
||||
const res = await fetchImpl(`${DISCORD_API_BASE}${path}`, {
|
||||
method: options?.method ?? (body === undefined ? "GET" : "POST"),
|
||||
headers,
|
||||
body,
|
||||
signal: requestSignal.signal,
|
||||
});
|
||||
if (!res.ok) {
|
||||
const text = await readResponseTextLimited(res, DISCORD_API_ERROR_BODY_LIMIT_BYTES).catch(
|
||||
() => "",
|
||||
);
|
||||
const detail = formatDiscordApiErrorText(text, res);
|
||||
const suffix = detail ? `: ${detail}` : "";
|
||||
const retryAfter =
|
||||
res.status === 429
|
||||
? (parseRetryAfterSeconds(text, res) ?? DISCORD_API_429_FALLBACK_RETRY_AFTER_SECONDS)
|
||||
: undefined;
|
||||
throw new DiscordApiError(
|
||||
`Discord API ${path} failed (${res.status})${suffix}`,
|
||||
res.status,
|
||||
retryAfter,
|
||||
);
|
||||
}
|
||||
const responseBody = await readResponseWithLimit(
|
||||
res,
|
||||
DISCORD_API_RESPONSE_BODY_LIMIT_BYTES,
|
||||
{
|
||||
onOverflow: ({ size, maxBytes }) =>
|
||||
new Error(
|
||||
`Discord API ${path} response body too large: ${size} bytes (limit: ${maxBytes} bytes)`,
|
||||
),
|
||||
},
|
||||
);
|
||||
const text = new TextDecoder().decode(responseBody);
|
||||
if (!text.trim()) {
|
||||
return undefined as T;
|
||||
}
|
||||
try {
|
||||
return JSON.parse(text) as T;
|
||||
} catch {
|
||||
throw new DiscordApiError(`Discord API ${path} returned malformed JSON`, 0);
|
||||
}
|
||||
} finally {
|
||||
requestSignal.cleanup();
|
||||
}
|
||||
},
|
||||
{
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts";
|
||||
import type { DirectoryConfigParams } from "openclaw/plugin-sdk/directory-runtime";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { DISCORD_DIRECTORY_LOOKUP_TIMEOUT_MS } from "./api.js";
|
||||
import { listDiscordDirectoryGroupsLive, listDiscordDirectoryPeersLive } from "./directory-live.js";
|
||||
|
||||
function makeParams(overrides: Partial<DirectoryConfigParams> = {}): DirectoryConfigParams {
|
||||
@@ -29,6 +30,23 @@ function resolveFetchUrl(input: string | URL | Request): string {
|
||||
return typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url;
|
||||
}
|
||||
|
||||
function hangingBodyResponse(signal?: AbortSignal): Response {
|
||||
const stream = new ReadableStream<Uint8Array>({
|
||||
start(controller) {
|
||||
controller.enqueue(new TextEncoder().encode("["));
|
||||
if (signal?.aborted) {
|
||||
controller.error(signal.reason);
|
||||
return;
|
||||
}
|
||||
signal?.addEventListener("abort", () => controller.error(signal.reason), { once: true });
|
||||
},
|
||||
});
|
||||
return new Response(stream, {
|
||||
status: 200,
|
||||
headers: { "content-type": "application/json" },
|
||||
});
|
||||
}
|
||||
|
||||
describe("discord directory live lookups", () => {
|
||||
beforeEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
@@ -36,6 +54,7 @@ describe("discord directory live lookups", () => {
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
vi.unstubAllEnvs();
|
||||
});
|
||||
|
||||
@@ -58,6 +77,20 @@ describe("discord directory live lookups", () => {
|
||||
expect(fetchSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("aborts hanging group directory response bodies after the lookup timeout", async () => {
|
||||
vi.useFakeTimers();
|
||||
const fetchSpy = vi.spyOn(globalThis, "fetch").mockImplementation(async (_input, init) => {
|
||||
return hangingBodyResponse(init?.signal ?? undefined);
|
||||
});
|
||||
|
||||
const rows = listDiscordDirectoryGroupsLive(makeParams({ query: "general" }));
|
||||
const rejection = expect(rows).rejects.toThrow(/abort/i);
|
||||
|
||||
await vi.advanceTimersByTimeAsync(DISCORD_DIRECTORY_LOOKUP_TIMEOUT_MS);
|
||||
await rejection;
|
||||
expect(fetchSpy).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("filters group channels by query and respects limit", async () => {
|
||||
vi.spyOn(globalThis, "fetch").mockImplementation(async (input) => {
|
||||
const url = resolveFetchUrl(input);
|
||||
|
||||
@@ -5,7 +5,7 @@ import type {
|
||||
} from "openclaw/plugin-sdk/directory-runtime";
|
||||
import { normalizeOptionalLowercaseString } from "openclaw/plugin-sdk/string-coerce-runtime";
|
||||
import { resolveDiscordAccount } from "./accounts.js";
|
||||
import { fetchDiscord } from "./api.js";
|
||||
import { DISCORD_DIRECTORY_LOOKUP_TIMEOUT_MS, fetchDiscord } from "./api.js";
|
||||
import { rememberDiscordDirectoryUser } from "./directory-cache.js";
|
||||
import { normalizeDiscordSlug } from "./monitor/allow-list.js";
|
||||
import { normalizeDiscordToken } from "./token.js";
|
||||
@@ -36,7 +36,9 @@ function resolveDiscordDirectoryAccess(
|
||||
}
|
||||
|
||||
async function listDiscordGuilds(token: string): Promise<DiscordGuild[]> {
|
||||
const rawGuilds = await fetchDiscord<DiscordGuild[]>("/users/@me/guilds", token);
|
||||
const rawGuilds = await fetchDiscord<DiscordGuild[]>("/users/@me/guilds", token, fetch, {
|
||||
timeoutMs: DISCORD_DIRECTORY_LOOKUP_TIMEOUT_MS,
|
||||
});
|
||||
return rawGuilds.filter((guild) => guild.id && guild.name);
|
||||
}
|
||||
|
||||
@@ -52,7 +54,12 @@ export async function listDiscordDirectoryGroupsLive(
|
||||
const rows: ChannelDirectoryEntry[] = [];
|
||||
|
||||
for (const guild of guilds) {
|
||||
const channels = await fetchDiscord<DiscordChannel[]>(`/guilds/${guild.id}/channels`, token);
|
||||
const channels = await fetchDiscord<DiscordChannel[]>(
|
||||
`/guilds/${guild.id}/channels`,
|
||||
token,
|
||||
fetch,
|
||||
{ timeoutMs: DISCORD_DIRECTORY_LOOKUP_TIMEOUT_MS },
|
||||
);
|
||||
for (const channel of channels) {
|
||||
const name = channel.name?.trim();
|
||||
if (!name) {
|
||||
@@ -101,6 +108,8 @@ export async function listDiscordDirectoryPeersLive(
|
||||
const members = await fetchDiscord<DiscordMember[]>(
|
||||
`/guilds/${guild.id}/members/search?${paramsObj.toString()}`,
|
||||
token,
|
||||
fetch,
|
||||
{ timeoutMs: DISCORD_DIRECTORY_LOOKUP_TIMEOUT_MS },
|
||||
);
|
||||
for (const member of members) {
|
||||
const user = member.user;
|
||||
|
||||
@@ -11,11 +11,13 @@ export type DiscordGuildSummary = {
|
||||
export async function listGuilds(
|
||||
token: string,
|
||||
fetcher: typeof fetch,
|
||||
options?: { timeoutMs?: number },
|
||||
): Promise<DiscordGuildSummary[]> {
|
||||
const raw = await fetchDiscord<Array<{ id?: string; name?: string }>>(
|
||||
"/users/@me/guilds",
|
||||
token,
|
||||
fetcher,
|
||||
options,
|
||||
);
|
||||
return raw
|
||||
.filter(
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
// Discord plugin module implements resolve channels behavior.
|
||||
import { DiscordApiError, fetchDiscord } from "./api.js";
|
||||
import { DISCORD_DIRECTORY_LOOKUP_TIMEOUT_MS, DiscordApiError, fetchDiscord } from "./api.js";
|
||||
import { listGuilds } from "./guilds.js";
|
||||
import { normalizeDiscordSlug } from "./monitor/allow-list.js";
|
||||
import {
|
||||
@@ -85,6 +85,7 @@ async function listGuildChannels(
|
||||
`/guilds/${guildId}/channels`,
|
||||
token,
|
||||
fetcher,
|
||||
{ timeoutMs: DISCORD_DIRECTORY_LOOKUP_TIMEOUT_MS },
|
||||
);
|
||||
return raw
|
||||
.map((channel) => {
|
||||
@@ -113,7 +114,9 @@ async function fetchChannel(
|
||||
): Promise<FetchChannelResult> {
|
||||
let raw: DiscordChannelPayload;
|
||||
try {
|
||||
raw = await fetchDiscord<DiscordChannelPayload>(`/channels/${channelId}`, token, fetcher);
|
||||
raw = await fetchDiscord<DiscordChannelPayload>(`/channels/${channelId}`, token, fetcher, {
|
||||
timeoutMs: DISCORD_DIRECTORY_LOOKUP_TIMEOUT_MS,
|
||||
});
|
||||
} catch (err) {
|
||||
if (err instanceof DiscordApiError && err.status === 403) {
|
||||
return { status: "forbidden" };
|
||||
@@ -164,7 +167,9 @@ export async function resolveDiscordChannelAllowlist(params: {
|
||||
}));
|
||||
}
|
||||
const fetcher = params.fetcher ?? fetch;
|
||||
const guilds = await listGuilds(token, fetcher);
|
||||
const guilds = await listGuilds(token, fetcher, {
|
||||
timeoutMs: DISCORD_DIRECTORY_LOOKUP_TIMEOUT_MS,
|
||||
});
|
||||
const channelsByGuild = new Map<string, Promise<DiscordChannelSummary[]>>();
|
||||
const getChannels = (guildId: string) => {
|
||||
const existing = channelsByGuild.get(guildId);
|
||||
|
||||
@@ -3,7 +3,7 @@ import {
|
||||
normalizeLowercaseStringOrEmpty,
|
||||
normalizeOptionalString,
|
||||
} from "openclaw/plugin-sdk/string-coerce-runtime";
|
||||
import { fetchDiscord } from "./api.js";
|
||||
import { DISCORD_DIRECTORY_LOOKUP_TIMEOUT_MS, fetchDiscord } from "./api.js";
|
||||
import { listGuilds, type DiscordGuildSummary } from "./guilds.js";
|
||||
import {
|
||||
buildDiscordUnresolvedResults,
|
||||
@@ -106,7 +106,9 @@ export async function resolveDiscordUserAllowlist(params: {
|
||||
let guilds: DiscordGuildSummary[] | null = null;
|
||||
const getGuilds = async (): Promise<DiscordGuildSummary[]> => {
|
||||
if (!guilds) {
|
||||
guilds = await listGuilds(token, fetcher);
|
||||
guilds = await listGuilds(token, fetcher, {
|
||||
timeoutMs: DISCORD_DIRECTORY_LOOKUP_TIMEOUT_MS,
|
||||
});
|
||||
}
|
||||
return guilds;
|
||||
};
|
||||
@@ -148,6 +150,7 @@ export async function resolveDiscordUserAllowlist(params: {
|
||||
`/guilds/${guild.id}/members/search?${paramsObj.toString()}`,
|
||||
token,
|
||||
fetcher,
|
||||
{ timeoutMs: DISCORD_DIRECTORY_LOOKUP_TIMEOUT_MS },
|
||||
);
|
||||
for (const member of members) {
|
||||
const score = scoreDiscordMember(member, query);
|
||||
|
||||
Reference in New Issue
Block a user