From 0f3b4d7df99b1af7cb8596ba4f855d6de4155aa5 Mon Sep 17 00:00:00 2001 From: Stephane Moser Date: Sun, 12 Nov 2023 16:00:38 +0000 Subject: [PATCH] feat: add retry (#79) resolves #71 - Add p-retry library - Extract logic to new functions to improve the usage of retry logic --- lib/main.js | 96 +++++++++++------- package-lock.json | 43 +++++++- package.json | 3 +- ...n-get-owner-set-repo-fail-response.test.js | 39 +++++++ ...et-owner-set-to-user-fail-response.test.js | 36 +++++++ tests/snapshots/index.js.md | 30 ++++++ tests/snapshots/index.js.snap | Bin 857 -> 955 bytes 7 files changed, 211 insertions(+), 36 deletions(-) create mode 100644 tests/main-token-get-owner-set-repo-fail-response.test.js create mode 100644 tests/main-token-get-owner-set-to-user-fail-response.test.js diff --git a/lib/main.js b/lib/main.js index 7b32a09..9dfe730 100644 --- a/lib/main.js +++ b/lib/main.js @@ -1,3 +1,4 @@ +import pRetry from "p-retry"; // @ts-check /** @@ -75,47 +76,26 @@ export async function main( let authentication; // If at least one repository is set, get installation ID from that repository - // https://docs.github.com/rest/apps/apps?apiVersion=2022-11-28#get-a-repository-installation-for-the-authenticated-app + if (parsedRepositoryNames) { - const response = await request("GET /repos/{owner}/{repo}/installation", { - owner: parsedOwner, - repo: parsedRepositoryNames.split(",")[0], - headers: { - authorization: `bearer ${appAuthentication.token}`, + authentication = await pRetry(() => getTokenFromRepository(request, auth, parsedOwner,appAuthentication, parsedRepositoryNames), { + onFailedAttempt: (error) => { + core.info( + `Failed to create token for "${parsedRepositoryNames}" (attempt ${error.attemptNumber}): ${error.message}` + ); }, + retries: 3, }); - // Get token for given repositories - authentication = await auth({ - type: "installation", - installationId: response.data.id, - repositoryNames: parsedRepositoryNames.split(","), - }); } else { // Otherwise get the installation for the owner, which can either be an organization or a user account - // https://docs.github.com/rest/apps/apps?apiVersion=2022-11-28#get-a-repository-installation-for-the-authenticated-app - const response = await request("GET /orgs/{org}/installation", { - org: parsedOwner, - headers: { - authorization: `bearer ${appAuthentication.token}`, + authentication = await pRetry(() => getTokenFromOwner(request, auth, appAuthentication, parsedOwner), { + onFailedAttempt: (error) => { + core.info( + `Failed to create token for "${parsedOwner}" (attempt ${error.attemptNumber}): ${error.message}` + ); }, - }).catch((error) => { - /* c8 ignore next */ - if (error.status !== 404) throw error; - - // https://docs.github.com/rest/apps/apps?apiVersion=2022-11-28#get-a-user-installation-for-the-authenticated-app - return request("GET /users/{username}/installation", { - username: parsedOwner, - headers: { - authorization: `bearer ${appAuthentication.token}`, - }, - }); - }); - - // Get token for for all repositories of the given installation - authentication = await auth({ - type: "installation", - installationId: response.data.id, + retries: 3, }); } @@ -129,3 +109,51 @@ export async function main( core.saveState("token", authentication.token); } } + +async function getTokenFromOwner(request, auth, appAuthentication, parsedOwner) { + // https://docs.github.com/en/rest/apps/apps?apiVersion=2022-11-28#get-an-organization-installation-for-the-authenticated-app + const response = await request("GET /orgs/{org}/installation", { + org: parsedOwner, + headers: { + authorization: `bearer ${appAuthentication.token}`, + }, + }).catch((error) => { + /* c8 ignore next */ + if (error.status !== 404) throw error; + + // https://docs.github.com/rest/apps/apps?apiVersion=2022-11-28#get-a-user-installation-for-the-authenticated-app + return request("GET /users/{username}/installation", { + username: parsedOwner, + headers: { + authorization: `bearer ${appAuthentication.token}`, + }, + }); + }); + + // Get token for for all repositories of the given installation + const authentication = await auth({ + type: "installation", + installationId: response.data.id, + }); + return authentication; +} + +async function getTokenFromRepository(request, auth, parsedOwner,appAuthentication, parsedRepositoryNames) { + // https://docs.github.com/rest/apps/apps?apiVersion=2022-11-28#get-a-repository-installation-for-the-authenticated-app + const response = await request("GET /repos/{owner}/{repo}/installation", { + owner: parsedOwner, + repo: parsedRepositoryNames.split(",")[0], + headers: { + authorization: `bearer ${appAuthentication.token}`, + }, + }); + + // Get token for given repositories + const authentication = await auth({ + type: "installation", + installationId: response.data.id, + repositoryNames: parsedRepositoryNames.split(","), + }); + + return authentication; +} \ No newline at end of file diff --git a/package-lock.json b/package-lock.json index 5bf8b49..3ce99ec 100644 --- a/package-lock.json +++ b/package-lock.json @@ -11,7 +11,8 @@ "dependencies": { "@actions/core": "^1.10.1", "@octokit/auth-app": "^6.0.1", - "@octokit/request": "^8.1.4" + "@octokit/request": "^8.1.4", + "p-retry": "^6.1.0" }, "devDependencies": { "ava": "^5.3.1", @@ -853,6 +854,11 @@ "integrity": "sha512-lqa4UEhhv/2sjjIQgjX8B+RBjj47eo0mzGasklVJ78UKGQY1r0VpB9XHDaZZO9qzEFDdy4MrXLuEaSmPrPSe/A==", "dev": true }, + "node_modules/@types/retry": { + "version": "0.12.2", + "resolved": "https://registry.npmjs.org/@types/retry/-/retry-0.12.2.tgz", + "integrity": "sha512-XISRgDJ2Tc5q4TRqvgJtzsRkFYNJzZrhTdtMoGVBttwzzQJkPnS3WWTFc7kuDRoPtPakl+T+OfdEUjYJj7Jbow==" + }, "node_modules/acorn": { "version": "8.10.0", "resolved": "https://registry.npmjs.org/acorn/-/acorn-8.10.0.tgz", @@ -2407,6 +2413,17 @@ "url": "https://github.com/sponsors/sindresorhus" } }, + "node_modules/is-network-error": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/is-network-error/-/is-network-error-1.0.0.tgz", + "integrity": "sha512-P3fxi10Aji2FZmHTrMPSNFbNC6nnp4U5juPAIjXPHkUNubi4+qK7vvdsaNpAUwXslhYm9oyjEYTxs1xd/+Ph0w==", + "engines": { + "node": ">=16" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, "node_modules/is-number": { "version": "7.0.0", "resolved": "https://registry.npmjs.org/is-number/-/is-number-7.0.0.tgz", @@ -3098,6 +3115,22 @@ "url": "https://github.com/sponsors/sindresorhus" } }, + "node_modules/p-retry": { + "version": "6.1.0", + "resolved": "https://registry.npmjs.org/p-retry/-/p-retry-6.1.0.tgz", + "integrity": "sha512-fJLEQ2KqYBJRuaA/8cKMnqhulqNM+bpcjYtXNex2t3mOXKRYPitAJt9NacSf8XAFzcYahSAbKpobiWDSqHSh2g==", + "dependencies": { + "@types/retry": "0.12.2", + "is-network-error": "^1.0.0", + "retry": "^0.13.1" + }, + "engines": { + "node": ">=16.17" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, "node_modules/p-timeout": { "version": "5.1.0", "resolved": "https://registry.npmjs.org/p-timeout/-/p-timeout-5.1.0.tgz", @@ -3465,6 +3498,14 @@ "node": ">=8" } }, + "node_modules/retry": { + "version": "0.13.1", + "resolved": "https://registry.npmjs.org/retry/-/retry-0.13.1.tgz", + "integrity": "sha512-XQBQ3I8W1Cge0Seh+6gjj03LbmRFWuoszgK9ooCpwYIrhhoO80pfq4cUkU5DkknwfOfFteRwlZ56PYOGYyFWdg==", + "engines": { + "node": ">= 4" + } + }, "node_modules/reusify": { "version": "1.0.4", "resolved": "https://registry.npmjs.org/reusify/-/reusify-1.0.4.tgz", diff --git a/package.json b/package.json index a9f1578..23c723e 100644 --- a/package.json +++ b/package.json @@ -14,7 +14,8 @@ "dependencies": { "@actions/core": "^1.10.1", "@octokit/auth-app": "^6.0.1", - "@octokit/request": "^8.1.4" + "@octokit/request": "^8.1.4", + "p-retry": "^6.1.0" }, "devDependencies": { "ava": "^5.3.1", diff --git a/tests/main-token-get-owner-set-repo-fail-response.test.js b/tests/main-token-get-owner-set-repo-fail-response.test.js new file mode 100644 index 0000000..9729376 --- /dev/null +++ b/tests/main-token-get-owner-set-repo-fail-response.test.js @@ -0,0 +1,39 @@ +import { test } from "./main.js"; + +// Verify `main` retry when the GitHub API returns a 500 error. +await test((mockPool) => { + process.env.INPUT_OWNER = 'actions' + process.env.INPUT_REPOSITORIES = 'failed-repo'; + const owner = process.env.INPUT_OWNER + const repo = process.env.INPUT_REPOSITORIES + const mockInstallationId = "123456"; + + mockPool + .intercept({ + path: `/repos/${owner}/${repo}/installation`, + method: "GET", + headers: { + accept: "application/vnd.github.v3+json", + "user-agent": "actions/create-github-app-token", + // Intentionally omitting the `authorization` header, since JWT creation is not idempotent. + }, + }) + .reply(500, 'GitHub API not available') + + mockPool + .intercept({ + path: `/repos/${owner}/${repo}/installation`, + method: "GET", + headers: { + accept: "application/vnd.github.v3+json", + "user-agent": "actions/create-github-app-token", + // Intentionally omitting the `authorization` header, since JWT creation is not idempotent. + }, + }) + .reply( + 200, + { id: mockInstallationId }, + { headers: { "content-type": "application/json" } } + ); + +}); diff --git a/tests/main-token-get-owner-set-to-user-fail-response.test.js b/tests/main-token-get-owner-set-to-user-fail-response.test.js new file mode 100644 index 0000000..d1edf81 --- /dev/null +++ b/tests/main-token-get-owner-set-to-user-fail-response.test.js @@ -0,0 +1,36 @@ +import { test } from "./main.js"; + +// Verify `main` successfully obtains a token when the `owner` input is set (to a user), but the `repositories` input isn’t set. +await test((mockPool) => { + process.env.INPUT_OWNER = "smockle"; + delete process.env.INPUT_REPOSITORIES; + + // Mock installation id request + const mockInstallationId = "123456"; + mockPool + .intercept({ + path: `/orgs/${process.env.INPUT_OWNER}/installation`, + method: "GET", + headers: { + accept: "application/vnd.github.v3+json", + "user-agent": "actions/create-github-app-token", + // Intentionally omitting the `authorization` header, since JWT creation is not idempotent. + }, + }) + .reply(500, 'GitHub API not available') + mockPool + .intercept({ + path: `/orgs/${process.env.INPUT_OWNER}/installation`, + method: "GET", + headers: { + accept: "application/vnd.github.v3+json", + "user-agent": "actions/create-github-app-token", + // Intentionally omitting the `authorization` header, since JWT creation is not idempotent. + }, + }) + .reply( + 200, + { id: mockInstallationId }, + { headers: { "content-type": "application/json" } } + ); +}); diff --git a/tests/snapshots/index.js.md b/tests/snapshots/index.js.md index 4d9a2ec..e7c6e86 100644 --- a/tests/snapshots/index.js.md +++ b/tests/snapshots/index.js.md @@ -56,6 +56,21 @@ Generated by [AVA](https://avajs.dev). '' +## main-token-get-owner-set-repo-fail-response.test.js + +> stderr + + '' + +> stdout + + `owner and repositories set, creating token for repositories "failed-repo" owned by "actions"␊ + Failed to create token for "failed-repo" (attempt 1): GitHub API not available␊ + ::add-mask::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊ + ␊ + ::set-output name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊ + ::save-state name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a` + ## main-token-get-owner-set-repo-set-to-many.test.js > stderr @@ -98,6 +113,21 @@ Generated by [AVA](https://avajs.dev). ::set-output name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊ ::save-state name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a` +## main-token-get-owner-set-to-user-fail-response.test.js + +> stderr + + '' + +> stdout + + `repositories not set, creating token for all repositories for given owner "smockle"␊ + Failed to create token for "smockle" (attempt 1): GitHub API not available␊ + ::add-mask::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊ + ␊ + ::set-output name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊ + ::save-state name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a` + ## main-token-get-owner-set-to-user-repo-unset.test.js > stderr diff --git a/tests/snapshots/index.js.snap b/tests/snapshots/index.js.snap index 902533097181cd76e356d0ea1eb75af80c2738c1..f7989c60e0237f307e83e20af6fca8e5ef7c89d1 100644 GIT binary patch literal 955 zcmV;s14R5mRzVoKTjEBE5QwO+dqKWh^uDP#S^}T)N zwuY!yN*{{|00000000B!nB7hjK@`WS#+ZK!<0LiL3_D>R5FJ1njv??6-+&{O|5 z>bd58v9+9M! zkx3kf3<956y)nGA3L@p~uH%fKMj*0{?Ks9Zc4`N^hcD{;+i&;Y)eiRdkL!Ey-)!%b zzS8SBq7hjgU-SO1rx^*7$%8<)MHyB zO#u|ukcqKHT3ooD7PKq19O}jeqLl*DI3o*o!ITTLugra&BqIeWA@|uuiU{2pN%^5q zb{Os^4YKvNW~zerR8QInVb=F)5O`fGJHFpOk@fQ0%POo^UR5?K&9#kkWxHA}FE`g$ z)>l@mTTrg9zgVTS>M5N?m15xW%wU3 zt7J0MsD#}vyhJUun1|YZhuU1h!C^7ew@y_vCVqwwk1EUVAdvqNu;{IvTd;K zz&W}~II^o=A3~VC`wcq%mpfnfS!clcpPzw4$kRpU@mMkj#Bf8)LuAt-;`Mj3^C)cl zDC>W?g{c?DW+DVc*jHK{E}0LfjF@*3vD~w$ilu_!b zA!8p>E*eMooVV1u1CwI==_45bT_GL1!{QjSKX_am{*e>@{wREt>gh#JsXB(*C}Z8+ dNx8{cm`4vBFSo20HY`LZ{x4O4(}UF$005uT#yS81 literal 857 zcmV-f1E%~zRzVV7OoOn1FO%@9Igy?TSYp?Sm~#)B@T~ zVnj>ZG9QZw00000000B!m(gz$F&xK1V@#GT(Kq83*X#wI92*SBxJ(mg=Ej50;1;(h zQn(*bIoh-BG52Qt1AOuyaDSSAiM^J=HRHy{#l#>lTuSfz{r2~1OZ%(c^jWJXeq4Yg z(uc1Iw)neAVJ8&XF&wTf#@%>7LC$==I6%z%%Z z;J1bJfOFH~G>hY1?$on52pYsgzrX!NIrE%DM4+)J!qh`&#P?AX5Qjb+s5U|oM(t3B z9MAx`P%#VWSb)}Wv<4Ltuvb_JI2kCNMi0)7OROQC)lkSWLYID`M}mfAJ)jK^1NCiO zuQaFgj^rFI=Zxfr3C3k-p}b_d)W?K6eIf**9mf6dt9}Yj(jt=Xmjl)mx_v&b< zmPh?i$f%$KB$5o}cC`Mh+-y;i6*Y!GIk0O5&QGmmOmw!4D^^Ah0TVcguJ|y%p6%RWF)G6!+n! zcD#jzQosXngjC6AQkV#F<3FH=GJ)1LP&NO1FS5d)YwvzO(blrNAT^IowpV$4^t<{|>faGh@bo({SetB%Ku6SyBp);(G jC&p!Bnn$rTDJSO16D!LDlZA{XqUHYw4)n3>`wsvBe7vCy