From 1763e1520ae770365c9f3e3cef3749bed172aca8 Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Wed, 12 Aug 2026 18:23:48 +0200 Subject: [PATCH 1/2] fix(scripts-github): stop hammering the GitHub API after an auth failure When the changelog renderer resolves PRs for a release, it calls the GitHub API once per changelog entry. If the token is expired, revoked or blocked by policy, every one of those calls fails with 401/403 - a v8 release logged 20 consecutive auth errors, and a v9 release would log ~90. The real problem (one bad token) was buried in the noise. Short-circuit after the first auth failure: the failure is logged once, and subsequent lookups skip the request and fall back to no PR link. Non-auth errors keep their existing per-entry behaviour, since those are genuinely per-PR (deleted PR, race, transient 5xx). Also exports hasGitHubAuthFailed()/resetGitHubAuthFailure() so callers can detect the degraded state, and fixes a pre-existing lint error on the type import. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- scripts/github/src/index.ts | 7 ++++- scripts/github/src/pullRequests.ts | 48 +++++++++++++++++++++++++++++- 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/scripts/github/src/index.ts b/scripts/github/src/index.ts index b184f7ce143e1..e62e713263d39 100644 --- a/scripts/github/src/index.ts +++ b/scripts/github/src/index.ts @@ -1,4 +1,9 @@ export { fluentRepoDetails } from './constants'; export type { IGetPullRequestFromCommitParams } from './pullRequests'; -export { getPullRequestForCommit, processPullRequestApiResponse } from './pullRequests'; +export { + getPullRequestForCommit, + hasGitHubAuthFailed, + processPullRequestApiResponse, + resetGitHubAuthFailure, +} from './pullRequests'; export type { IPullRequest, IRepoDetails, IUser } from './types'; diff --git a/scripts/github/src/pullRequests.ts b/scripts/github/src/pullRequests.ts index 4d38ed109c51d..8c397ecaee404 100644 --- a/scripts/github/src/pullRequests.ts +++ b/scripts/github/src/pullRequests.ts @@ -1,5 +1,24 @@ import type { Octokit, RestEndpointMethodTypes } from '@octokit/rest'; -import { IPullRequest, IRepoDetails } from './types'; +import type { IPullRequest, IRepoDetails } from './types'; + +/** + * Once GitHub rejects our credentials, every subsequent request will be rejected too. Tracking this + * lets callers short-circuit instead of retrying for every changelog entry. + * + * Context: during the 2026-06-30 release an expired PAT produced ~180 near-identical 403 stack + * traces, which buried the single line that actually mattered (the failed git push). + */ +let authFailureLogged = false; + +/** True if a previous GitHub API call failed with an authentication/authorization error. */ +export function hasGitHubAuthFailed(): boolean { + return authFailureLogged; +} + +/** Reset the cached auth-failure state (intended for tests). */ +export function resetGitHubAuthFailure(): void { + authFailureLogged = false; +} // eslint-disable-next-line @typescript-eslint/naming-convention export interface IGetPullRequestFromCommitParams { @@ -21,6 +40,12 @@ export async function getPullRequestForCommit( ): Promise { const { github, repoDetails, commit, authorEmail, verbose } = params; + // Skip the request entirely if we already know the credentials are rejected. Without this, a bad + // token causes one failed request (plus a full stack trace) for every single changelog entry. + if (authFailureLogged) { + return; + } + verbose && console.log(`Looking for the PR containing ${commit}...`); try { @@ -44,6 +69,27 @@ export async function getPullRequestForCommit( return processPullRequestApiResponse(prs[0], authorEmail); } } catch (ex) { + const status = (ex as { status?: number }).status; + + // 401/403 means the token is bad, expired, or blocked by policy - retrying for every remaining + // commit is pure noise. Log once with the actionable detail, then degrade gracefully: changelog + // entries fall back to commit links instead of PR links. + if (status === 401 || status === 403) { + authFailureLogged = true; + const message = (ex as { message?: string }).message ?? 'Unknown error'; + console.warn( + [ + '', + `##vso[task.logissue type=warning]GitHub API authentication failed (HTTP ${status}) while building changelogs.`, + ` ${message}`, + ' Changelog entries will link to commits instead of pull requests.', + ' Further PR lookups are skipped for this run.', + '', + ].join('\n'), + ); + return; + } + console.warn(`Error finding PR for ${commit}`, ex); return; } From b26a75c34c14fdc0de73be6129832e36a9d40788 Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Wed, 12 Aug 2026 19:36:37 +0200 Subject: [PATCH 2/2] fix(scripts-github): don't report rate limiting as an authentication failure GitHub overloads 403: it means 'forbidden' for a bad or policy-blocked token, but also for primary and secondary rate limits. Reporting all of them as 'authentication failed' would send someone to rotate a perfectly good PAT in the middle of a release - the exact misdirection this logging exists to stop. Tell them apart via the rate-limit headers (retry-after, x-ratelimit-remaining) and handle 429 too. Both still latch, since further lookups cannot succeed either way, but the advice now matches the cause. Also renames the module-level flag: it gates requests and backs hasGitHubAuthFailed(), so 'authFailureLogged' understated what it controlled. It now records why lookups are disabled, and hasGitHubAuthFailed() reports only genuine auth failures. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- scripts/github/src/pullRequests.ts | 75 ++++++++++++++++++++++++------ 1 file changed, 60 insertions(+), 15 deletions(-) diff --git a/scripts/github/src/pullRequests.ts b/scripts/github/src/pullRequests.ts index 8c397ecaee404..efc72584cee37 100644 --- a/scripts/github/src/pullRequests.ts +++ b/scripts/github/src/pullRequests.ts @@ -2,22 +2,56 @@ import type { Octokit, RestEndpointMethodTypes } from '@octokit/rest'; import type { IPullRequest, IRepoDetails } from './types'; /** - * Once GitHub rejects our credentials, every subsequent request will be rejected too. Tracking this - * lets callers short-circuit instead of retrying for every changelog entry. + * Why further PR lookups are being skipped, if they are. + * + * Once GitHub rejects our credentials every subsequent request will be rejected too, and once we + * are rate limited the remaining lookups will only make it worse - in both cases the useful move is + * to stop asking. The reason is kept because the two need very different advice. * * Context: during the 2026-06-30 release an expired PAT produced ~180 near-identical 403 stack * traces, which buried the single line that actually mattered (the failed git push). */ -let authFailureLogged = false; +let lookupsDisabledReason: 'auth' | 'rate-limit' | undefined; /** True if a previous GitHub API call failed with an authentication/authorization error. */ export function hasGitHubAuthFailed(): boolean { - return authFailureLogged; + return lookupsDisabledReason === 'auth'; } -/** Reset the cached auth-failure state (intended for tests). */ +/** Reset the cached failure state (intended for tests). */ export function resetGitHubAuthFailure(): void { - authFailureLogged = false; + lookupsDisabledReason = undefined; +} + +/** + * Separate a genuine credential problem from rate limiting. + * + * GitHub overloads 403: it means "forbidden" for a bad or policy-blocked token, but also for + * primary and secondary rate limits. Telling someone to rotate a perfectly good PAT in the middle + * of a release is exactly the kind of misdirection this logging is meant to prevent, so the two are + * told apart by the rate-limit headers GitHub sends. + */ +function classifyFailure(ex: unknown): 'auth' | 'rate-limit' | undefined { + const status = (ex as { status?: number }).status; + + if (status === 429) { + return 'rate-limit'; + } + + if (status !== 401 && status !== 403) { + return undefined; + } + + const headers = ((ex as { response?: { headers?: Record } }).response?.headers ?? {}) as Record< + string, + unknown + >; + + if (headers['retry-after'] !== undefined || String(headers['x-ratelimit-remaining']) === '0') { + return 'rate-limit'; + } + + return 'auth'; } // eslint-disable-next-line @typescript-eslint/naming-convention @@ -40,9 +74,9 @@ export async function getPullRequestForCommit( ): Promise { const { github, repoDetails, commit, authorEmail, verbose } = params; - // Skip the request entirely if we already know the credentials are rejected. Without this, a bad - // token causes one failed request (plus a full stack trace) for every single changelog entry. - if (authFailureLogged) { + // Skip the request entirely if we already know it cannot succeed. Without this, a bad token + // causes one failed request (plus a full stack trace) for every single changelog entry. + if (lookupsDisabledReason) { return; } @@ -70,18 +104,29 @@ export async function getPullRequestForCommit( } } catch (ex) { const status = (ex as { status?: number }).status; + const reason = classifyFailure(ex); - // 401/403 means the token is bad, expired, or blocked by policy - retrying for every remaining - // commit is pure noise. Log once with the actionable detail, then degrade gracefully: changelog - // entries fall back to commit links instead of PR links. - if (status === 401 || status === 403) { - authFailureLogged = true; + // A rejected token or a rate limit means retrying for every remaining commit is pure noise. + // Log once with the actionable detail, then degrade gracefully: changelog entries fall back to + // commit links instead of PR links. + if (reason) { + lookupsDisabledReason = reason; const message = (ex as { message?: string }).message ?? 'Unknown error'; + const headline = + reason === 'auth' + ? `GitHub API authentication failed (HTTP ${status}) while building changelogs.` + : `GitHub API rate limit reached (HTTP ${status}) while building changelogs.`; + const advice = + reason === 'auth' + ? ' Check that the pipeline token is valid and has not expired.' + : ' This is a rate limit, not a token problem - the token does not need rotating.'; + console.warn( [ '', - `##vso[task.logissue type=warning]GitHub API authentication failed (HTTP ${status}) while building changelogs.`, + `##vso[task.logissue type=warning]${headline}`, ` ${message}`, + advice, ' Changelog entries will link to commits instead of pull requests.', ' Further PR lookups are skipped for this run.', '',