Skip to content

Commit f955aaf

Browse files
authored
Replace custom retry() with @octokit/plugin-retry (#947)
1 parent fc1f2ce commit f955aaf

File tree

4 files changed

+35
-70
lines changed

4 files changed

+35
-70
lines changed

server/package-lock.json

Lines changed: 24 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

server/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
"@aws-sdk/client-secrets-manager": "^3.1004.0",
1717
"@hono/node-server": "^1.19.11",
1818
"@octokit/auth-app": "^8.2.0",
19+
"@octokit/plugin-retry": "^8.1.0",
1920
"@octokit/rest": "^22.0.1",
2021
"hono": "^4.12.5",
2122
"jose": "^6.2.1",

server/src/access-token-manager.ts

Lines changed: 10 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import {Octokit as OctokitCore} from '@octokit/core';
22
import {paginateRest} from "@octokit/plugin-paginate-rest";
33
import {restEndpointMethods} from "@octokit/plugin-rest-endpoint-methods";
4+
import {retry as retryPlugin} from '@octokit/plugin-retry';
45
import {z} from 'zod';
56
import type {components} from '@octokit/openapi-types';
67
import {createAppAuth} from '@octokit/auth-app';
@@ -17,7 +18,6 @@ import {
1718
isRecord,
1819
mapObjectEntries,
1920
resultOf,
20-
retry,
2121
unique,
2222
} from './common/common-utils.js';
2323
import {
@@ -39,7 +39,7 @@ import {logger} from './logger.js';
3939
import {RestEndpointMethodTypes} from '@octokit/rest';
4040

4141
const Octokit = OctokitCore
42-
.plugin(restEndpointMethods).plugin(paginateRest);
42+
.plugin(restEndpointMethods, paginateRest, retryPlugin);
4343

4444
const ACCESS_POLICY_MAX_SIZE = 100 * 1024; // 100kb
4545
const GITHUB_API_CONCURRENCY_LIMIT = limit(8);
@@ -604,12 +604,11 @@ async function getAccessPolicy<T extends typeof GitHubAccessPolicySchema>(client
604604
preprocessor: (value: unknown) => unknown,
605605
}): Promise<z.infer<T>> {
606606
const policyValue = await findFirstNotNull(paths, async (path) => {
607-
try {
608-
return await getRepositoryFileContent(client, {owner, repo, path, maxSize: ACCESS_POLICY_MAX_SIZE});
609-
} catch (error) {
610-
logger.error({owner, repo, path, error: String(error)}, 'Failed to get access policy file content');
611-
return null;
612-
}
607+
return getRepositoryFileContent(client, {owner, repo, path, maxSize: ACCESS_POLICY_MAX_SIZE})
608+
.catch((error) => {
609+
logger.error({owner, repo, path, error: String(error)}, 'Failed to get access policy file content');
610+
return null;
611+
});
613612
});
614613
if (!policyValue) {
615614
throw new GithubAccessPolicyError(`Access policy not found`);
@@ -920,17 +919,9 @@ function formatAccessPolicyError(error: GithubAccessPolicyError) {
920919
async function getAppInstallation(client: Octokit, {owner}: {
921920
owner: string
922921
}): Promise<GitHubAppInstallation | null> {
923-
// WORKAROUND: for some reason sometimes the request connection gets closed unexpectedly (line closed),
924-
// therefore, we retry on any error
925-
return retry(
926-
async () => client.rest.apps.getUserInstallation({username: owner})
927-
.then((res) => res.data)
928-
.catch(async (error) => (error.status === Status.NOT_FOUND ? null : _throw(error))),
929-
{
930-
delay: 1000,
931-
retries: 3,
932-
},
933-
);
922+
return client.rest.apps.getUserInstallation({username: owner})
923+
.then((res) => res.data)
924+
.catch(async (error) => (error.status === Status.NOT_FOUND ? null : _throw(error)));
934925
}
935926

936927
/**

server/src/common/common-utils.ts

Lines changed: 0 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -153,57 +153,6 @@ export function filterObjectEntries<V>(
153153
return Object.fromEntries(Object.entries(object).filter(fn));
154154
}
155155

156-
/**
157-
* This function will return a promise that will resolve after the given time
158-
* @param ms - time in milliseconds
159-
* @return promise
160-
*/
161-
export function sleep(ms: number): Promise<void> {
162-
return new Promise((resolve) => setTimeout(resolve, ms));
163-
}
164-
165-
/**
166-
* This function will return a promise that will resolve after the given time
167-
* @param fn - function to retry
168-
* @param options - retry options
169-
* @param options.retries - number of retries
170-
* @param options.delay - delay between retries
171-
* @param options.onRetry - function to call on retry, return false to stop retrying
172-
* @param options.onError - function to call on error, return false to stop retrying
173-
* @return promise
174-
*/
175-
export async function retry<T>(
176-
fn: () => Promise<T>,
177-
options: {
178-
retries: number,
179-
delay: number,
180-
onRetry?: (result: T) => boolean | Promise<boolean>,
181-
onError?: (error: unknown) => boolean | Promise<boolean>,
182-
} = {
183-
retries: 1,
184-
delay: 1000,
185-
},
186-
): Promise<T> {
187-
const {retries, delay} = options;
188-
for (let attempts = 0; attempts < retries; attempts++) {
189-
try {
190-
const result = await fn();
191-
if (!options.onRetry || !options.onRetry(result)) {
192-
return result;
193-
}
194-
} catch (error: unknown) {
195-
if (options.onError && !options.onError(error)) {
196-
throw error;
197-
}
198-
if (attempts >= retries) {
199-
throw error;
200-
}
201-
await sleep(delay);
202-
}
203-
}
204-
throw Error('Illegal state');
205-
}
206-
207156
/**
208157
* Indent string
209158
* @param string - string to indent

0 commit comments

Comments
 (0)