Skip to content

Commit

Permalink
Exempt initial requests from ratelimits (#510)
Browse files Browse the repository at this point in the history
* Add support for bypassing ratelimits

* Store pending requests as objects for clarity

* Add types to the rest of these props for fun

* Use bypassRatelimit for initial info requests

* bypassLimit is in the request options, derp
  • Loading branch information
eritbh authored Jul 9, 2021
1 parent 5e66603 commit 692ab4b
Show file tree
Hide file tree
Showing 5 changed files with 130 additions and 36 deletions.
2 changes: 1 addition & 1 deletion .eslintrc.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"parserOptions": {
"ecmaVersion": 2017
"ecmaVersion": 2018
},
"env": {
"browser": true,
Expand Down
49 changes: 36 additions & 13 deletions extension/data/background/handlers/webrequest.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);
Expand All @@ -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
Expand Down
84 changes: 65 additions & 19 deletions extension/data/background/ratelimiter.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,39 +5,87 @@
*/
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<Promise<Response>>}
*/
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
if (this.requestsPending.length <= 0) {
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
Expand All @@ -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
Expand All @@ -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);
}
Expand Down Expand Up @@ -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);
}

/**
Expand Down
19 changes: 17 additions & 2 deletions extension/data/tbapi.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -34,6 +46,7 @@
body,
oauth,
okOnly,
bypassRatelimit,
});

// The reply from that message will always be an object. It can have these keys:
Expand Down Expand Up @@ -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());

/**
Expand Down
12 changes: 11 additions & 1 deletion extension/data/tbcore.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
Expand Down

0 comments on commit 692ab4b

Please sign in to comment.