From 16f261e773a353c54a75f38008f9b28435ae6603 Mon Sep 17 00:00:00 2001 From: Ven Date: Wed, 9 Jun 2021 18:45:04 +1100 Subject: [PATCH] feat(Rest): optional ratelimit errors (#5659) Co-authored-by: SpaceEEC --- src/client/Client.js | 6 ++++ src/index.js | 1 + src/rest/RateLimitError.js | 55 ++++++++++++++++++++++++++++++++++ src/rest/RequestHandler.js | 60 +++++++++++++++++++++++++++++++++----- src/util/Constants.js | 22 ++++++++++++++ typings/index.d.ts | 8 +++++ 6 files changed, 145 insertions(+), 7 deletions(-) create mode 100644 src/rest/RateLimitError.js diff --git a/src/client/Client.js b/src/client/Client.js index d522f2430c9e..f20a1eded44b 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.rejectOnRateLimit !== 'undefined' && + !(typeof options.rejectOnRateLimit === 'function' || Array.isArray(options.rejectOnRateLimit)) + ) { + throw new TypeError('CLIENT_INVALID_OPTION', 'rejectOnRateLimit', 'an array or a function'); + } } } diff --git a/src/index.js b/src/index.js index a358a303bbc8..5e14595f7961 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..f551539e36ec --- /dev/null +++ b/src/rest/RateLimitError.js @@ -0,0 +1,55 @@ +'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'; + + /** + * Time until this rate limit ends, in ms + * @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 + * @type {string} + */ + this.route = route; + + /** + * Whether this rate limit is global + * @type {boolean} + */ + this.global = global; + + /** + * The maximum amount of requests of this end point + * @type {number} + */ + this.limit = limit; + } +} + +module.exports = RateLimitError; diff --git a/src/rest/RequestHandler.js b/src/rest/RequestHandler.js index 1c27a172a28f..1ce8118b6ae9 100644 --- a/src/rest/RequestHandler.js +++ b/src/rest/RequestHandler.js @@ -3,6 +3,7 @@ const { AsyncQueue } = require('@sapphire/async-queue'); 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,30 @@ class RequestHandler { }); } + /* + * 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.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); + } + } + async execute(request) { /* * After calculations have been done, pre-emptively stop further requests @@ -90,17 +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(); - // 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(); - delayPromise = Util.delayFor(timeout); } if (this.manager.client.listenerCount(RATE_LIMIT)) { @@ -125,6 +143,20 @@ class RequestHandler { }); } + 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); + } + + // 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 } @@ -225,6 +257,20 @@ class RequestHandler { if (res.status === 429) { // 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; + 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(); + } + 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) { await Util.delayFor(sublimitTimeout); diff --git a/src/util/Constants.js b/src/util/Constants.js index 02c44ab0cff1..23741b321d25 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 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 + * @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,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} [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 * @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 diff --git a/typings/index.d.ts b/typings/index.d.ts index 8ee313f3b6e4..9b1ce4696d9f 100644 --- a/typings/index.d.ts +++ b/typings/index.d.ts @@ -1081,6 +1081,13 @@ declare module 'discord.js' { public requestData: HTTPErrorData; } + // 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: unknown, guild: Guild); public account: IntegrationAccount; @@ -2875,6 +2882,7 @@ declare module 'discord.js' { intents: BitFieldResolvable; ws?: WebSocketOptions; http?: HTTPOptions; + rejectOnRateLimit?: string[] | ((data: RateLimitData) => boolean | Promise); } type ClientPresenceStatus = 'online' | 'idle' | 'dnd';