From 6efc911b881490dc1fc812783cd059df46316830 Mon Sep 17 00:00:00 2001 From: Peter Bengtsson Date: Mon, 14 Aug 2023 13:59:42 -0400 Subject: [PATCH] Better and simpler handling of operational errors (#40730) --- .../purge-old-workflow-runs.js | 34 ++++++++++++++----- package-lock.json | 28 +-------------- package.json | 1 - 3 files changed, 27 insertions(+), 36 deletions(-) diff --git a/.github/actions-scripts/purge-old-workflow-runs.js b/.github/actions-scripts/purge-old-workflow-runs.js index 0867241f60..ec9f663d2b 100755 --- a/.github/actions-scripts/purge-old-workflow-runs.js +++ b/.github/actions-scripts/purge-old-workflow-runs.js @@ -25,7 +25,6 @@ import fs from 'fs' import assert from 'node:assert/strict' import { getOctokit } from '@actions/github' -import { RequestError } from '@octokit/request-error' main() async function main() { @@ -33,7 +32,7 @@ async function main() { const MAX_DELETIONS = parseInt(JSON.parse(process.env.MAX_DELETIONS || '100')) const MIN_AGE_DAYS = parseInt(process.env.MIN_AGE_DAYS || '90', 10) - const [owner, repo] = process.env.GITHUB_REPOSITORY.split('/') + const [owner, repo] = (process.env.GITHUB_REPOSITORY || 'github/docs-internal').split('/') if (!owner || !repo) { throw new Error('GITHUB_REPOSITORY environment variable not set') } @@ -55,13 +54,16 @@ async function main() { repo, }) } catch (error) { + console.log('Error happened when getting workflows') + console.warn('Status: %O', error.status) + console.warn('Message: %O', error.message) + // Generally, if it fails, it's because of a network error or // because busy servers. It's not our fault, but considering that // this script is supposed to run on frequent schedule, we don't // need to fret. We'll just try again next time. - if (error instanceof RequestError && error.status >= 500) { - console.log(`RequestError: ${error.message}`) - console.log(` status: ${error.status}`) + if (isOperationalError(error.status, error.message)) { + return } else { throw error } @@ -90,13 +92,15 @@ async function main() { maxDeletions: MAX_DELETIONS - deletions, }) } catch (error) { + console.log("Error happened when calling 'deleteWorkflowRuns'") + console.warn('Status: %O', error.status) + console.warn('Message: %O', error.message) + // Generally, if it fails, it's because of a network error or // because busy servers. It's not our fault, but considering that // this script is supposed to run on frequent schedule, we don't // need to fret. We'll just try again next time. - if (error instanceof RequestError && error.status >= 500) { - console.log(`RequestError: ${error.message}`) - console.log(` status: ${error.status}`) + if (isOperationalError(error.status, error.message)) { break } else { throw error @@ -111,6 +115,20 @@ async function main() { console.log(`Deleted ${deletions} runs in total`) } +function isOperationalError(status, message) { + if (status && status >= 500) { + return true + } + if (/Unable to delete logs while the workflow is running/.test(message)) { + return true + } + if (status === 403 && /API rate limit exceeded/.test(message)) { + return true + } + + return false +} + async function deleteWorkflowRuns( github, owner, diff --git a/package-lock.json b/package-lock.json index 7c61f84b90..3809dec472 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9,7 +9,6 @@ "dependencies": { "@elastic/elasticsearch": "7.11.0", "@github/failbot": "0.8.3", - "@octokit/request-error": "5.0.0", "@primer/behaviors": "^1.3.3", "@primer/css": "^21.0.1", "@primer/octicons": "^19.1.0", @@ -2092,32 +2091,6 @@ "universal-user-agent": "^6.0.0" } }, - "node_modules/@octokit/request-error": { - "version": "5.0.0", - "resolved": "https://registry.npmjs.org/@octokit/request-error/-/request-error-5.0.0.tgz", - "integrity": "sha512-1ue0DH0Lif5iEqT52+Rf/hf0RmGO9NWFjrzmrkArpG9trFfDM/efx00BJHdLGuro4BR/gECxCU2Twf5OKrRFsQ==", - "dependencies": { - "@octokit/types": "^11.0.0", - "deprecation": "^2.0.0", - "once": "^1.4.0" - }, - "engines": { - "node": ">= 18" - } - }, - "node_modules/@octokit/request-error/node_modules/@octokit/openapi-types": { - "version": "18.0.0", - "resolved": "https://registry.npmjs.org/@octokit/openapi-types/-/openapi-types-18.0.0.tgz", - "integrity": "sha512-V8GImKs3TeQRxRtXFpG2wl19V7444NIOTDF24AWuIbmNaNYOQMWRbjcGDXV5B+0n887fgDcuMNOmlul+k+oJtw==" - }, - "node_modules/@octokit/request-error/node_modules/@octokit/types": { - "version": "11.1.0", - "resolved": "https://registry.npmjs.org/@octokit/types/-/types-11.1.0.tgz", - "integrity": "sha512-Fz0+7GyLm/bHt8fwEqgvRBWwIV1S6wRRyq+V6exRKLVWaKGsuy6H9QFYeBVDV7rK6fO3XwHgQOPxv+cLj2zpXQ==", - "dependencies": { - "@octokit/openapi-types": "^18.0.0" - } - }, "node_modules/@octokit/request/node_modules/@octokit/request-error": { "version": "2.1.0", "resolved": "https://registry.npmjs.org/@octokit/request-error/-/request-error-2.1.0.tgz", @@ -4917,6 +4890,7 @@ }, "node_modules/deprecation": { "version": "2.3.1", + "dev": true, "license": "ISC" }, "node_modules/dequal": { diff --git a/package.json b/package.json index 51343774cd..fd50fbe6fa 100644 --- a/package.json +++ b/package.json @@ -58,7 +58,6 @@ "dependencies": { "@elastic/elasticsearch": "7.11.0", "@github/failbot": "0.8.3", - "@octokit/request-error": "5.0.0", "@primer/behaviors": "^1.3.3", "@primer/css": "^21.0.1", "@primer/octicons": "^19.1.0",