diff --git a/dist/check-group/core/config_getter.js b/dist/check-group/core/config_getter.js index ea576fb2..48dea75e 100644 --- a/dist/check-group/core/config_getter.js +++ b/dist/check-group/core/config_getter.js @@ -73,6 +73,7 @@ Object.defineProperty(exports, "__esModule", { value: true }); exports.fetchConfig = void 0; var user_config_parser_1 = require("./user_config_parser"); var core = __importStar(require("@actions/core")); +var transient_error_1 = require("./transient_error"); /** * Fetches the app configuration from the user's repository. * @@ -116,7 +117,7 @@ var readConfig = function (context, branch) { return __awaiter(void 0, void 0, v switch (_a.label) { case 0: params = context.repo({ path: '.github/checkgroup.yml' }); - return [4 /*yield*/, context.octokit.config.get(__assign(__assign({}, params), { branch: branch }))]; + return [4 /*yield*/, (0, transient_error_1.withTransientRetry)("Reading '.github/checkgroup.yml' from '".concat(branch, "'"), function () { return context.octokit.config.get(__assign(__assign({}, params), { branch: branch })); })]; case 1: config = (_a.sent()).config; return [2 /*return*/, config]; diff --git a/dist/check-group/core/index.js b/dist/check-group/core/index.js index 92f9ed0e..35d67654 100644 --- a/dist/check-group/core/index.js +++ b/dist/check-group/core/index.js @@ -69,7 +69,7 @@ var subproj_matching_1 = require("./subproj_matching"); var satisfy_expected_checks_1 = require("./satisfy_expected_checks"); var config_getter_1 = require("./config_getter"); Object.defineProperty(exports, "fetchConfig", { enumerable: true, get: function () { return config_getter_1.fetchConfig; } }); -var request_error_1 = require("@octokit/request-error"); +var transient_error_1 = require("./transient_error"); /** * The orchestration class. */ @@ -79,6 +79,7 @@ var CheckGroup = /** @class */ (function () { this.timeoutTimer = setTimeout(function () { return ''; }, 0); this.inputs = {}; this.canComment = true; + this.lastTransientError = ""; this.pullRequestNumber = pullRequestNumber; this.config = config; this.context = context; @@ -115,6 +116,9 @@ var CheckGroup = /** @class */ (function () { this.timeoutTimer = setTimeout(function () { clearTimeout(_this.intervalTimer); core.setFailed("The timeout of ".concat(timeout, " minutes has triggered but not all required jobs were passing.") + + (_this.lastTransientError + ? " The GitHub API was also returning errors, the last being: ".concat(_this.lastTransientError) + : "") + " This job will need to be re-run to merge your PR." + " If you do not have write access to the repository you can ask ".concat(maintainers, " to re-run it for you.") + " If you have any other questions, you can reach out to ".concat(owner, " for help.")); @@ -131,7 +135,7 @@ var CheckGroup = /** @class */ (function () { return __generator(this, function (_a) { switch (_a.label) { case 0: - _a.trys.push([0, 2, , 3]); + _a.trys.push([0, 3, , 4]); // print in a group to reduce verbosity core.startGroup("Check ".concat(tries)); return [4 /*yield*/, getPostedChecks(this.context, this.sha)]; @@ -139,7 +143,9 @@ var CheckGroup = /** @class */ (function () { postedChecks = _a.sent(); core.debug("postedChecks: ".concat(JSON.stringify(postedChecks))); result = (0, satisfy_expected_checks_1.getSubProjResult)(subprojs, postedChecks); - this.notifyProgress(subprojs, postedChecks, result); + return [4 /*yield*/, this.notifyProgress(subprojs, postedChecks, result)]; + case 2: + _a.sent(); core.endGroup(); if (result === "all_passing") { core.info("All required checks were successful!"); @@ -149,15 +155,25 @@ var CheckGroup = /** @class */ (function () { else { this.intervalTimer = setTimeout(function () { return _this.runCheck(subprojs, tries + 1, interval); }, interval); } - return [3 /*break*/, 3]; - case 2: + return [3 /*break*/, 4]; + case 3: error_1 = _a.sent(); + core.endGroup(); + if ((0, transient_error_1.isTransientError)(error_1)) { + // A GitHub incident says nothing about the PR, so keep polling until the + // timeout timer fires rather than failing a PR whose checks are green. + this.lastTransientError = (0, transient_error_1.describeError)(error_1); + core.warning("Check ".concat(tries, " hit a transient GitHub API error, retrying in ").concat(interval / 1000, "s:") + + " ".concat(this.lastTransientError)); + this.intervalTimer = setTimeout(function () { return _this.runCheck(subprojs, tries + 1, interval); }, interval); + return [2 /*return*/]; + } // bubble up the error to the job core.setFailed(error_1); clearTimeout(this.intervalTimer); clearTimeout(this.timeoutTimer); - return [3 /*break*/, 3]; - case 3: return [2 /*return*/]; + return [3 /*break*/, 4]; + case 4: return [2 /*return*/]; } }); }); @@ -179,16 +195,18 @@ var CheckGroup = /** @class */ (function () { return [3 /*break*/, 4]; case 3: e_1 = _a.sent(); - if (e_1 instanceof request_error_1.RequestError && e_1.status === 403) { + // The comment is informational, so a failure to write it must never fail + // the job — the check statuses it summarises are unaffected. + if ((0, transient_error_1.httpStatus)(e_1) === 403) { // Forbidden: Resource not accessible by integration if (this.canComment) { - core.info("Failed to comment on the PR: ".concat(JSON.stringify(e_1))); + core.info("Failed to comment on the PR: ".concat((0, transient_error_1.describeError)(e_1))); } // Use this boolean to only print the info message once this.canComment = false; } else { - throw e_1; + core.warning("Failed to update the PR comment: ".concat((0, transient_error_1.describeError)(e_1))); } return [3 /*break*/, 4]; case 4: return [2 /*return*/]; @@ -203,9 +221,10 @@ var CheckGroup = /** @class */ (function () { CheckGroup.prototype.files = function () { return __awaiter(this, void 0, void 0, function () { var pullRequestFiles, filenames; + var _this = this; return __generator(this, function (_a) { switch (_a.label) { - case 0: return [4 /*yield*/, this.context.octokit.paginate(this.context.octokit.pulls.listFiles, this.context.repo({ "pull_number": this.pullRequestNumber }), function (response) { return response.data; })]; + case 0: return [4 /*yield*/, (0, transient_error_1.withTransientRetry)("Listing the files changed in the PR", function () { return _this.context.octokit.paginate(_this.context.octokit.pulls.listFiles, _this.context.repo({ "pull_number": _this.pullRequestNumber }), function (response) { return response.data; }); })]; case 1: pullRequestFiles = _a.sent(); filenames = []; diff --git a/dist/check-group/core/transient_error.js b/dist/check-group/core/transient_error.js new file mode 100644 index 00000000..6af60ceb --- /dev/null +++ b/dist/check-group/core/transient_error.js @@ -0,0 +1,167 @@ +"use strict"; +var __createBinding = (this && this.__createBinding) || (Object.create ? (function(o, m, k, k2) { + if (k2 === undefined) k2 = k; + var desc = Object.getOwnPropertyDescriptor(m, k); + if (!desc || ("get" in desc ? !m.__esModule : desc.writable || desc.configurable)) { + desc = { enumerable: true, get: function() { return m[k]; } }; + } + Object.defineProperty(o, k2, desc); +}) : (function(o, m, k, k2) { + if (k2 === undefined) k2 = k; + o[k2] = m[k]; +})); +var __setModuleDefault = (this && this.__setModuleDefault) || (Object.create ? (function(o, v) { + Object.defineProperty(o, "default", { enumerable: true, value: v }); +}) : function(o, v) { + o["default"] = v; +}); +var __importStar = (this && this.__importStar) || function (mod) { + if (mod && mod.__esModule) return mod; + var result = {}; + if (mod != null) for (var k in mod) if (k !== "default" && Object.prototype.hasOwnProperty.call(mod, k)) __createBinding(result, mod, k); + __setModuleDefault(result, mod); + return result; +}; +var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, generator) { + function adopt(value) { return value instanceof P ? value : new P(function (resolve) { resolve(value); }); } + return new (P || (P = Promise))(function (resolve, reject) { + function fulfilled(value) { try { step(generator.next(value)); } catch (e) { reject(e); } } + function rejected(value) { try { step(generator["throw"](value)); } catch (e) { reject(e); } } + function step(result) { result.done ? resolve(result.value) : adopt(result.value).then(fulfilled, rejected); } + step((generator = generator.apply(thisArg, _arguments || [])).next()); + }); +}; +var __generator = (this && this.__generator) || function (thisArg, body) { + var _ = { label: 0, sent: function() { if (t[0] & 1) throw t[1]; return t[1]; }, trys: [], ops: [] }, f, y, t, g; + return g = { next: verb(0), "throw": verb(1), "return": verb(2) }, typeof Symbol === "function" && (g[Symbol.iterator] = function() { return this; }), g; + function verb(n) { return function (v) { return step([n, v]); }; } + function step(op) { + if (f) throw new TypeError("Generator is already executing."); + while (_) try { + if (f = 1, y && (t = op[0] & 2 ? y["return"] : op[0] ? y["throw"] || ((t = y["return"]) && t.call(y), 0) : y.next) && !(t = t.call(y, op[1])).done) return t; + if (y = 0, t) op = [op[0] & 2, t.value]; + switch (op[0]) { + case 0: case 1: t = op; break; + case 4: _.label++; return { value: op[1], done: false }; + case 5: _.label++; y = op[1]; op = [0]; continue; + case 7: op = _.ops.pop(); _.trys.pop(); continue; + default: + if (!(t = _.trys, t = t.length > 0 && t[t.length - 1]) && (op[0] === 6 || op[0] === 2)) { _ = 0; continue; } + if (op[0] === 3 && (!t || (op[1] > t[0] && op[1] < t[3]))) { _.label = op[1]; break; } + if (op[0] === 6 && _.label < t[1]) { _.label = t[1]; t = op; break; } + if (t && _.label < t[2]) { _.label = t[2]; _.ops.push(op); break; } + if (t[2]) _.ops.pop(); + _.trys.pop(); continue; + } + op = body.call(thisArg, _); + } catch (e) { op = [6, e]; y = 0; } finally { f = t = 0; } + if (op[0] & 5) throw op[1]; return { value: op[0] ? op[1] : void 0, done: true }; + } +}; +Object.defineProperty(exports, "__esModule", { value: true }); +exports.withTransientRetry = exports.describeError = exports.isTransientError = exports.httpStatus = void 0; +/** + * Helpers for surviving GitHub API errors that say nothing about the PR being + * checked — 5xx during an incident, rate limiting, or a dropped connection. + */ +var core = __importStar(require("@actions/core")); +/** + * Several copies of `@octokit/request-error` end up installed side by side, so + * the class an octokit error is an instance of depends on which copy threw it + * and `instanceof RequestError` can't be relied on. The `status` property is + * stable across all of them. + */ +var httpStatus = function (error) { + var status = error === null || error === void 0 ? void 0 : error.status; + return typeof status === 'number' ? status : undefined; +}; +exports.httpStatus = httpStatus; +var TRANSIENT_NETWORK_CODES = new Set([ + 'ECONNABORTED', + 'ECONNREFUSED', + 'ECONNRESET', + 'EAI_AGAIN', + 'EHOSTUNREACH', + 'ENETUNREACH', + 'ENOTFOUND', + 'EPIPE', + 'ETIMEDOUT', + 'UND_ERR_CONNECT_TIMEOUT', + 'UND_ERR_SOCKET' +]); +var isTransientError = function (error) { + var status = (0, exports.httpStatus)(error); + if (status !== undefined) { + // 408 request timeout, 429 rate/abuse limit, 5xx server or gateway failure + return status >= 500 || status === 408 || status === 429; + } + var code = error === null || error === void 0 ? void 0 : error.code; + if (typeof code === 'string' && TRANSIENT_NETWORK_CODES.has(code)) { + return true; + } + var message = error === null || error === void 0 ? void 0 : error.message; + return typeof message === 'string' && /socket hang up|network timeout|request to .* failed/i.test(message); +}; +exports.isTransientError = isTransientError; +var describeError = function (error) { + var _a; + var status = (0, exports.httpStatus)(error); + var message = (_a = error === null || error === void 0 ? void 0 : error.message) !== null && _a !== void 0 ? _a : String(error); + return status === undefined ? "".concat(message) : "[HTTP ".concat(status, "] ").concat(message); +}; +exports.describeError = describeError; +/** + * Runs `operation`, retrying it on transient failures. Used for the one-shot + * calls made before the check loop starts, which have no later poll to fall + * back on. + */ +var withTransientRetry = function (description, operation, attempts, delayMs) { + if (attempts === void 0) { attempts = 5; } + if (delayMs === void 0) { delayMs = 5000; } + return __awaiter(void 0, void 0, void 0, function () { + var _loop_1, attempt, state_1; + return __generator(this, function (_a) { + switch (_a.label) { + case 0: + _loop_1 = function (attempt) { + var _b, error_1, backoffMs_1; + return __generator(this, function (_c) { + switch (_c.label) { + case 0: + _c.trys.push([0, 2, , 4]); + _b = {}; + return [4 /*yield*/, operation()]; + case 1: return [2 /*return*/, (_b.value = _c.sent(), _b)]; + case 2: + error_1 = _c.sent(); + if (attempt >= attempts || !(0, exports.isTransientError)(error_1)) { + throw error_1; + } + backoffMs_1 = delayMs * attempt; + core.warning("".concat(description, " failed with a transient error, retrying in ").concat(backoffMs_1 / 1000, "s") + + " (attempt ".concat(attempt, "/").concat(attempts, "): ").concat((0, exports.describeError)(error_1))); + return [4 /*yield*/, new Promise(function (resolve) { return setTimeout(resolve, backoffMs_1); })]; + case 3: + _c.sent(); + return [3 /*break*/, 4]; + case 4: return [2 /*return*/]; + } + }); + }; + attempt = 1; + _a.label = 1; + case 1: return [5 /*yield**/, _loop_1(attempt)]; + case 2: + state_1 = _a.sent(); + if (typeof state_1 === "object") + return [2 /*return*/, state_1.value]; + _a.label = 3; + case 3: + attempt++; + return [3 /*break*/, 1]; + case 4: return [2 /*return*/]; + } + }); + }); +}; +exports.withTransientRetry = withTransientRetry; diff --git a/src/check-group/core/config_getter.ts b/src/check-group/core/config_getter.ts index dcc11e99..fb19ffa5 100644 --- a/src/check-group/core/config_getter.ts +++ b/src/check-group/core/config_getter.ts @@ -3,6 +3,7 @@ import { Context } from "probot"; import { parseUserConfig } from "./user_config_parser"; import { PullRequestEvent } from '@octokit/webhooks-types'; import * as core from '@actions/core' +import { withTransientRetry } from "./transient_error"; /** * Fetches the app configuration from the user's repository. @@ -32,6 +33,9 @@ export const fetchConfig = async (context: Context): Promise = const readConfig = async (context: Context, branch: string): Promise> => { const params = context.repo({path: '.github/checkgroup.yml'}) // https://github.com/probot/octokit-plugin-config - const { config } = await context.octokit.config.get({...params, branch: branch}) + const { config } = await withTransientRetry( + `Reading '.github/checkgroup.yml' from '${branch}'`, + () => context.octokit.config.get({...params, branch: branch}), + ) return config } diff --git a/src/check-group/core/index.ts b/src/check-group/core/index.ts index 1422d84c..3e5147dc 100644 --- a/src/check-group/core/index.ts +++ b/src/check-group/core/index.ts @@ -12,7 +12,7 @@ import { getSubProjResult } from "./satisfy_expected_checks"; import { fetchConfig } from "./config_getter"; import type { CheckGroupConfig, CheckResult, SubProjConfig } from "../types"; import type { Context } from "probot"; -import { RequestError } from "@octokit/request-error"; +import { describeError, httpStatus, isTransientError, withTransientRetry } from "./transient_error"; /** * The orchestration class. @@ -28,6 +28,7 @@ export class CheckGroup { inputs: Record = {}; canComment: boolean = true; + lastTransientError: string = ""; constructor( pullRequestNumber: number, @@ -73,6 +74,9 @@ export class CheckGroup { clearTimeout(this.intervalTimer) core.setFailed( `The timeout of ${timeout} minutes has triggered but not all required jobs were passing.` + + (this.lastTransientError + ? ` The GitHub API was also returning errors, the last being: ${this.lastTransientError}` + : ``) + ` This job will need to be re-run to merge your PR.` + ` If you do not have write access to the repository you can ask ${maintainers} to re-run it for you.` + ` If you have any other questions, you can reach out to ${owner} for help.` @@ -88,7 +92,7 @@ export class CheckGroup { const postedChecks = await getPostedChecks(this.context, this.sha); core.debug(`postedChecks: ${JSON.stringify(postedChecks)}`); const result = getSubProjResult(subprojs, postedChecks); - this.notifyProgress(subprojs, postedChecks, result) + await this.notifyProgress(subprojs, postedChecks, result) core.endGroup(); if (result === "all_passing") { @@ -100,6 +104,18 @@ export class CheckGroup { } } catch (error) { + core.endGroup(); + if (isTransientError(error)) { + // A GitHub incident says nothing about the PR, so keep polling until the + // timeout timer fires rather than failing a PR whose checks are green. + this.lastTransientError = describeError(error) + core.warning( + `Check ${tries} hit a transient GitHub API error, retrying in ${interval / 1000}s:` + + ` ${this.lastTransientError}` + ) + this.intervalTimer = setTimeout(() => this.runCheck(subprojs, tries + 1, interval), interval); + return; + } // bubble up the error to the job core.setFailed(error); clearTimeout(this.intervalTimer) @@ -119,15 +135,17 @@ export class CheckGroup { try { await commentOnPr(this.context, result, this.inputs, subprojs, postedChecks) } catch (e) { - if (e instanceof RequestError && e.status === 403) { + // The comment is informational, so a failure to write it must never fail + // the job — the check statuses it summarises are unaffected. + if (httpStatus(e) === 403) { // Forbidden: Resource not accessible by integration if (this.canComment) { - core.info(`Failed to comment on the PR: ${JSON.stringify(e)}`) + core.info(`Failed to comment on the PR: ${describeError(e)}`) } // Use this boolean to only print the info message once this.canComment = false } else { - throw e + core.warning(`Failed to update the PR comment: ${describeError(e)}`) } } } @@ -137,10 +155,13 @@ export class CheckGroup { * a pull request. */ async files(): Promise { - const pullRequestFiles = await this.context.octokit.paginate( - this.context.octokit.pulls.listFiles, - this.context.repo({"pull_number": this.pullRequestNumber}), - (response) => response.data, + const pullRequestFiles = await withTransientRetry( + "Listing the files changed in the PR", + () => this.context.octokit.paginate( + this.context.octokit.pulls.listFiles, + this.context.repo({"pull_number": this.pullRequestNumber}), + (response) => response.data, + ), ); const filenames: string[] = []; pullRequestFiles.forEach((pullRequestFile: any) => { diff --git a/src/check-group/core/transient_error.ts b/src/check-group/core/transient_error.ts new file mode 100644 index 00000000..86381a85 --- /dev/null +++ b/src/check-group/core/transient_error.ts @@ -0,0 +1,78 @@ +/** + * Helpers for surviving GitHub API errors that say nothing about the PR being + * checked — 5xx during an incident, rate limiting, or a dropped connection. + */ +import * as core from '@actions/core'; + +/** + * Several copies of `@octokit/request-error` end up installed side by side, so + * the class an octokit error is an instance of depends on which copy threw it + * and `instanceof RequestError` can't be relied on. The `status` property is + * stable across all of them. + */ +export const httpStatus = (error: unknown): number | undefined => { + const status = (error as {status?: unknown})?.status; + return typeof status === 'number' ? status : undefined; +}; + +const TRANSIENT_NETWORK_CODES = new Set([ + 'ECONNABORTED', + 'ECONNREFUSED', + 'ECONNRESET', + 'EAI_AGAIN', + 'EHOSTUNREACH', + 'ENETUNREACH', + 'ENOTFOUND', + 'EPIPE', + 'ETIMEDOUT', + 'UND_ERR_CONNECT_TIMEOUT', + 'UND_ERR_SOCKET' +]); + +export const isTransientError = (error: unknown): boolean => { + const status = httpStatus(error); + if (status !== undefined) { + // 408 request timeout, 429 rate/abuse limit, 5xx server or gateway failure + return status >= 500 || status === 408 || status === 429; + } + const code = (error as {code?: unknown})?.code; + if (typeof code === 'string' && TRANSIENT_NETWORK_CODES.has(code)) { + return true; + } + const message = (error as {message?: unknown})?.message; + return typeof message === 'string' && /socket hang up|network timeout|request to .* failed/i.test(message); +}; + +export const describeError = (error: unknown): string => { + const status = httpStatus(error); + const message = (error as {message?: unknown})?.message ?? String(error); + return status === undefined ? `${message}` : `[HTTP ${status}] ${message}`; +}; + +/** + * Runs `operation`, retrying it on transient failures. Used for the one-shot + * calls made before the check loop starts, which have no later poll to fall + * back on. + */ +export const withTransientRetry = async ( + description: string, + operation: () => Promise, + attempts = 5, + delayMs = 5000 +): Promise => { + for (let attempt = 1; ; attempt++) { + try { + return await operation(); + } catch (error) { + if (attempt >= attempts || !isTransientError(error)) { + throw error; + } + const backoffMs = delayMs * attempt; + core.warning( + `${description} failed with a transient error, retrying in ${backoffMs / 1000}s` + + ` (attempt ${attempt}/${attempts}): ${describeError(error)}` + ); + await new Promise(resolve => setTimeout(resolve, backoffMs)); + } + } +};