fix(discord): default non-finite REST numeric options

steipete · steipete · commit cb085ec5f1f5 · 2026-05-28T23:30:47.000-04:00
diff --git a/extensions/discord/src/internal/rest.test.ts b/extensions/discord/src/internal/rest.test.ts
@@ -71,6 +71,59 @@ describe("RequestClient", () => {
     expect(client.queueSize).toBe(0);
   });
 
+  it("defaults non-finite REST client numeric options before scheduling requests", async () => {
+    const fetchSpy = vi.fn(async (input: string | URL | Request) => {
+      expect(new URL(readRequestUrl(input)).pathname).toBe("/api/v10/guilds/g1/roles");
+      return createJsonResponse({ ok: true });
+    });
+    const client = new RequestClient("test-token", {
+      fetch: fetchSpy,
+      apiVersion: Number.NaN,
+      timeout: Number.NaN,
+      maxQueueSize: Number.NaN,
+      scheduler: {
+        maxConcurrency: Number.NaN,
+        maxRateLimitRetries: Number.NaN,
+        lanes: {
+          background: {
+            maxQueueSize: Number.NaN,
+            staleAfterMs: Number.NaN,
+            weight: Number.NaN,
+          },
+        },
+      },
+    });
+
+    await expect(client.get("/guilds/g1/roles")).resolves.toEqual({ ok: true });
+    expect(client.getSchedulerMetrics().maxConcurrentWorkers).toBe(4);
+  });
+
+  it("uses the default background stale timeout for non-finite lane overrides", async () => {
+    vi.useFakeTimers();
+    vi.setSystemTime(0);
+    const firstResponse = createDeferred<Response>();
+    const fetchSpy = vi.fn(async () => await firstResponse.promise);
+    const client = new RequestClient("test-token", {
+      fetch: fetchSpy,
+      scheduler: {
+        maxConcurrency: 1,
+        lanes: {
+          background: { staleAfterMs: Number.NaN },
+        },
+      },
+    });
+
+    const first = client.get("/guilds/g1/roles");
+    const stale = client.get("/guilds/g2/roles");
+    await vi.waitFor(() => expect(fetchSpy).toHaveBeenCalledTimes(1));
+
+    await vi.advanceTimersByTimeAsync(20_001);
+    firstResponse.resolve(createJsonResponse({ ok: "first" }));
+
+    await expect(first).resolves.toEqual({ ok: "first" });
+    await expect(stale).rejects.toThrow(/Dropped stale background request/);
+  });
+
   it("dispatches critical interaction callbacks before older background requests", async () => {
     const firstResponse = createDeferred<Response>();
     const responses = new Map<string, Promise<Response>>([
diff --git a/extensions/discord/src/internal/rest.ts b/extensions/discord/src/internal/rest.ts
@@ -1,5 +1,6 @@
 import { randomBytes } from "node:crypto";
 import { inspect } from "node:util";
+import { parseFiniteNumber } from "openclaw/plugin-sdk/number-runtime";
 import { serializeRequestBody } from "./rest-body.js";
 import {
   DiscordError,
@@ -41,6 +42,12 @@ export type RequestClientOptions = {
   fetch?: (input: string | URL | Request, init?: RequestInit) => Promise<Response>;
 };
 
+type NormalizedRequestClientOptions = RequestClientOptions & {
+  apiVersion: number;
+  maxQueueSize: number;
+  timeout: number;
+};
+
 export type RequestData = {
   body?: unknown;
   multipartStyle?: "message" | "form";
@@ -134,7 +141,7 @@ async function normalizeFetchBody(
 }
 
 export class RequestClient {
-  readonly options: RequestClientOptions;
+  readonly options: NormalizedRequestClientOptions;
   protected token: string;
   protected customFetch: RequestClientOptions["fetch"];
   protected requestControllers = new Set<AbortController>();
@@ -143,16 +150,23 @@ export class RequestClient {
   constructor(token: string, options?: RequestClientOptions) {
     this.token = token.replace(/^Bot\s+/i, "");
     this.customFetch = options?.fetch;
-    this.options = { ...defaultOptions, ...options };
+    this.options = normalizeRequestClientOptions(options);
     this.scheduler = new RestScheduler<RequestData>(
       {
-        lanes: normalizeSchedulerLanes(
-          this.options.maxQueueSize ?? defaultOptions.maxQueueSize,
-          this.options.scheduler?.lanes,
+        lanes: normalizeSchedulerLanes(this.options.maxQueueSize, this.options.scheduler?.lanes),
+        maxConcurrency: normalizeIntegerOption(
+          this.options.scheduler?.maxConcurrency,
+          DEFAULT_MAX_CONCURRENT_WORKERS,
+          { min: 1 },
+        ),
+        maxQueueSize: this.options.maxQueueSize,
+        maxRateLimitRetries: normalizeIntegerOption(
+          this.options.scheduler?.maxRateLimitRetries,
+          3,
+          {
+            min: 0,
+          },
         ),
-        maxConcurrency: this.options.scheduler?.maxConcurrency ?? DEFAULT_MAX_CONCURRENT_WORKERS,
-        maxQueueSize: this.options.maxQueueSize ?? defaultOptions.maxQueueSize,
-        maxRateLimitRetries: this.options.scheduler?.maxRateLimitRetries ?? 3,
       },
       async (request) =>
         await this.executeRequest(
@@ -280,11 +294,36 @@ export class RequestClient {
   }
 }
 
+function normalizeIntegerOption(
+  value: number | undefined,
+  fallback: number,
+  params: { min: number },
+): number {
+  const candidate = parseFiniteNumber(value) ?? fallback;
+  return Math.max(params.min, Math.floor(candidate));
+}
+
+function normalizeRequestClientOptions(
+  options?: RequestClientOptions,
+): NormalizedRequestClientOptions {
+  const merged = { ...defaultOptions, ...options };
+  return {
+    ...merged,
+    apiVersion: normalizeIntegerOption(merged.apiVersion, defaultOptions.apiVersion, { min: 1 }),
+    timeout: normalizeIntegerOption(merged.timeout, defaultOptions.timeout, { min: 1 }),
+    maxQueueSize: normalizeIntegerOption(merged.maxQueueSize, defaultOptions.maxQueueSize, {
+      min: 1,
+    }),
+  };
+}
+
 function normalizeSchedulerLanes(
   maxQueueSize: number,
   lanes?: RequestSchedulerOptions["lanes"],
 ): Record<RestRequestPriority, { maxQueueSize: number; staleAfterMs?: number; weight: number }> {
-  const fallbackMaxQueueSize = Math.max(1, Math.floor(maxQueueSize));
+  const fallbackMaxQueueSize = normalizeIntegerOption(maxQueueSize, defaultOptions.maxQueueSize, {
+    min: 1,
+  });
   return {
     critical: normalizeSchedulerLane("critical", fallbackMaxQueueSize, lanes?.critical),
     standard: normalizeSchedulerLane("standard", fallbackMaxQueueSize, lanes?.standard),
@@ -298,17 +337,20 @@ function normalizeSchedulerLane(
   options?: { maxQueueSize?: number; staleAfterMs?: number; weight?: number },
 ): { maxQueueSize: number; staleAfterMs?: number; weight: number } {
   const defaults = defaultLaneOptions[lane];
+  const staleAfterMs =
+    options?.staleAfterMs !== undefined
+      ? normalizeIntegerOption(options.staleAfterMs, defaults.staleAfterMs ?? 0, { min: 0 })
+      : defaults.staleAfterMs;
   return {
     maxQueueSize:
       options?.maxQueueSize !== undefined
-        ? Math.max(1, Math.floor(options.maxQueueSize))
+        ? normalizeIntegerOption(options.maxQueueSize, maxQueueSize, { min: 1 })
         : maxQueueSize,
-    staleAfterMs:
-      options?.staleAfterMs !== undefined
-        ? Math.max(0, Math.floor(options.staleAfterMs))
-        : defaults.staleAfterMs,
+    ...(staleAfterMs !== undefined ? { staleAfterMs } : {}),
     weight:
-      options?.weight !== undefined ? Math.max(1, Math.floor(options.weight)) : defaults.weight,
+      options?.weight !== undefined
+        ? normalizeIntegerOption(options.weight, defaults.weight, { min: 1 })
+        : defaults.weight,
   };
 }
 
