diff --git a/.eslintrc.json b/.eslintrc.json index 001ec42ca..88345cc96 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -1,6 +1,6 @@ { "parserOptions": { - "ecmaVersion": 2017 + "ecmaVersion": 2018 }, "env": { "browser": true, diff --git a/extension/data/background/handlers/webrequest.js b/extension/data/background/handlers/webrequest.js index 3de157d4f..cfe1a5e83 100644 --- a/extension/data/background/handlers/webrequest.js +++ b/extension/data/background/handlers/webrequest.js @@ -137,10 +137,26 @@ function makeFormData (obj) { * @param {boolean?} [options.okOnly] If true, non-2xx responses will result * in an error being rejected. The error will have a `response` property * containing the full `Response` object. + * @param {boolean?} [options.absolute] If true, the request endpoint will be + * treated as a full URL and will not be subjected to any Ratelimiter. + * @param {boolean?} [options.bypassRatelimit] If true, the request will be sent + * immediately, even if the current ratelimit bucket is empty. Use sparingly, + * only for requests which block all of Toolbox, and definitely never for mass + * actions. * @returns {Promise} Resolves to a Response object, or rejects an Error * @todo Ratelimit handling */ -async function makeRequest ({method, endpoint, query, body, oauth, okOnly, absolute}) { +async function makeRequest ({ + method, + endpoint, + query, + body, + oauth, + okOnly, + absolute, + bypassRatelimit, +}) { + // Construct the request URL query = queryString(query); // If we have a query object and additional parameters in the endpoint, we // just stick the object parameters on the end with `&` instead of `?`, and @@ -149,45 +165,50 @@ async function makeRequest ({method, endpoint, query, body, oauth, okOnly, absol query = query.replace('?', '&'); } const url = absolute ? endpoint : `https://${oauth ? 'oauth' : 'old'}.reddit.com${endpoint}${query}`; - const options = { + + // Construct the options object passed to fetch() + const fetchOptions = { credentials: 'include', // required for cookies to be sent redirect: 'error', // prevents strange reddit API shenanigans method, cache: 'no-store', }; - - // Post requests need their body to be in formdata format if (body) { if (typeof body === 'object') { - // If the body is passed as an object, convert it to a FormData object - options.body = makeFormData(body); + // If the body is passed as an object, convert it to FormData (this + // is needed for POST requests) + fetchOptions.body = makeFormData(body); } else { // Otherwise, we assume the body is a string and use it as-is - options.body = body; + fetchOptions.body = body; } } - // If requested, fetch OAuth tokens and add `Authorization` header if (oauth) { try { const tokens = await getOAuthTokens(); - options.headers = {Authorization: `bearer ${tokens.accessToken}`}; + fetchOptions.headers = {Authorization: `bearer ${tokens.accessToken}`}; } catch (error) { console.error('getOAuthTokens: ', error); throw error; } } + // Construct the ratelimiter options object + const ratelimiterOptions = { + bypassLimit: !!bypassRatelimit, + }; + // Perform the request let response; try { if (absolute) { // Absolute URLs may hit non-Reddit domains, don't try to limit them - response = await fetch(url, options); + response = await fetch(url, fetchOptions); } else if (oauth) { - response = await oauthRatelimiter.request(url, options); + response = await oauthRatelimiter.request(ratelimiterOptions, url, fetchOptions); } else { - response = await oldRedditRatelimiter.request(url, options); + response = await oldRedditRatelimiter.request(ratelimiterOptions, url, fetchOptions); } } catch (error) { console.error('Fetch request failed:', error); @@ -208,7 +229,9 @@ async function makeRequest ({method, endpoint, query, body, oauth, okOnly, absol // Makes a request and sends a reply with response and error properties messageHandlers.set('tb-request', requestOptions => makeRequest(requestOptions).then( // For succeeded requests, we send only the raw `response` - async response => ({response: await serializeResponse(response)}), + async response => ({ + response: await serializeResponse(response), + }), // For failed requests, we send: // - `error: true` to indicate the failure // - `message` containing information about the error diff --git a/extension/data/background/ratelimiter.js b/extension/data/background/ratelimiter.js index ebeb748f8..60127e7e1 100644 --- a/extension/data/background/ratelimiter.js +++ b/extension/data/background/ratelimiter.js @@ -5,32 +5,69 @@ */ class Ratelimiter { // eslint-disable-line no-unused-vars constructor () { - /** Array of data for pending requests. */ + /** + * Array of data for pending requests. + * @type {RatelimiterPendingRequest[]} + */ this.requestsPending = []; - /** Set of promises for in-flight requests. */ + /** + * Set of promises for in-flight requests. + * @type {Set>} + */ this.requestsInFlight = new Set(); - /** The number of requests that can be sent before the limit resets */ + /** + * The number of requests that can be sent before the limit resets + * @type {number} + */ this.ratelimitRemaining = 0; - /** When the limit will be reset */ + /** + * When the limit will be reset. + * @type {Date} + */ this.ratelimitResetDate = new Date(); - /** ID of the timer currently waiting for the ratelimit reset, if any */ + /** + * ID of the timer currently waiting for the ratelimit reset, if any. + * @type {number} + */ this.resetTimerID = null; } + /** + * An object containing options that modify how a request is handled. + * @typedef RatelimiterRequestOptions + * @property {boolean} options.bypassLimit If true, this request will be + * sent immediately even if the current ratelimit bucket is empty. Use + * sparingly, only for requests which block all of Toolbox, and definitely + * never for mass actions. + */ + + /** + * An object containing details about a pending request. + * @typedef RatelimiterPendingRequest + * @property {any[]} fetchArgs Arguments to fetch() + * @property {((response: Response) => void)} resolve Function to call if + * the request is completed successfully + * @property {(error: Error) => void} reject Function to call if an error + * occurs while processing the request + * @property {RatelimiterRequestOptions} options Request handling options + */ + /** * Queues a request. + * @param {RatelimiterRequestOptions} options Request handling options * @param {...any} fetchArgs Arguments to fetch() */ - request (...fetchArgs) { + request (options, ...fetchArgs) { return new Promise((resolve, reject) => { - this.requestsPending.push([fetchArgs, resolve, reject]); + this.requestsPending.push({fetchArgs, resolve, reject, options}); this._processQueue(); }); } /** * Recursively sends all queued requests. + * @private */ async _processQueue () { // If there are no queued requests, there's nothing to do @@ -38,6 +75,17 @@ class Ratelimiter { // eslint-disable-line no-unused-vars return; } + // Pull the next queued request + const nextRequest = this.requestsPending.shift(); + + // If this request ignores the limits, send it immediately and move on + if (nextRequest.options.bypassLimit) { + console.debug('Bypassing ratelimit for request:', nextRequest); + this._sendRequest(nextRequest); + this._processQueue(); + return; + } + // If we have as many in-flight requests as we do remaining ratelimit, // we can't do anything right now, but we can try again after the reset // (but if the ratelimit reset date has passed, allow sending serial @@ -63,24 +111,22 @@ class Ratelimiter { // eslint-disable-line no-unused-vars this.resetTimerID = null; } - // Shift the next request off the queue and send it - await this._sendRequest(...this.requestsPending.shift()); + // Send the next request + await this._sendRequest(nextRequest); - // Try another - return this._processQueue(); + // Try to send another + this._processQueue(); } /** * Sends a request, updating ratelimit information from response headers. If * the response is a 429, the request is added to the front of the queue and * retried. - * @param {any[]} fetchArgs Arguments to fetch() - * @param {Function} resolve Function called when the request completes - * @param {Function} reject Function called if the request throws an error + * @param {RatelimiterPendingRequest} request The request to send. */ - async _sendRequest (fetchArgs, resolve, reject) { + async _sendRequest (request) { // Send the request and add it to the set of in-flight requests - const requestPromise = fetch(...fetchArgs); + const requestPromise = fetch(...request.fetchArgs); this.requestsInFlight.add(requestPromise); // Wait for the response and then remove it from the in-flight set @@ -89,7 +135,7 @@ class Ratelimiter { // eslint-disable-line no-unused-vars response = await requestPromise; } catch (error) { // If the request rejects, we don't update the ratelimit - return reject(error); + return request.reject(error); } finally { this.requestsInFlight.delete(requestPromise); } @@ -121,12 +167,12 @@ class Ratelimiter { // eslint-disable-line no-unused-vars // If the response is a 429, add the request back to the front of the // queue and try again, and do not send back the response if (response.status === 429) { - this.requestsPending.unshift([fetchArgs, resolve, reject]); + this.requestsPending.unshift(request); return; } // Return the response we got - resolve(response); + request.resolve(response); } /** diff --git a/extension/data/tbapi.js b/extension/data/tbapi.js index 81c9d177c..34dedfa71 100644 --- a/extension/data/tbapi.js +++ b/extension/data/tbapi.js @@ -22,9 +22,21 @@ * @param {boolean?} [options.okOnly] If true, non-2xx responses will result * in an error being rejected. The error will have a `response` property * containing the full `Response` object. + * @param {boolean?} [options.bypassRatelimit] If true, the request will be + * sent immediately, even if the current ratelimit bucket is empty. Use + * sparingly, only for requests which block all of Toolbox, and definitely + * never for mass actions. * @returns {Promise} Resolves to a `Response` object or rejects an `Error`. */ - TBApi.sendRequest = async ({method, endpoint, query, body, oauth, okOnly}) => { + TBApi.sendRequest = async ({ + method, + endpoint, + query, + body, + oauth, + okOnly, + bypassRatelimit, + }) => { // Make the request const messageReply = await browser.runtime.sendMessage({ action: 'tb-request', @@ -34,6 +46,7 @@ body, oauth, okOnly, + bypassRatelimit, }); // The reply from that message will always be an object. It can have these keys: @@ -61,12 +74,14 @@ * @function * @param {string} endpoint The endpoint to request * @param {object} data Query parameters as an object + * @param {object} options Additional options passed to sendRequest() */ - TBApi.getJSON = (endpoint, query) => TBApi.sendRequest({ + TBApi.getJSON = (endpoint, query, options) => TBApi.sendRequest({ okOnly: true, method: 'GET', endpoint, query, + ...options, }).then(response => response.json()); /** diff --git a/extension/data/tbcore.js b/extension/data/tbcore.js index 8312a7aeb..3906d7173 100644 --- a/extension/data/tbcore.js +++ b/extension/data/tbcore.js @@ -1762,6 +1762,11 @@ function initwrapper ({userDetails, newModSubs, cacheDetails}) { const json = await TBApi.getJSON('/subreddits/mine/moderator.json', { after, limit: 100, + }, { + // This function has the potential to hang Toolbox's entire load + // sequence, so we ignore ratelimits intentionally and rely on + // Reddit handling the request gracefully anyway. + bypassRatelimit: true, }); TBStorage.purifyObject(json); modSubs = modSubs.concat(json.data.children); @@ -1786,7 +1791,12 @@ function initwrapper ({userDetails, newModSubs, cacheDetails}) { } function getUserDetails (tries = 0) { - return TBApi.getJSON('/api/me.json').then(data => { + return TBApi.getJSON('/api/me.json', {}, { + // This function has the potential to hang Toolbox's entire load + // sequence, so we ignore ratelimits intentionally and rely on + // Reddit handling the request gracefully anyway. + bypassRatelimit: true, + }).then(data => { TBStorage.purifyObject(data); logger.log(data); return data;