Merge pull request #81 from nathonius/feat/21/rate-limit

Mitigate some rate limit issues

package-lock.json

@@ -28,6 +28,7 @@
 				"obsidian": "latest",
 				"octokit": "^3.1.2",
 				"prettier": "^3.2.5",
+				"queue": "^7.0.0",
 				"tslib": "2.6.2",
 				"typescript": "5.3.3"
 			}
@@ -2480,6 +2481,12 @@
 				"node": ">=6"
 			}
 		},
+		"node_modules/queue": {
+			"version": "7.0.0",
+			"resolved": "https://registry.npmjs.org/queue/-/queue-7.0.0.tgz",
+			"integrity": "sha512-sphwS7HdfQnvrJAXUNAUgpf9H/546IE3p/5Lf2jr71O4udEYlqAhkevykumas2FYuMkX/29JMOgrRdRoYZ/X9w==",
+			"dev": true
+		},
 		"node_modules/queue-microtask": {
 			"version": "1.2.3",
 			"resolved": "https://registry.npmjs.org/queue-microtask/-/queue-microtask-1.2.3.tgz",

package.json

@@ -28,6 +28,7 @@
 		"obsidian": "latest",
 		"octokit": "^3.1.2",
 		"prettier": "^3.2.5",
+		"queue": "^7.0.0",
 		"tslib": "2.6.2",
 		"typescript": "5.3.3"
 	},

src/github/api.ts

@@ -12,10 +12,14 @@ import type {
 } from "./response";
 import type { RequestUrlParam, RequestUrlResponse } from "obsidian";
 
-import { RequestError } from "src/util";
-import { requestUrl } from "obsidian";
+import { RequestError, promiseWithResolvers } from "src/util";
+import { Notice, requestUrl } from "obsidian";
+import { Logger } from "src/plugin";
+import Queue from "queue";
 
 const baseApi = "https://api.github.com";
+let rateLimitReset: Date | null = null;
+const q = new Queue({ autostart: true, concurrency: 1 });
 
 export function addParams(href: string, params: Record<string, unknown>): string {
 	const url = new URL(href);
@@ -25,7 +29,27 @@ export function addParams(href: string, params: Record<string, unknown>): string
 	return url.toString();
 }
 
-export async function githubRequest(config: RequestUrlParam, token?: string): Promise<RequestUrlResponse> {
+export async function queueRequest(config: RequestUrlParam, token?: string): Promise<RequestUrlResponse> {
+	const { resolve, promise } = promiseWithResolvers<RequestUrlResponse>();
+	q.push(() => {
+		return githubRequest(config, token).then((result) => {
+			resolve(result);
+		});
+	});
+	return promise;
+}
+
+async function githubRequest(config: RequestUrlParam, token?: string): Promise<RequestUrlResponse> {
+	if (rateLimitReset !== null && rateLimitReset > new Date()) {
+		Logger.warn(
+			`GitHub rate limit exceeded. No more requests will be made until ${rateLimitReset.toLocaleTimeString()}`,
+		);
+		throw new Error("GitHub rate limit exceeded.");
+	} else if (rateLimitReset !== null) {
+		// Rate limit wait has passed, we can make requests again.
+		rateLimitReset = null;
+	}
+
 	if (!config.headers) {
 		config.headers = {};
 	}
@@ -35,7 +59,31 @@ export async function githubRequest(config: RequestUrlParam, token?: string): Pr
 		config.headers.Authorization = `Bearer ${token}`;
 	}
 	try {
+		Logger.debug(`Request: ${config.url}`);
+		Logger.debug(config);
 		const response = await requestUrl(config);
+		Logger.debug(`Response:`);
+		Logger.debug(response);
+
+		// Handle rate limit
+		const retryAfterSeconds = parseInt(response.headers["retry-after"]);
+		const rateLimitRemaining = parseInt(response.headers["x-ratelimit-remaining"]);
+		const rateLimitResetSeconds = parseInt(response.headers["x-ratelimit-reset"]);
+		if (!isNaN(retryAfterSeconds)) {
+			Logger.warn(`Got retry-after header with value ${retryAfterSeconds}`);
+			await sleep(retryAfterSeconds * 1000);
+			return githubRequest(config, token);
+		} else if (!isNaN(rateLimitRemaining) && rateLimitRemaining === 0 && !isNaN(rateLimitResetSeconds)) {
+			rateLimitReset = new Date(rateLimitResetSeconds * 1000);
+			let message = `GitHub rate limit exceeded. No more requests will be made until after ${rateLimitReset.toLocaleTimeString()}`;
+			if (!token) {
+				message += " Consider adding an authentication token for a significantly higher rate limit.";
+			}
+			new Notice(message);
+		} else if (!isNaN(rateLimitRemaining) && rateLimitRemaining <= 5) {
+			Logger.warn("GitHub rate limit approaching.");
+		}
+
 		return response;
 	} catch (err) {
 		throw new RequestError(err as Error);
@@ -43,14 +91,14 @@ export async function githubRequest(config: RequestUrlParam, token?: string): Pr
 }
 
 async function getIssue(org: string, repo: string, issue: number, token?: string): Promise<IssueResponse> {
-	const result = await githubRequest({ url: `${baseApi}/repos/${org}/${repo}/issues/${issue}` }, token);
+	const result = await queueRequest({ url: `${baseApi}/repos/${org}/${repo}/issues/${issue}` }, token);
 
 	return result.json as IssueResponse;
 }
 
 async function listIssuesForToken(params: IssueListParams = {}, token: string): Promise<IssueListResponse> {
 	const url = addParams(`${baseApi}/issues`, params as Record<string, unknown>);
-	const result = await githubRequest({ url }, token);
+	const result = await queueRequest({ url }, token);
 	return result.json as IssueListResponse;
 }
 
@@ -61,12 +109,12 @@ async function listIssuesForRepo(
 	token?: string,
 ): Promise<IssueListResponse> {
 	const url = addParams(`${baseApi}/repos/${org}/${repo}/issues`, params as Record<string, unknown>);
-	const result = await githubRequest({ url }, token);
+	const result = await queueRequest({ url }, token);
 	return result.json as IssueListResponse;
 }
 
 async function getPullRequest(org: string, repo: string, pr: number, token?: string): Promise<PullResponse> {
-	const result = await githubRequest(
+	const result = await queueRequest(
 		{
 			url: `${baseApi}/repos/${org}/${repo}/pulls/${pr}`,
 		},
@@ -82,12 +130,12 @@ async function listPullRequestsForRepo(
 	token?: string,
 ): Promise<PullListResponse> {
 	const url = addParams(`${baseApi}/repos/${org}/${repo}/pulls`, params as Record<string, unknown>);
-	const result = await githubRequest({ url }, token);
+	const result = await queueRequest({ url }, token);
 	return result.json as PullListResponse;
 }
 
 async function getCode(org: string, repo: string, path: string, branch: string, token?: string): Promise<CodeResponse> {
-	const result = await githubRequest(
+	const result = await queueRequest(
 		{
 			url: `${baseApi}/repos/${org}/${repo}/contents/${path}?ref=${branch}`,
 		},
@@ -97,7 +145,7 @@ async function getCode(org: string, repo: string, path: string, branch: string,
 }
 
 async function searchRepos(query: string, token?: string): Promise<RepoSearchResponse> {
-	const result = await githubRequest({ url: `${baseApi}/search/repositories?q=${encodeURIComponent(query)}` }, token);
+	const result = await queueRequest({ url: `${baseApi}/search/repositories?q=${encodeURIComponent(query)}` }, token);
 	return result.json as RepoSearchResponse;
 }
 

src/github/cache.ts

@@ -18,7 +18,6 @@ class CacheEntry<T> {
 
 	get expired(): boolean {
 		const expiry = this.created.getTime() + this.ttl * 60 * 1000;
-
 		return new Date().getTime() > expiry;
 	}
 }
@@ -60,13 +59,13 @@ export class Cache {
 
 	setIssue(org: string, repo: string, issue: IssueResponse): void {
 		const issueCache = this.getRepoCache(org, repo).issueCache;
-		const existingCache = issueCache[issue.id];
+		const existingCache = issueCache[issue.number];
 		if (existingCache) {
 			const now = new Date();
 			existingCache.created = now;
 			existingCache.value = issue;
 		} else {
-			issueCache[issue.id] = new CacheEntry<IssueResponse>(issue);
+			issueCache[issue.number] = new CacheEntry<IssueResponse>(issue);
 		}
 	}
 
@@ -107,13 +106,13 @@ export class Cache {
 
 	setPullRequest(org: string, repo: string, pullRequest: PullResponse): void {
 		const pullCache = this.getRepoCache(org, repo).pullCache;
-		const existingCache = pullCache[pullRequest.id];
+		const existingCache = pullCache[pullRequest.number];
 		if (existingCache) {
 			const now = new Date();
 			existingCache.created = now;
 			existingCache.value = pullRequest;
 		} else {
-			pullCache[pullRequest.id] = new CacheEntry<PullResponse>(pullRequest);
+			pullCache[pullRequest.number] = new CacheEntry<PullResponse>(pullRequest);
 		}
 	}
 

src/github/github.ts

@@ -13,7 +13,7 @@ import type {
 } from "./response";
 import type { RemoveIndexSignature } from "src/util";
 import { RequestError, sanitizeObject } from "src/util";
-import { api, githubRequest } from "./api";
+import { api, queueRequest } from "./api";
 
 import { Cache } from "./cache";
 import type { GithubAccount } from "src/settings";
@@ -220,7 +220,7 @@ export async function getPRForIssue(timelineUrl: string, org?: string): Promise<
 	let response = cache.getGeneric(timelineUrl) as IssueTimelineResponse | null;
 	if (response === null) {
 		try {
-			response = (await githubRequest({ url: timelineUrl }, getToken(org))).json;
+			response = (await queueRequest({ url: timelineUrl }, getToken(org))).json;
 		} catch (err) {
 			// 404 means there's no timeline for this, we can ignore the error
 			if (err instanceof RequestError && err.status === 404) {

src/inline/inline.ts

@@ -1,12 +1,11 @@
-import { getIssueStatus, getPRStatus } from "src/github/response";
+import { IssueStatus, getIssueStatus, getPRStatus } from "src/github/response";
 import { getIssue, getPullRequest } from "../github/github";
 
 import { parseUrl } from "../github/url-parse";
 import { setIcon } from "obsidian";
 import { setIssueIcon, setPRIcon } from "src/icon";
-import { Logger } from "src/plugin";
 
-export async function createTag(href: string) {
+export function createTag(href: string) {
 	const parsedUrl = parseUrl(href);
 	const container = createEl("a", { cls: "github-link-inline", href });
 
@@ -35,35 +34,38 @@ export async function createTag(href: string) {
 	if (parsedUrl.repo && parsedUrl.org) {
 		// Get issue info
 		if (parsedUrl.issue !== undefined) {
-			const issue = await getIssue(parsedUrl.org, parsedUrl.repo, parsedUrl.issue);
-			Logger.debug("Rendering tag for issue:");
-			Logger.debug(issue);
-			if (issue.title) {
-				const status = getIssueStatus(issue);
-				setIssueIcon(icon, status);
-				createTagSection(container).createSpan({
-					cls: "github-link-inline-issue-title",
-					text: issue.title,
-				});
-			}
+			setIssueIcon(icon, IssueStatus.Open);
+			const issueContainer = createTagSection(container).createSpan({
+				cls: "github-link-inline-issue-title",
+				text: `${parsedUrl.issue}`,
+			});
+			getIssue(parsedUrl.org, parsedUrl.repo, parsedUrl.issue).then((issue) => {
+				if (issue.title) {
+					const status = getIssueStatus(issue);
+					setIssueIcon(icon, status);
+					issueContainer.setText(issue.title);
+				}
+			});
 		}
 
 		// Get PR info
 		if (parsedUrl.pr !== undefined) {
-			const pull = await getPullRequest(parsedUrl.org, parsedUrl.repo, parsedUrl.pr);
-			Logger.debug("Rendering tag for pull request:");
-			Logger.debug(pull);
-			if (pull.title) {
-				const status = getPRStatus(pull);
-				setPRIcon(icon, status);
-				createTagSection(container).createSpan({
-					cls: "github-link-inline-pr-title",
-					text: pull.title,
-				});
-			}
+			setPRIcon(icon, IssueStatus.Open);
+			const prContainer = createTagSection(container).createSpan({
+				cls: "github-link-inline-pr-title",
+				text: `${parsedUrl.pr}`,
+			});
+			getPullRequest(parsedUrl.org, parsedUrl.repo, parsedUrl.pr).then((pr) => {
+				if (pr.title) {
+					const status = getPRStatus(pr);
+					setPRIcon(icon, status);
+					prContainer.setText(pr.title);
+				}
+			});
 		}
 		// TODO: add support for other stuff here
 	}
+
 	return container;
 }
 
@@ -75,7 +77,7 @@ export async function InlineRenderer(el: HTMLElement) {
 	const githubLinks = el.querySelectorAll<HTMLAnchorElement>(`a.external-link[href^="https://github.com"]`);
 	for (const anchor of Array.from(githubLinks)) {
 		if (anchor.href === anchor.innerText) {
-			const container = await createTag(anchor.href);
+			const container = createTag(anchor.href);
 			anchor.replaceWith(container);
 		}
 	}

src/inline/view-plugin.ts

@@ -17,15 +17,8 @@ class InlineTagWidget extends WidgetType {
 		dispatch: () => void,
 	) {
 		super();
-		createTag(href)
-			.then((tag) => {
-				this.container.appendChild(tag);
-			})
-			.catch((err) => {
-				console.error(err);
-				this.error = true;
-				dispatch(); // Force an update of decorations
-			});
+		const tag = createTag(href);
+		this.container.appendChild(tag);
 	}
 
 	eq(widget: WidgetType): boolean {
@@ -83,11 +76,11 @@ export function createInlineViewPlugin(_plugin: GithubLinkPlugin) {
 
 		shouldRender(view: EditorView, decorationFrom: number, decorationTo: number, match: RegExpMatchArray) {
 			// Ignore matches inside a markdown link
-			const input = match.input ?? '';
+			const input = match.input ?? "";
 			const index = match.index ?? 0;
 			const matchValue = match[0];
 			const endIndex = index + matchValue.length;
-			if (input[index - 1] === '(' && matchValue.endsWith(')')) {
+			if (input[index - 1] === "(" && matchValue.endsWith(")")) {
 				return false;
 			}
 

src/plugin.ts

@@ -14,7 +14,6 @@ export class GithubLinkPlugin extends Plugin {
 	async onload() {
 		PluginSettings = Object.assign({}, DEFAULT_SETTINGS, await this.loadData());
 		Logger = verboseFactory(PluginSettings.logLevel);
-		// Logger.debug(getIconIds());
 
 		this.addSettingTab(new GithubLinkPluginSettingsTab(this.app, this));
 		this.registerMarkdownPostProcessor(InlineRenderer);

src/query/column/issue.ts

@@ -28,7 +28,7 @@ export const IssueColumns: ColumnsMap<IssueSearchResponse["items"][number]> = {
 			if (!pullRequestUrl) {
 				return;
 			}
-			const tag = await createTag(pullRequestUrl);
+			const tag = createTag(pullRequestUrl);
 			el.appendChild(tag);
 		},
 	},

src/util.ts

@@ -119,6 +119,27 @@ export const DateFormat = {
 	}),
 };
 
+export function sleep(ms: number): Promise<void> {
+	return new Promise((resolve) => {
+		setTimeout(resolve, ms);
+	});
+}
+
+/**
+ * Implementation of Promise.withResolvers.
+ * See: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/withResolvers
+ */
+export function promiseWithResolvers<T>() {
+	let resolve!: (value: T | PromiseLike<T>) => void;
+	// eslint-disable-next-line @typescript-eslint/no-explicit-any
+	let reject!: (reason?: any) => void;
+	const promise = new Promise<T>((res, rej) => {
+		resolve = res;
+		reject = rej;
+	});
+	return { resolve, reject, promise };
+}
+
 export class RequestError implements Error {
 	name: string;
 	message: string;