From 9102fc4eccf64105570bbdf9227d942bcfc1e1ef Mon Sep 17 00:00:00 2001 From: NotDemonix <90858555+NotDemonix@users.noreply.github.com> Date: Tue, 16 Jun 2026 19:25:40 +0200 Subject: [PATCH] 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 | 104 +++++++++++++++++++++++++++-------- packages/rest/src/types.ts | 19 +++++++ 2 files changed, 99 insertions(+), 24 deletions(-) diff --git a/packages/rest/src/manager.ts b/packages/rest/src/manager.ts index 1906462dd..45aa3c643 100644 --- a/packages/rest/src/manager.ts +++ b/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 => { + 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'; +} diff --git a/packages/rest/src/types.ts b/packages/rest/src/types.ts index eb6d1fee7..d2f6c7c6a 100644 --- a/packages/rest/src/types.ts +++ b/packages/rest/src/types.ts @@ -207,6 +207,23 @@ export interface CreateRestManagerOptions { logger?: Pick; /** Events for the rest manager */ events?: Partial; + /** + * 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. */