feat(Rest): improve global rate limit and invalid request tracking

Ported from discordjs/discord.js#4711

packages/rest/src/lib/REST.ts

@@ -22,6 +22,17 @@ export interface RESTOptions {
 	 * @default {}
 	 */
 	headers: Record<string, string>;
+	/*
+	 * The number of invalid REST requests (those that return 401, 403, or 429) in a 10 minute window between emitted warnings (0 for no warnings).
+	 * That is, if set to 500, warnings will be emitted at invalid request number 500, 1000, 1500, and so on.
+	 * @default 0
+	 */
+	invalidRequestWarningInterval: number;
+	/**
+	 * How many requests to allow sending per second (0 for unlimited, 50 for the standard global limit used by Discord)
+	 * @default 0
+	 */
+	globalLimit: number;
 	/**
 	 * The extra offset to add to rate limits in milliseconds
 	 * @default 50
@@ -81,9 +92,25 @@ export interface RateLimitData {
 	 * If there is no major parameter (e.g: `/bot/gateway`) this will be `global`.
 	 */
 	majorParameter: string;
+	/**
+	 * Whether the rate limit that was reached was the global limit
+	 */
+	global: boolean;
+}
+
+export interface InvalidRequestWarningData {
+	/**
+	 * Number of invalid requests that have been made in the window
+	 */
+	count: number;
+	/**
+	 * Time in ms remaining before the count resets
+	 */
+	remainingTime: number;
 }
 
 interface RestEvents {
+	invalidRequestWarning: [invalidRequestInfo: InvalidRequestWarningData];
 	restDebug: [info: string];
 	rateLimited: [rateLimitInfo: RateLimitData];
 }

packages/rest/src/lib/RequestManager.ts

@@ -108,10 +108,19 @@ export interface RouteData {
  */
 export class RequestManager extends EventEmitter {
 	/**
-	 * A timeout promise that is set when we hit the global rate limit
-	 * @default null
+	 * The number of requests remaining in the global bucket
 	 */
-	public globalTimeout: Promise<void> | null = null;
+	public globalRemaining: number;
+
+	/**
+	 * The promise used to wait out the global rate limit
+	 */
+	public globalDelay: Promise<void> | null = null;
+
+	/**
+	 * The timestamp at which the global bucket resets
+	 */
+	public globalReset = -1;
 
 	/**
 	 * API bucket hashes that are cached from provided routes
@@ -132,6 +141,7 @@ export class RequestManager extends EventEmitter {
 		super();
 		this.options = { ...DefaultRestOptions, ...options };
 		this.options.offset = Math.max(0, this.options.offset);
+		this.globalRemaining = this.options.globalLimit;
 	}
 
 	/**

packages/rest/src/lib/handlers/SequentialHandler.ts

@@ -8,6 +8,15 @@ import type { RequestManager, RouteData } from '../RequestManager';
 import { RESTEvents } from '../utils/constants';
 import { parseResponse } from '../utils/utils';
 
+/* Invalid request limiting is done on a per-IP basis, not a per-token basis.
+ * The best we can do is track invalid counts process-wide (on the theory that
+ * users could have multiple bots run from one process) rather than per-bot.
+ * Therefore, store these at file scope here rather than in the client's
+ * RESTManager object.
+ */
+let invalidCount = 0;
+let invalidCountResetTime: number | null = null;
+
 /**
  * The structure used to handle requests for a given bucket
  */
@@ -58,18 +67,32 @@ export class SequentialHandler {
 		return this.#asyncQueue.remaining === 0 && !this.limited;
 	}
 
+	/**
+	 * If the rate limit bucket is currently limited by the global limit
+	 */
+	private get globalLimited(): boolean {
+		return this.manager.globalRemaining <= 0 && Date.now() < this.manager.globalReset;
+	}
+
+	/**
+	 * If the rate limit bucket is currently limited by its limit
+	 */
+	private get localLimited(): boolean {
+		return this.remaining <= 0 && Date.now() < this.reset;
+	}
+
 	/**
 	 * If the rate limit bucket is currently limited
 	 */
 	private get limited(): boolean {
-		return this.remaining <= 0 && Date.now() < this.reset;
+		return this.globalLimited || this.localLimited;
 	}
 
 	/**
 	 * The time until queued requests can continue
 	 */
 	private get timeToReset(): number {
-		return this.reset - Date.now();
+		return this.reset + this.manager.options.offset - Date.now();
 	}
 
 	/**
@@ -80,6 +103,20 @@ export class SequentialHandler {
 		this.manager.emit(RESTEvents.Debug, `[REST ${this.id}] ${message}`);
 	}
 
+	/**
+	 * Delay all requests for the specified amount of time, handling global rate limits
+	 * @param time The amount of time to delay all requests for
+	 * @returns
+	 */
+	private globalDelayFor(time: number): Promise<void> {
+		return new Promise((resolve) =>
+			setTimeout(() => {
+				this.manager.globalDelay = null;
+				resolve();
+			}, time).unref(),
+		);
+	}
+
 	/**
 	 * Queues a request to be sent
 	 * @param routeId The generalized api route with literal ids for major parameters
@@ -90,23 +127,56 @@ export class SequentialHandler {
 		// Wait for any previous requests to be completed before this one is run
 		await this.#asyncQueue.wait();
 		try {
-			// Wait for any global rate limits to pass before continuing to process requests
-			await this.manager.globalTimeout;
-			// Check if this request handler is currently rate limited
-			if (this.limited) {
+			/*
+			 * After calculations have been done, pre-emptively stop further requests
+			 * Potentially loop until this task can run if e.g. the global rate limit is hit twice
+			 */
+			while (this.limited) {
+				const isGlobal = this.globalLimited;
+				let limit: number;
+				let timeout: number;
+				let delay: Promise<void>;
+
+				if (isGlobal) {
+					// Set RateLimitData based on the globl limit
+					limit = this.manager.options.globalLimit;
+					timeout = this.manager.globalReset + this.manager.options.offset - Date.now();
+					// If this is the first task to reach the global timeout, set the global delay
+					if (!this.manager.globalDelay) {
+						// The global delay function clears the global delay state when it is resolved
+						this.manager.globalDelay = this.globalDelayFor(timeout);
+					}
+					delay = this.manager.globalDelay;
+				} else {
+					// Set RateLimitData based on the route-specific limit
+					limit = this.limit;
+					timeout = this.timeToReset;
+					delay = sleep(timeout);
+				}
 				// Let library users know they have hit a rate limit
 				this.manager.emit(RESTEvents.RateLimited, {
-					timeToReset: this.timeToReset,
-					limit: this.limit,
+					timeToReset: timeout,
+					limit,
 					method: options.method,
 					hash: this.hash,
 					route: routeId.bucketRoute,
 					majorParameter: this.majorParameter,
+					global: isGlobal,
 				});
-				this.debug(`Waiting ${this.timeToReset}ms for rate limit to pass`);
+				if (isGlobal) {
+					this.debug(`We are globally rate limited, blocking all requests for ${timeout}ms`);
+				} else {
+					this.debug(`Waiting ${timeout}ms for rate limit to pass`);
+				}
 				// Wait the remaining time left before the rate limit resets
-				await sleep(this.timeToReset);
+				await delay;
+			}
+			// As the request goes out, update the global usage information
+			if (!this.manager.globalReset || this.manager.globalReset < Date.now()) {
+				this.manager.globalReset = Date.now() + 1000;
+				this.manager.globalRemaining = this.manager.options.globalLimit;
 			}
+			this.manager.globalRemaining--;
 			// Make the request, and return the results
 			return await this.runRequest(routeId, url, options);
 		} finally {
@@ -167,14 +237,41 @@ export class SequentialHandler {
 			this.manager.hashes.set(`${method}:${routeId.bucketRoute}`, hash);
 		}
 
-		// Handle global rate limit
-		if (res.headers.get('X-RateLimit-Global')) {
-			this.debug(`We are globally rate limited, blocking all requests for ${retryAfter}ms`);
-			// Set the manager's global timeout as the promise for other requests to "wait"
-			this.manager.globalTimeout = sleep(retryAfter).then(() => {
-				// After the timer is up, clear the promise
-				this.manager.globalTimeout = null;
-			});
+		// Handle retryAfter, which means we have actually hit a rate limit
+		let sublimitTimeout: number | null = null;
+		if (retryAfter > 0) {
+			if (res.headers.get('X-RateLimit-Global')) {
+				this.manager.globalRemaining = 0;
+				this.manager.globalReset = Date.now() + retryAfter;
+			} else if (!this.localLimited) {
+				/*
+				 * This is a sublimit (e.g. 2 channel name changes/10 minutes) since the headers don't indicate a
+				 * route-wide rate limit. Don't update remaining or reset to avoid rate limiting the whole
+				 * endpoint, just set a reset time on the request itself to avoid retrying too soon.
+				 */
+				sublimitTimeout = retryAfter;
+			}
+		}
+
+		// Count the invalid requests
+		if (res.status === 401 || res.status === 403 || res.status === 429) {
+			if (!invalidCountResetTime || invalidCountResetTime < Date.now()) {
+				invalidCountResetTime = Date.now() + 1000 * 60 * 10;
+				invalidCount = 0;
+			}
+			invalidCount++;
+
+			const emitInvalid =
+				this.manager.listenerCount(RESTEvents.InvalidRequestWarning) &&
+				this.manager.options.invalidRequestWarningInterval > 0 &&
+				invalidCount % this.manager.options.invalidRequestWarningInterval === 0;
+			if (emitInvalid) {
+				// Let library users know periodically about invalid requests
+				this.manager.emit(RESTEvents.InvalidRequestWarning, {
+					count: invalidCount,
+					remainingTime: invalidCountResetTime - Date.now(),
+				});
+			}
 		}
 
 		if (res.ok) {
@@ -190,8 +287,10 @@ export class SequentialHandler {
 					`  Retry After    : ${retryAfter}ms`,
 				].join('\n'),
 			);
-			// Wait the retryAfter amount of time before retrying the request
-			await sleep(retryAfter);
+			// If caused by a sublimit, wait it out here so other requests on the route can be handled
+			if (sublimitTimeout) {
+				await sleep(sublimitTimeout);
+			}
 			// Since this is not a server side issue, the next request should pass, so we don't bump the retries counter
 			return this.runRequest(routeId, url, options, retries);
 		} else if (res.status >= 500 && res.status < 600) {

packages/rest/src/lib/utils/constants.ts

@@ -10,6 +10,8 @@ export const DefaultRestOptions: Required<RESTOptions> = {
 	api: 'https://discord.com/api',
 	cdn: 'https://cdn.discordapp.com',
 	headers: {},
+	invalidRequestWarningInterval: 0,
+	globalLimit: 0,
 	offset: 50,
 	retries: 3,
 	timeout: 15_000,
@@ -22,6 +24,7 @@ export const DefaultRestOptions: Required<RESTOptions> = {
  */
 export const enum RESTEvents {
 	Debug = 'restDebug',
+	InvalidRequestWarning = 'invalidRequestWarning',
 	RateLimited = 'rateLimited',
 }