Promisify internals

package.json

@@ -22,11 +22,10 @@
     "npm": ">=6.10"
   },
   "dependencies": {
+    "@octokit/plugin-throttling": "^3.3.0",
     "@octokit/rest": "^18.0.6",
-    "async": "^3.2.0",
     "commander": "^6.1.0",
     "confirm-simple": "^1.0.3",
-    "queue": "^6.0.1",
     "single-line-log": "^1.1.2"
   },
   "repository": {
@@ -64,10 +63,6 @@
       "no-var": "off",
       "prefer-rest-params": "off",
       "prefer-spread": "off",
-      "promise/always-return": "off",
-      "promise/catch-or-return": "off",
-      "promise/no-callback-in-promise": "off",
-      "promise/prefer-await-to-then": "off",
       "unicorn/explicit-length-check": "off",
       "unicorn/prevent-abbreviations": "off"
     }

src/github-factory.js

@@ -1,117 +1,34 @@
 const {Octokit} = require('@octokit/rest');
-const queue = require('queue');
+const {throttling} = require('@octokit/plugin-throttling');
+const ThrottledOctokit = Octokit.plugin(throttling);
 
-const rawGithubFactory = (token) => {
+module.exports = (token) => {
 	// Allow to inject a GitHub API instead of a token
 	if (token.repos) {
 		return token;
 	}
 
-	return new Octokit({
+	return new ThrottledOctokit({
 		auth: token,
-		version: '3.0.0'
-	});
-};
-
-module.exports = (token) => {
-	const api = rawGithubFactory(token);
-
-	const q = queue({
-		concurrency: 1,
-		timeout: 2000
-	});
-
-	// Transform a github function call into a queued function call
-	// to ensure that only one API call runs at a time
-	// https://developer.github.com/guides/best-practices-for-integrators/#dealing-with-abuse-rate-limits
-	const qd = (call) => {
-		if (typeof call !== 'function') {
-			throw new TypeError(`call should be a function: ${call}`);
-		}
-
-		return (...arguments_) =>
-			new Promise((resolve, reject) => {
-				q.push((callback) => {
-					let argumentsCallback = arguments_.pop();
-					if (typeof argumentsCallback !== 'function') {
-						arguments_.push(argumentsCallback);
-						argumentsCallback = null;
-					}
-
-					call.apply(null, arguments_).then(
-						(result) => {
-							callback();
-							if (argumentsCallback) {
-								argumentsCallback(null, result);
-							}
-
-							resolve(result);
-						},
-						(error) => {
-							callback();
-							if (argumentsCallback) {
-								argumentsCallback(error);
-							}
-
-							reject(error);
-						}
-					);
-				});
-				q.start();
-			});
-	};
-
-	const makeResponseTransformer = (transform) => (call) => {
-		if (typeof call !== 'function') {
-			throw new TypeError(`call should be a function: ${call}`);
-		}
-
-		return (...arguments_) => {
-			let argumentsCallback = arguments_.pop();
-			if (typeof argumentsCallback !== 'function') {
-				arguments_.push(argumentsCallback);
-				argumentsCallback = null;
-			}
-
-			return call.apply(null, arguments_).then(
-				(result) => {
-					result = transform(result);
-					if (argumentsCallback) {
-						argumentsCallback(null, result);
-					}
-
-					return result;
-				},
-				(error) => {
-					if (argumentsCallback) {
-						argumentsCallback(error);
-					}
+		version: '3.0.0',
+		throttle: {
+			onRateLimit: (retryAfter, options, octokit) => {
+				octokit.log.warn(
+					`Request quota exhausted for request ${options.method} ${options.url}`
+				);
+
+				if (options.request.retryCount === 0) {
+					// Only retries once
+					octokit.log.info(`Retrying after ${retryAfter} seconds!`);
+					return true;
 				}
-			);
-		};
-	};
-
-	// Unwrap .data in the response
-	const nd = makeResponseTransformer(({data}) => data);
-
-	// Add a warning about subtly invalid params
-	const hw = (f) => (...arguments_) => {
-		if (arguments_[0] && arguments_[0].url) {
-			throw new Error('Avoid passing url option');
-		}
-
-		return f.apply(null, arguments_);
-	};
-
-	const paginate = (f) => (...arguments_) => api.paginate(f, ...arguments_);
-
-	return {
-		repos: {
-			compareCommits: hw(nd(qd(api.repos.compareCommits))),
-			delete: hw(nd(qd(api.repos.delete))),
-			get: hw(nd(qd(api.repos.get))),
-			list: hw(qd(paginate(api.repos.listForAuthenticatedUser))),
-			listBranches: hw(nd(qd(api.repos.listBranches)))
+			},
+			onAbuseLimit: (retryAfter, options, octokit) => {
+				// Does not retry, only logs a warning
+				octokit.log.warn(
+					`Abuse detected for request ${options.method} ${options.url}`
+				);
+			}
 		}
-	};
+	});
 };

src/index.js

@@ -1,7 +1,5 @@
 'use strict';
 
-const async = require('async');
-
 const githubFactory = require('./github-factory');
 const shouldDeleteFork = require('./should-delete-fork');
 
@@ -20,89 +18,86 @@ const api = (token, options, callback) => {
 	});
 };
 
-api.get = (token, options, getCallback) => {
-	if (!getCallback) {
-		getCallback = options;
-		options = {};
-	}
-
+const _apiGet = async (token, options = {}) => {
 	options.progress = options.progress || (() => {});
 	options.warnings = options.warnings || (() => {});
 
 	const github = githubFactory(token);
 
 	// Get all repositories
-	github.repos
-		.list({type: 'public'})
-		.then((repos) => {
-			// Keep only forks
-			let forks = repos.filter(({fork}) => fork);
-
-			// Keep only forks owned by a specified user
-			if (options.user) {
-				forks = forks.filter(({owner}) => owner.login === options.user);
-			}
+	const repos = await github.paginate(github.repos.listForAuthenticatedUser, {
+		type: 'public'
+	});
+	// Keep only forks
+	let forks = repos.filter(({fork}) => fork);
 
-			let countDoneForks = 0;
-			options.progress({
-				countInspected: 0,
-				totalToInspect: forks.length
-			});
-			const forkDone = (fork) => {
-				countDoneForks += 1;
-				options.progress({
-					countInspected: countDoneForks,
-					lastInspected: fork.name,
-					totalToInspect: forks.length
-				});
-			};
-
-			// Keep only useless forks
-			async.filter(
-				forks,
-				(fork, filterCallback) => {
-					shouldDeleteFork(github, fork, (error, result) => {
-						if (error) {
-							options.warnings(
-								`Failed to inspect ${fork.name}, skipping`,
-								error
-							);
-							result = false;
-						}
-
-						forkDone(fork);
-						filterCallback(null, result);
-					});
-				},
-				(error, forksToDelete) => {
-					if (error) {
-						return getCallback(error);
-					}
-
-					// Map to our simple objects
-					const response = forksToDelete.map((fork) => ({
-						owner: fork.owner.login,
-						repo: fork.name,
-						url: fork.html_url
-					}));
-					getCallback(null, response);
-				}
-			);
-		})
-		.then(null, (error) => {
-			getCallback(error);
+	// Keep only forks owned by a specified user
+	if (options.user) {
+		forks = forks.filter(({owner}) => owner.login === options.user);
+	}
+
+	let countDoneForks = 0;
+	options.progress({
+		countInspected: 0,
+		totalToInspect: forks.length
+	});
+	const forkDone = (fork) => {
+		countDoneForks += 1;
+		options.progress({
+			countInspected: countDoneForks,
+			lastInspected: fork.name,
+			totalToInspect: forks.length
 		});
+	};
+
+	// Keep only useless forks
+	const processedForks = forks.map(async (fork) => {
+		try {
+			if (await shouldDeleteFork(github, fork)) {
+				// Map to our simple objects
+				return {
+					owner: fork.owner.login,
+					repo: fork.name,
+					url: fork.html_url
+				};
+			}
+		} catch (error) {
+			options.warnings(`Failed to inspect ${fork.name}, skipping`, error);
+		} finally {
+			forkDone(fork);
+		}
+	});
+
+	const forksToDelete = await Promise.all(processedForks);
+	return forksToDelete.filter(Boolean);
 };
 
-api.remove = (token, repos, removeCallback) => {
+const _apiRemove = async (token, repos) => {
 	const github = githubFactory(token);
-	async.each(
-		repos,
-		(repo, callback) => {
-			github.repos.delete({owner: repo.owner, repo: repo.repo}, callback);
-		},
-		removeCallback
+	await Promise.all(
+		repos.map((repo) =>
+			github.repos.delete({owner: repo.owner, repo: repo.repo})
+		)
 	);
 };
 
+/* eslint-disable promise/catch-or-return, promise/prefer-await-to-then, promise/always-return, promise/no-callback-in-promise */
+api.remove = (token, repos, callback) => {
+	_apiRemove(token, repos).then((result) => {
+		callback(null, result);
+	}, callback);
+};
+
+api.get = (token, options, callback) => {
+	if (!callback) {
+		callback = options;
+		options = {};
+	}
+
+	_apiGet(token, options).then((result) => {
+		callback(null, result);
+	}, callback);
+};
+/* eslint-enable promise/catch-or-return, promise/prefer-await-to-then, promise/always-return, promise/no-callback-in-promise */
+
 module.exports = api;