From fb2400274358dc81fd7a1b07f89766f0df87094c Mon Sep 17 00:00:00 2001 From: Vendicated Date: Fri, 21 May 2021 23:14:19 +0200 Subject: [PATCH 01/13] feat(RequestHandler): optionally throw on rate limit this can be configured via the disableRateLimitQueue client option, which may be an array of routes or a function --- src/client/Client.js | 6 +++++ src/index.js | 1 + src/rest/RateLimitError.js | 54 ++++++++++++++++++++++++++++++++++++++ src/rest/RequestHandler.js | 45 ++++++++++++++++++++++++++++++- typings/index.d.ts | 12 ++++++--- 5 files changed, 114 insertions(+), 4 deletions(-) create mode 100644 src/rest/RateLimitError.js diff --git a/src/client/Client.js b/src/client/Client.js index e68da9ce4524..2d1a28198529 100644 --- a/src/client/Client.js +++ b/src/client/Client.js @@ -492,6 +492,12 @@ class Client extends BaseClient { if (typeof options.retryLimit !== 'number' || isNaN(options.retryLimit)) { throw new TypeError('CLIENT_INVALID_OPTION', 'retryLimit', 'a number'); } + if ( + typeof options.disableRateLimitQueue !== 'undefined' && + !(typeof options.disableRateLimitQueue === 'function' || Array.isArray(options.disableRateLimitQueue)) + ) { + throw new TypeError('CLIENT_INVALID_OPTION', 'disableRateLimitQueue', 'an array or a function'); + } } } diff --git a/src/index.js b/src/index.js index 92fd0789bd40..17f90a813688 100644 --- a/src/index.js +++ b/src/index.js @@ -21,6 +21,7 @@ module.exports = { BaseManager: require('./managers/BaseManager'), DiscordAPIError: require('./rest/DiscordAPIError'), HTTPError: require('./rest/HTTPError'), + RateLimitError: require('./rest/RateLimitError'), MessageFlags: require('./util/MessageFlags'), Intents: require('./util/Intents'), Permissions: require('./util/Permissions'), diff --git a/src/rest/RateLimitError.js b/src/rest/RateLimitError.js new file mode 100644 index 000000000000..29a02e18da2e --- /dev/null +++ b/src/rest/RateLimitError.js @@ -0,0 +1,54 @@ +'use strict'; + +/** + * Represents a RateLimit error from a request. + * @extends Error + */ +class RateLimitError extends Error { + constructor({ timeout, limit, method, path, route, global }) { + super(`A ${global ? 'global ' : ''}rate limit was hit on route ${route}`); + + /** + * The name of the error + * @type {string} + */ + this.name = 'RateLimitError'; + + /** + * HTTP error code returned from the request + * @type {number} + */ + this.timeout = timeout; + + /** + * The HTTP method used for the request + * @type {string} + */ + this.method = method; + + /** + * The path of the request relative to the HTTP endpoint + * @type {string} + */ + this.path = path; + + /** + * The route of the request relative to the HTTP endpoint + */ + this.route = route; + + /** + * Whether this rate limit is global + * @type {boolean} + */ + this.global = global; + + /** + * The rate limit of this endpoint + * @type {number} + */ + this.limit = limit; + } +} + +module.exports = RateLimitError; diff --git a/src/rest/RequestHandler.js b/src/rest/RequestHandler.js index 2ede1f5a7a01..b55df91e2515 100644 --- a/src/rest/RequestHandler.js +++ b/src/rest/RequestHandler.js @@ -3,6 +3,7 @@ const AsyncQueue = require('./AsyncQueue'); const DiscordAPIError = require('./DiscordAPIError'); const HTTPError = require('./HTTPError'); +const RateLimitError = require('./RateLimitError'); const { Events: { RATE_LIMIT, INVALID_REQUEST_WARNING }, } = require('../util/Constants'); @@ -77,6 +78,28 @@ class RequestHandler { }); } + onRateLimit(request, limit, timeout, isGlobal) { + if (this.manager.client.options.disableRateLimitQueue) { + const rateLimitData = { + timeout, + limit, + method: request.method, + path: request.path, + route: request.route, + global: isGlobal, + }; + const shouldThrow = + typeof this.manager.client.options.disableRateLimitQueue === 'function' + ? this.manager.client.options.disableRateLimitQueue(rateLimitData) + : this.manager.client.options.disableRateLimitQueue.some(route => + rateLimitData.route.startsWith(`/${route}/`), + ); + if (shouldThrow) { + throw new RateLimitError(rateLimitData); + } + } + } + async execute(request) { /* * After calculations have been done, pre-emptively stop further requests @@ -90,6 +113,9 @@ class RequestHandler { // Set the variables based on the global rate limit limit = this.manager.globalLimit; timeout = this.manager.globalReset + this.manager.client.options.restTimeOffset - Date.now(); + + this.onRateLimit(request, limit, timeout, isGlobal); + // If this is the first task to reach the global timeout, set the global delay if (!this.manager.globalDelay) { // The global delay function should clear the global delay state when it is resolved @@ -100,6 +126,9 @@ class RequestHandler { // Set the variables based on the route-specific rate limit limit = this.limit; timeout = this.reset + this.manager.client.options.restTimeOffset - Date.now(); + + this.onRateLimit(request, limit, timeout, isGlobal); + delayPromise = Util.delayFor(timeout); } @@ -223,8 +252,22 @@ class RequestHandler { if (res.status >= 400 && res.status < 500) { // Handle ratelimited requests if (res.status === 429) { - // A ratelimit was hit - this should never happen + // A ratelimit was hit - this should only happen if client is already rate limited when connecting this.manager.client.emit('debug', `429 hit on route ${request.route}${sublimitTimeout ? ' for sublimit' : ''}`); + + const isGlobal = this.globalLimited; + let limit, timeout; + if (isGlobal) { + // Set the variables based on the global rate limit + limit = this.manager.globalLimit; + timeout = this.manager.globalReset + this.manager.client.options.restTimeOffset - Date.now(); + } else { + // Set the variables based on the route-specific rate limit + limit = this.limit; + timeout = this.reset + this.manager.client.options.restTimeOffset - Date.now(); + } + this.onRateLimit(request, limit, timeout, isGlobal); + // If caused by a sublimit, wait it out here so other requests on the route can be handled if (sublimitTimeout) { await Util.delayFor(sublimitTimeout); diff --git a/typings/index.d.ts b/typings/index.d.ts index 0e574ac27145..8ad16a38970a 100644 --- a/typings/index.d.ts +++ b/typings/index.d.ts @@ -461,9 +461,7 @@ declare module 'discord.js' { Endpoints: { botGateway: string; invite: (root: string, code: string) => string; - CDN: ( - root: string, - ) => { + CDN: (root: string) => { Asset: (name: string) => string; DefaultAvatar: (id: string | number) => string; Emoji: (emojiID: string, format: 'png' | 'gif') => string; @@ -2659,6 +2657,7 @@ declare module 'discord.js' { intents: BitFieldResolvable; ws?: WebSocketOptions; http?: HTTPOptions; + disableRateLimitQueue: string[] | ((data: RateLimitData) => boolean); } type ClientPresenceStatus = 'online' | 'idle' | 'dnd'; @@ -3489,6 +3488,13 @@ declare module 'discord.js' { global: boolean; } + interface RateLimitError extends RateLimitData {} + + export class RateLimitError extends Error { + constructor(data: RateLimitData); + public name: 'RateLimitError'; + } + interface InvalidRequestWarningData { count: number; remainingTime: number; From 7b58f0f6da2d0ec51e531bc4669ead081269d4e1 Mon Sep 17 00:00:00 2001 From: Vendicated Date: Fri, 21 May 2021 23:28:02 +0200 Subject: [PATCH 02/13] fix(Typings): mark option as optional --- typings/index.d.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/typings/index.d.ts b/typings/index.d.ts index 8ad16a38970a..7b94ee78e3ed 100644 --- a/typings/index.d.ts +++ b/typings/index.d.ts @@ -2657,7 +2657,7 @@ declare module 'discord.js' { intents: BitFieldResolvable; ws?: WebSocketOptions; http?: HTTPOptions; - disableRateLimitQueue: string[] | ((data: RateLimitData) => boolean); + disableRateLimitQueue?: string[] | ((data: RateLimitData) => boolean); } type ClientPresenceStatus = 'online' | 'idle' | 'dnd'; From 698a125bc2f108e7a22156403390867a1da5f43d Mon Sep 17 00:00:00 2001 From: Vendicated Date: Fri, 21 May 2021 23:40:22 +0200 Subject: [PATCH 03/13] fix(Typings): disable no-empty-interface for interface merge --- typings/index.d.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/typings/index.d.ts b/typings/index.d.ts index 7b94ee78e3ed..6c6398967ddc 100644 --- a/typings/index.d.ts +++ b/typings/index.d.ts @@ -3488,8 +3488,8 @@ declare module 'discord.js' { global: boolean; } + // tslint:disable-next-line:no-empty-interface - Merge RateLimitData interface to not have to type it again interface RateLimitError extends RateLimitData {} - export class RateLimitError extends Error { constructor(data: RateLimitData); public name: 'RateLimitError'; From f57b151de8565699e35c65396c0b94f5bc4d7c54 Mon Sep 17 00:00:00 2001 From: Vendicated Date: Fri, 21 May 2021 23:41:02 +0200 Subject: [PATCH 04/13] fix(Docs): fix route jsdoc --- src/rest/RateLimitError.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/rest/RateLimitError.js b/src/rest/RateLimitError.js index 29a02e18da2e..6ac86b7a1abf 100644 --- a/src/rest/RateLimitError.js +++ b/src/rest/RateLimitError.js @@ -34,6 +34,7 @@ class RateLimitError extends Error { /** * The route of the request relative to the HTTP endpoint + * @type {string} */ this.route = route; From 96a6ff54bb880d5081ea81ceac142e15fe45bcf3 Mon Sep 17 00:00:00 2001 From: Vendicated Date: Sat, 22 May 2021 18:32:39 +0200 Subject: [PATCH 05/13] docs: docs docs docs! --- src/rest/RateLimitError.js | 4 ++-- src/util/Constants.js | 21 +++++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/rest/RateLimitError.js b/src/rest/RateLimitError.js index 6ac86b7a1abf..f551539e36ec 100644 --- a/src/rest/RateLimitError.js +++ b/src/rest/RateLimitError.js @@ -15,7 +15,7 @@ class RateLimitError extends Error { this.name = 'RateLimitError'; /** - * HTTP error code returned from the request + * Time until this rate limit ends, in ms * @type {number} */ this.timeout = timeout; @@ -45,7 +45,7 @@ class RateLimitError extends Error { this.global = global; /** - * The rate limit of this endpoint + * The maximum amount of requests of this end point * @type {number} */ this.limit = limit; diff --git a/src/util/Constants.js b/src/util/Constants.js index 9d7aead77721..e92ceaf06691 100644 --- a/src/util/Constants.js +++ b/src/util/Constants.js @@ -3,6 +3,24 @@ const Package = (exports.Package = require('../../package.json')); const { Error, RangeError } = require('../errors'); +/** + * Rate limit data + * @typedef {Object} RateLimitData + * @property {number} timeout Time until this rate limit ends, in ms + * @property {number} limit The maximum amount of requests of this end point + * @property {string} method The http method of this request + * @property {string} path The path of the request relative to the HTTP endpoint + * @property {string} route The route of the request relative to the HTTP endpoint + * @property {boolean} global Whether this is a global rate limit + */ + +/** + * Whether this rate limit should throw an Error + * @typedef {Function} RateLimitQueueFilter + * @param {RateLimitData} rateLimitData The data of this rate limit + * @returns {boolean|Promise} + */ + /** * Options for a client. * @typedef {Object} ClientOptions @@ -34,6 +52,9 @@ const { Error, RangeError } = require('../errors'); * (or 0 for never) * @property {number} [restGlobalRateLimit=0] How many requests to allow sending per second (0 for unlimited, 50 for * the standard global limit used by Discord) + * @property {string[]|RateLimitQueueFilter} [disableRateLimitQueue] Decides how rate limits (429) should be handled. + * If this option is an array containing the route of the request or a function returning true, a {@link RateLimitError} + * will be thrown. Otherwise the request will be queued for later * @property {number} [retryLimit=1] How many times to retry on 5XX errors (Infinity for indefinite amount of retries) * @property {PresenceData} [presence={}] Presence data to use upon login * @property {IntentsResolvable} intents Intents to enable for this connection From 1ada68fec905ea28581d0571727a42e2059cbcc1 Mon Sep 17 00:00:00 2001 From: Vendicated Date: Sat, 22 May 2021 18:40:20 +0200 Subject: [PATCH 06/13] fix: implement suggested changes, and allow promises --- src/rest/RequestHandler.js | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/rest/RequestHandler.js b/src/rest/RequestHandler.js index b55df91e2515..d2924dcfdbf2 100644 --- a/src/rest/RequestHandler.js +++ b/src/rest/RequestHandler.js @@ -78,8 +78,12 @@ class RequestHandler { }); } - onRateLimit(request, limit, timeout, isGlobal) { - if (this.manager.client.options.disableRateLimitQueue) { + /* + * Determines whether the request should be queued or whether a RateLimitError should be thrown + */ + async onRateLimit(request, limit, timeout, isGlobal) { + const { options } = this.manager.client; + if (options.disableRateLimitQueue) { const rateLimitData = { timeout, limit, @@ -89,11 +93,9 @@ class RequestHandler { global: isGlobal, }; const shouldThrow = - typeof this.manager.client.options.disableRateLimitQueue === 'function' - ? this.manager.client.options.disableRateLimitQueue(rateLimitData) - : this.manager.client.options.disableRateLimitQueue.some(route => - rateLimitData.route.startsWith(`/${route}/`), - ); + typeof options.disableRateLimitQueue === 'function' + ? await options.disableRateLimitQueue(rateLimitData) + : options.disableRateLimitQueue.some(route => rateLimitData.route.startsWith(`/${route}/`)); if (shouldThrow) { throw new RateLimitError(rateLimitData); } @@ -114,7 +116,7 @@ class RequestHandler { limit = this.manager.globalLimit; timeout = this.manager.globalReset + this.manager.client.options.restTimeOffset - Date.now(); - this.onRateLimit(request, limit, timeout, isGlobal); + await this.onRateLimit(request, limit, timeout, isGlobal); // eslint-disable-line no-await-in-loop // If this is the first task to reach the global timeout, set the global delay if (!this.manager.globalDelay) { @@ -127,7 +129,7 @@ class RequestHandler { limit = this.limit; timeout = this.reset + this.manager.client.options.restTimeOffset - Date.now(); - this.onRateLimit(request, limit, timeout, isGlobal); + await this.onRateLimit(request, limit, timeout, isGlobal); // eslint-disable-line no-await-in-loop delayPromise = Util.delayFor(timeout); } @@ -266,7 +268,7 @@ class RequestHandler { limit = this.limit; timeout = this.reset + this.manager.client.options.restTimeOffset - Date.now(); } - this.onRateLimit(request, limit, timeout, isGlobal); + await this.onRateLimit(request, limit, timeout, isGlobal); // If caused by a sublimit, wait it out here so other requests on the route can be handled if (sublimitTimeout) { From f16230172ca3242a5dcfdc3f383cf0f4f9109f78 Mon Sep 17 00:00:00 2001 From: Vendicated Date: Sat, 22 May 2021 18:51:11 +0200 Subject: [PATCH 07/13] types: update typings; move RateLimitError up to other classes --- typings/index.d.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/typings/index.d.ts b/typings/index.d.ts index 6c6398967ddc..b2d9cc43c700 100644 --- a/typings/index.d.ts +++ b/typings/index.d.ts @@ -1020,6 +1020,13 @@ declare module 'discord.js' { public path: string; } + // tslint:disable-next-line:no-empty-interface - Merge RateLimitData into RateLimitError to not have to type it again + interface RateLimitError extends RateLimitData {} + export class RateLimitError extends Error { + constructor(data: RateLimitData); + public name: 'RateLimitError'; + } + export class Integration extends Base { constructor(client: Client, data: object, guild: Guild); public account: IntegrationAccount; @@ -2657,7 +2664,7 @@ declare module 'discord.js' { intents: BitFieldResolvable; ws?: WebSocketOptions; http?: HTTPOptions; - disableRateLimitQueue?: string[] | ((data: RateLimitData) => boolean); + disableRateLimitQueue?: string[] | ((data: RateLimitData) => boolean | Promise); } type ClientPresenceStatus = 'online' | 'idle' | 'dnd'; @@ -3488,13 +3495,6 @@ declare module 'discord.js' { global: boolean; } - // tslint:disable-next-line:no-empty-interface - Merge RateLimitData interface to not have to type it again - interface RateLimitError extends RateLimitData {} - export class RateLimitError extends Error { - constructor(data: RateLimitData); - public name: 'RateLimitError'; - } - interface InvalidRequestWarningData { count: number; remainingTime: number; From 20f3ad8c434a00a6fb629f278f1a036d6690a7d0 Mon Sep 17 00:00:00 2001 From: Vendicated Date: Sun, 23 May 2021 15:15:44 +0200 Subject: [PATCH 08/13] refactor: implement requested changes --- src/rest/RequestHandler.js | 29 +++++++++++++++-------------- src/util/Constants.js | 2 +- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/src/rest/RequestHandler.js b/src/rest/RequestHandler.js index d2924dcfdbf2..6653663a31ba 100644 --- a/src/rest/RequestHandler.js +++ b/src/rest/RequestHandler.js @@ -95,7 +95,7 @@ class RequestHandler { const shouldThrow = typeof options.disableRateLimitQueue === 'function' ? await options.disableRateLimitQueue(rateLimitData) - : options.disableRateLimitQueue.some(route => rateLimitData.route.startsWith(`/${route}/`)); + : options.disableRateLimitQueue.some(route => rateLimitData.route.startsWith(route.toLowerCase())); if (shouldThrow) { throw new RateLimitError(rateLimitData); } @@ -115,23 +115,10 @@ class RequestHandler { // Set the variables based on the global rate limit limit = this.manager.globalLimit; timeout = this.manager.globalReset + this.manager.client.options.restTimeOffset - Date.now(); - - await this.onRateLimit(request, limit, timeout, isGlobal); // eslint-disable-line no-await-in-loop - - // If this is the first task to reach the global timeout, set the global delay - if (!this.manager.globalDelay) { - // The global delay function should clear the global delay state when it is resolved - this.manager.globalDelay = this.globalDelayFor(timeout); - } - delayPromise = this.manager.globalDelay; } else { // Set the variables based on the route-specific rate limit limit = this.limit; timeout = this.reset + this.manager.client.options.restTimeOffset - Date.now(); - - await this.onRateLimit(request, limit, timeout, isGlobal); // eslint-disable-line no-await-in-loop - - delayPromise = Util.delayFor(timeout); } if (this.manager.client.listenerCount(RATE_LIMIT)) { @@ -156,6 +143,20 @@ class RequestHandler { }); } + // Determine whether RateLimitError should be thrown + await this.onRateLimit(request, limit, timeout, isGlobal); // eslint-disable-line no-await-in-loop + + if (isGlobal) { + // If this is the first task to reach the global timeout, set the global delay + if (!this.manager.globalDelay) { + // The global delay function should clear the global delay state when it is resolved + this.manager.globalDelay = this.globalDelayFor(timeout); + } + delayPromise = this.manager.globalDelay; + } else { + delayPromise = Util.delayFor(timeout); + } + // Wait for the timeout to expire in order to avoid an actual 429 await delayPromise; // eslint-disable-line no-await-in-loop } diff --git a/src/util/Constants.js b/src/util/Constants.js index e92ceaf06691..c24c4ef3b6fc 100644 --- a/src/util/Constants.js +++ b/src/util/Constants.js @@ -7,7 +7,7 @@ const { Error, RangeError } = require('../errors'); * Rate limit data * @typedef {Object} RateLimitData * @property {number} timeout Time until this rate limit ends, in ms - * @property {number} limit The maximum amount of requests of this end point + * @property {number} limit The maximum amount of requests of this endpoint * @property {string} method The http method of this request * @property {string} path The path of the request relative to the HTTP endpoint * @property {string} route The route of the request relative to the HTTP endpoint From 5522066fef7efc464d5cedb4ea1ffd1162588e6b Mon Sep 17 00:00:00 2001 From: Vendicated Date: Tue, 25 May 2021 01:52:49 +0200 Subject: [PATCH 09/13] revert: change comment back --- src/rest/RequestHandler.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rest/RequestHandler.js b/src/rest/RequestHandler.js index 6653663a31ba..2cc28d3354b7 100644 --- a/src/rest/RequestHandler.js +++ b/src/rest/RequestHandler.js @@ -255,7 +255,7 @@ class RequestHandler { if (res.status >= 400 && res.status < 500) { // Handle ratelimited requests if (res.status === 429) { - // A ratelimit was hit - this should only happen if client is already rate limited when connecting + // A ratelimit was hit - this should never happen this.manager.client.emit('debug', `429 hit on route ${request.route}${sublimitTimeout ? ' for sublimit' : ''}`); const isGlobal = this.globalLimited; From 13b724ddec08c6e941c3ac062fefa28ca55ee141 Mon Sep 17 00:00:00 2001 From: Ven Date: Thu, 27 May 2021 08:19:14 +0600 Subject: [PATCH 10/13] Update src/rest/RequestHandler.js Co-authored-by: SpaceEEC --- src/rest/RequestHandler.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rest/RequestHandler.js b/src/rest/RequestHandler.js index 2cc28d3354b7..c88ed8549f45 100644 --- a/src/rest/RequestHandler.js +++ b/src/rest/RequestHandler.js @@ -143,7 +143,7 @@ class RequestHandler { }); } - // Determine whether RateLimitError should be thrown + // Determine whether a RateLimitError should be thrown await this.onRateLimit(request, limit, timeout, isGlobal); // eslint-disable-line no-await-in-loop if (isGlobal) { From 5e10d37ae7eb3a5eae21a68eb6412e4f08b4e543 Mon Sep 17 00:00:00 2001 From: Vendicated Date: Thu, 27 May 2021 04:27:06 +0200 Subject: [PATCH 11/13] docs: make explanation more specific --- src/util/Constants.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/util/Constants.js b/src/util/Constants.js index c24c4ef3b6fc..fbfb8193f0db 100644 --- a/src/util/Constants.js +++ b/src/util/Constants.js @@ -52,9 +52,10 @@ const { Error, RangeError } = require('../errors'); * (or 0 for never) * @property {number} [restGlobalRateLimit=0] How many requests to allow sending per second (0 for unlimited, 50 for * the standard global limit used by Discord) - * @property {string[]|RateLimitQueueFilter} [disableRateLimitQueue] Decides how rate limits (429) should be handled. - * If this option is an array containing the route of the request or a function returning true, a {@link RateLimitError} - * will be thrown. Otherwise the request will be queued for later + * @property {string[]|RateLimitQueueFilter} [disableRateLimitQueue] Decides how rate limits and pre-emptive throttles + * should be handled. If this option is an array containing the prefix of the request route (e.g. /channels to match any + * route starting with /channels, such as /channels/222197033908436994/messages) or a function returning true, a + * {@link RateLimitError} will be thrown. Otherwise the request will be queued for later * @property {number} [retryLimit=1] How many times to retry on 5XX errors (Infinity for indefinite amount of retries) * @property {PresenceData} [presence={}] Presence data to use upon login * @property {IntentsResolvable} intents Intents to enable for this connection From b36339903ed3ae259a5ba929d3fdde3904cd1816 Mon Sep 17 00:00:00 2001 From: Vendicated Date: Mon, 7 Jun 2021 04:08:29 +0200 Subject: [PATCH 12/13] refactor: implement requested changes --- src/client/Client.js | 6 +++--- src/rest/RequestHandler.js | 32 ++++++++++++++++---------------- src/util/Constants.js | 2 +- typings/index.d.ts | 2 +- 4 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/client/Client.js b/src/client/Client.js index 2d1a28198529..845c8fa88f17 100644 --- a/src/client/Client.js +++ b/src/client/Client.js @@ -493,10 +493,10 @@ class Client extends BaseClient { throw new TypeError('CLIENT_INVALID_OPTION', 'retryLimit', 'a number'); } if ( - typeof options.disableRateLimitQueue !== 'undefined' && - !(typeof options.disableRateLimitQueue === 'function' || Array.isArray(options.disableRateLimitQueue)) + typeof options.rejectOnRateLimit !== 'undefined' && + !(typeof options.rejectOnRateLimit === 'function' || Array.isArray(options.rejectOnRateLimit)) ) { - throw new TypeError('CLIENT_INVALID_OPTION', 'disableRateLimitQueue', 'an array or a function'); + throw new TypeError('CLIENT_INVALID_OPTION', 'rejectOnRateLimit', 'an array or a function'); } } } diff --git a/src/rest/RequestHandler.js b/src/rest/RequestHandler.js index c88ed8549f45..7d134db63d7a 100644 --- a/src/rest/RequestHandler.js +++ b/src/rest/RequestHandler.js @@ -83,22 +83,22 @@ class RequestHandler { */ async onRateLimit(request, limit, timeout, isGlobal) { const { options } = this.manager.client; - if (options.disableRateLimitQueue) { - const rateLimitData = { - timeout, - limit, - method: request.method, - path: request.path, - route: request.route, - global: isGlobal, - }; - const shouldThrow = - typeof options.disableRateLimitQueue === 'function' - ? await options.disableRateLimitQueue(rateLimitData) - : options.disableRateLimitQueue.some(route => rateLimitData.route.startsWith(route.toLowerCase())); - if (shouldThrow) { - throw new RateLimitError(rateLimitData); - } + if (!options.rejectOnRateLimit) return; + + const rateLimitData = { + timeout, + limit, + method: request.method, + path: request.path, + route: request.route, + global: isGlobal, + }; + const shouldThrow = + typeof options.rejectOnRateLimit === 'function' + ? await options.rejectOnRateLimit(rateLimitData) + : options.rejectOnRateLimit.some(route => rateLimitData.route.startsWith(route.toLowerCase())); + if (shouldThrow) { + throw new RateLimitError(rateLimitData); } } diff --git a/src/util/Constants.js b/src/util/Constants.js index fbfb8193f0db..f4e0b4b9b9ea 100644 --- a/src/util/Constants.js +++ b/src/util/Constants.js @@ -52,7 +52,7 @@ const { Error, RangeError } = require('../errors'); * (or 0 for never) * @property {number} [restGlobalRateLimit=0] How many requests to allow sending per second (0 for unlimited, 50 for * the standard global limit used by Discord) - * @property {string[]|RateLimitQueueFilter} [disableRateLimitQueue] Decides how rate limits and pre-emptive throttles + * @property {string[]|RateLimitQueueFilter} [rejectOnRateLimit] Decides how rate limits and pre-emptive throttles * should be handled. If this option is an array containing the prefix of the request route (e.g. /channels to match any * route starting with /channels, such as /channels/222197033908436994/messages) or a function returning true, a * {@link RateLimitError} will be thrown. Otherwise the request will be queued for later diff --git a/typings/index.d.ts b/typings/index.d.ts index b2d9cc43c700..36ace169a331 100644 --- a/typings/index.d.ts +++ b/typings/index.d.ts @@ -2664,7 +2664,7 @@ declare module 'discord.js' { intents: BitFieldResolvable; ws?: WebSocketOptions; http?: HTTPOptions; - disableRateLimitQueue?: string[] | ((data: RateLimitData) => boolean | Promise); + rejectOnRateLimit?: string[] | ((data: RateLimitData) => boolean | Promise); } type ClientPresenceStatus = 'online' | 'idle' | 'dnd'; From 44b6329459abd31589804ce042f32d344e1345da Mon Sep 17 00:00:00 2001 From: Vendicated Date: Tue, 8 Jun 2021 16:05:53 +0200 Subject: [PATCH 13/13] fix: correctly prevent future requests on global rate limit --- src/rest/RequestHandler.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/rest/RequestHandler.js b/src/rest/RequestHandler.js index 7d134db63d7a..b291325ff5ab 100644 --- a/src/rest/RequestHandler.js +++ b/src/rest/RequestHandler.js @@ -143,9 +143,6 @@ class RequestHandler { }); } - // Determine whether a RateLimitError should be thrown - await this.onRateLimit(request, limit, timeout, isGlobal); // eslint-disable-line no-await-in-loop - if (isGlobal) { // If this is the first task to reach the global timeout, set the global delay if (!this.manager.globalDelay) { @@ -157,6 +154,9 @@ class RequestHandler { delayPromise = Util.delayFor(timeout); } + // Determine whether a RateLimitError should be thrown + await this.onRateLimit(request, limit, timeout, isGlobal); // eslint-disable-line no-await-in-loop + // Wait for the timeout to expire in order to avoid an actual 429 await delayPromise; // eslint-disable-line no-await-in-loop }