feat(rest): add request timeout to manage stalled requests (#5085)

* feat(rest): add request timeout feature to manage request retries

* fix(rest): avoid calling Node-only Timeout.unref() on non-Node runtimes

* fix(rest): use try/catch for fetch to guarantee a Response on the success path

* refactor(rest): use AbortSignal.timeout and retry proxy requests on timeout

* refactor(rest): simplify timeout detection and use .catch() in proxy path

* refactor(rest): move isTimeoutError helper to bottom of file

packages/rest/src/manager.ts

@@ -114,6 +114,7 @@ export function createRestManager(options: CreateRestManagerOptions): RestManage
     isProxied: !baseUrl.startsWith(DISCORD_API_URL),
     updateBearerTokenEndpoint: options.proxy?.updateBearerTokenEndpoint,
     maxRetryCount: Infinity,
+    requestTimeout: options.requestTimeout ?? 30000,
     processingRateLimitedPaths: false,
     queues: new Map(),
     rateLimitedPaths: new Map(),
@@ -428,6 +429,25 @@ export function createRestManager(options: CreateRestManagerOptions): RestManage
       const url = `${rest.baseUrl}/v${rest.version}${options.route}`;
       const payload = rest.createRequestBody(options.method, options.requestBodyOptions);
 
+      // Retries a request that was aborted because it hit `rest.requestTimeout`, or rejects it once the retry
+      // budget is exhausted. The caller is responsible for the `invalidBucket`/event bookkeeping beforehand.
+      const handleTimeout = async (options: SendRequestOptions, error: Error): Promise<void> => {
+        if (options.retryCount >= rest.maxRetryCount) {
+          rest.logger.debug(`request to ${url} timed out and exceeded the maximum allowed retries.`);
+          options.reject({
+            ok: false,
+            status: 999,
+            error: 'The request timed out and it maxed out the retries limit.',
+            errorObject: error,
+          });
+          return;
+        }
+
+        rest.logger.debug(`request to ${url} timed out after ${rest.requestTimeout}ms, retrying.`, error);
+        options.retryCount += 1;
+        await options.retryRequest?.(options);
+      };
+
       const loggingHeaders = { ...payload.headers };
 
       if (payload.headers.authorization) {
@@ -435,17 +455,25 @@ export function createRestManager(options: CreateRestManagerOptions): RestManage
         loggingHeaders.authorization = `${authorizationScheme} tokenhere`;
       }
 
-      const request = new Request(url, payload);
+      // Give this attempt a hard deadline. `AbortSignal.timeout` aborts the fetch (and any in-progress body
+      // read) once it fires, which makes the awaited fetch reject so a stalled connection can never keep this
+      // queue's `processPending` loop (and therefore the whole queue) wedged forever. Omitted when disabled.
+      const request = new Request(url, { ...payload, signal: rest.requestTimeout > 0 ? AbortSignal.timeout(rest.requestTimeout) : undefined });
       rest.events.request(request, {
         body: options.requestBodyOptions?.body,
       });
 
       rest.logger.debug(`sending request to ${url}`, 'with payload:', { ...payload, headers: loggingHeaders });
       const response = await fetch(request).catch(async (error) => {
-        rest.logger.debug(`request fetch to ${url} failed.`, error);
         rest.events.requestError(request, error, { body: options.requestBodyOptions?.body });
         // Mark request as completed
         rest.invalidBucket.handleCompletedRequest(999, false);
+
+        // The attempt hit `rest.requestTimeout` and was aborted. Treat it like a transient failure and retry
+        // it through the queue, so a single stalled connection doesn't permanently fail the request.
+        if (isTimeoutError(error)) return await handleTimeout(options, error);
+
+        rest.logger.debug(`request fetch to ${url} failed.`, error);
         options.reject({
           ok: false,
           status: 999,
@@ -610,32 +638,55 @@ export function createRestManager(options: CreateRestManagerOptions): RestManage
           options.headers[rest.authorizationHeader] = rest.authorization;
         }
 
-        const request = new Request(`${rest.baseUrl}/v${rest.version}${route}`, rest.createRequestBody(method, options));
-        rest.events.request(request, {
-          body: options?.body,
-        });
+        const url = `${rest.baseUrl}/v${rest.version}${route}`;
 
-        const result = await fetch(request);
-
-        // Sometimes the Content-Type may be "application/json; charset=utf-8", for this reason, we need to check the start of the header
-        const body = await (result.headers.get('Content-Type')?.startsWith('application/json') ? result.json() : result.text()).catch(() => null);
-
-        rest.events.response(request, result, {
-          requestBody: options?.body,
-          responseBody: body,
-        });
-
-        if (!result.ok) {
-          error.cause = Object.assign(Object.create(baseErrorPrototype), {
-            ok: false,
-            status: result.status,
-            body,
+        // Retry on timeout up to `maxRetryCount`, mirroring the queued `sendRequest` path, so a stalled proxy
+        // connection doesn't permanently fail the request (and can never hang the caller forever).
+        for (let retryCount = 0; ; retryCount++) {
+          // Give the request to the proxy a hard deadline too via `AbortSignal.timeout` (omitted when disabled).
+          const request = new Request(url, {
+            ...rest.createRequestBody(method, options),
+            signal: rest.requestTimeout > 0 ? AbortSignal.timeout(rest.requestTimeout) : undefined,
+          });
+          rest.events.request(request, {
+            body: options?.body,
           });
 
-          throw error;
-        }
+          const result = await fetch(request).catch((fetchError) => {
+            rest.events.requestError(request, fetchError, { body: options?.body });
 
-        return result.status !== 204 ? (typeof body === 'string' ? JSON.parse(body) : body) : undefined;
+            // The attempt hit `rest.requestTimeout` and was aborted; retry it until the budget is exhausted.
+            if (isTimeoutError(fetchError) && retryCount < rest.maxRetryCount) {
+              rest.logger.debug(`request to proxy ${url} timed out after ${rest.requestTimeout}ms, retrying.`, fetchError);
+              return undefined;
+            }
+
+            throw fetchError;
+          });
+
+          // If result is undefined, the attempt timed out and is being retried.
+          if (!result) continue;
+
+          // Sometimes the Content-Type may be "application/json; charset=utf-8", for this reason, we need to check the start of the header
+          const body = await (result.headers.get('Content-Type')?.startsWith('application/json') ? result.json() : result.text()).catch(() => null);
+
+          rest.events.response(request, result, {
+            requestBody: options?.body,
+            responseBody: body,
+          });
+
+          if (!result.ok) {
+            error.cause = Object.assign(Object.create(baseErrorPrototype), {
+              ok: false,
+              status: result.status,
+              body,
+            });
+
+            throw error;
+          }
+
+          return result.status !== 204 ? (typeof body === 'string' ? JSON.parse(body) : body) : undefined;
+        }
       }
 
       return await new Promise(async (resolve, reject) => {
@@ -1916,3 +1967,8 @@ enum HttpResponseCode {
   /** This request got rate limited. */
   TooManyRequests = 429,
 }
+
+/** Whether an error is the `TimeoutError` thrown by `AbortSignal.timeout()`. */
+function isTimeoutError(error: unknown): boolean {
+  return error instanceof DOMException && error.name === 'TimeoutError';
+}

packages/rest/src/types.ts

@@ -207,6 +207,23 @@ export interface CreateRestManagerOptions {
   logger?: Pick<typeof logger, 'debug' | 'info' | 'warn' | 'error' | 'fatal'>;
   /** Events for the rest manager */
   events?: Partial<RestManagerEvents>;
+  /**
+   * The maximum time in milliseconds a single request attempt may take before it is aborted.
+   *
+   * @remarks
+   * This is a total deadline for each attempt (it also covers reading the response body), not a per-chunk timeout.
+   * When an attempt times out it is retried through the queue up to {@link RestManager.maxRetryCount} times before failing.
+   * Without it, a connection that stalls after connecting could keep a queue from ever progressing.
+   *
+   * Because it is a total deadline rather than a per-chunk one, a slow but healthy request (e.g. uploading a
+   * large attachment over a slow connection) can legitimately exceed it and be aborted/retried. Raise this value
+   * for upload-heavy bots if you see such requests timing out.
+   *
+   * Set to `0` to disable it and rely on the runtime's default fetch timeouts.
+   *
+   * @default 30000 // 30 seconds
+   */
+  requestTimeout?: number;
 }
 
 export interface RestManager {
@@ -236,6 +253,8 @@ export interface RestManager {
   updateBearerTokenEndpoint?: string;
   /** The maximum amount of times a request should be retried. Defaults to Infinity */
   maxRetryCount: number;
+  /** The maximum time in milliseconds a single request attempt may take before it is aborted and retried. Defaults to 30000 (30 seconds). Set to 0 to disable. */
+  requestTimeout: number;
   /** Whether or not the manager is rate limited globally across all requests. Defaults to false. */
   globallyRateLimited: boolean;
   /** Whether or not the rate limited paths are being processed to allow requests to be made once time is up. Defaults to false. */