From ceed61d5575a1d2078c0f36443177b0cbf42de25 Mon Sep 17 00:00:00 2001 From: Pascal Garber Date: Tue, 15 Sep 2026 22:34:05 +0200 Subject: [PATCH 1/2] fix(release): unknown npm codes retry and say so MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Twice a publish defect has been fixed by adding a code to one of two lists, and twice the partition kept its shape: two named halves and an unnamed remainder that silently meant "give up". `E409` in 4.2.0, then `IDENTITY_TOKEN_READ_ERROR` in 5.2.0, where `@girs/clutter-7` and `@girs/meta-8` died on a transient OIDC token read 1h26m and 2h50m into a 3h10m sweep, were retried ZERO times, and sat at 5.1.0 while the other 714 went to 5.2.0. A third one-line fix would have been the third time this was written down and the third time it recurred. So the classifier now answers three ways, and "unknown" is a value the caller must handle. An unrecognised code DEFAULTS TO RETRY and announces itself by name — a log line per package and one `::warning::` per distinct code, saying which list it belongs in — so the fourth occurrence is a grep away instead of a diff of 716 versions against the registry. The default is only safe because the budget is small. Measured against this repository's own settings (`NPM_MAX_RETRIES=10`, base 5 s, cap 300 s) the full budget is 11 attempts and 25 minutes of sleep per package: a systematic unrecognised code would exhaust the 360-minute job after about 12 of 716 packages, trading two missing packages for a stopped release. Two retries costs ~35 s and still catches the blip this exists for. `IDENTITY_TOKEN_READ_ERROR` is also named outright — being recognised beats being caught by the default. Self-test vectors cover the unknown case, including a code that deliberately does not exist; reverting the default to "terminal" fails the self-test on exactly those vectors. release.yml gains the header it never had: a merge to main publishes, measured on PR #20, plus what npm's own `max-age=300` does to a retry loop that reads the registry. --- .github/release-script/src/index.ts | 148 ++++++++++++++++++++++++++-- .github/workflows/release.yml | 25 +++++ 2 files changed, 164 insertions(+), 9 deletions(-) diff --git a/.github/release-script/src/index.ts b/.github/release-script/src/index.ts index 97205eea31..96f6b4a02b 100644 --- a/.github/release-script/src/index.ts +++ b/.github/release-script/src/index.ts @@ -200,7 +200,23 @@ function isTerminalNpmError(message: string): boolean { * already-published before this is ever consulted. The two 409-shaped * conditions must never collapse into one rule. */ -const RETRYABLE_NPM_CODES = new Set(["E409", "E500", "E502", "E503", "E504", "ETIMEDOUT", "ECONNRESET"]); +const RETRYABLE_NPM_CODES = new Set([ + "E409", + "E500", + "E502", + "E503", + "E504", + "ETIMEDOUT", + "ECONNRESET", + // The 5.2.0 release, again: `@girs/clutter-7` and `@girs/meta-8` died on + // npm error code IDENTITY_TOKEN_READ_ERROR + // npm error error retrieving identity token + // — the OIDC token read for Trusted Publishing hiccuping 1h26m and 2h50m + // into a 3h10m sweep. Transient by nature, retried ZERO times, because the + // code was in neither list. Two packages sat at 5.1.0 while the other 714 + // were at 5.2.0, and nothing was red enough to say which two. + "IDENTITY_TOKEN_READ_ERROR", +]); function isRetryableNpmError(message: string): boolean { const code = npmErrorCode(message); @@ -214,18 +230,52 @@ function isRetryableNpmError(message: string): boolean { * `shouldRetry` closure that actually decides was not, which is how a code that * is neither terminal nor rate-limited came to mean "give up". */ -function isRetryablePublishError(message: string): boolean { +export type PublishErrorVerdict = "terminal" | "retry" | "unknown"; + +/** + * THREE answers, because the third one is what kept going wrong. Twice now a + * defect has been fixed by adding a code to one of the two lists, and twice the + * partition stayed the same shape: two named halves and an unnamed remainder + * that silently meant "give up". `E409` in the 4.2.0 release, then + * `IDENTITY_TOKEN_READ_ERROR` in 5.2.0 — each time the code we had already been + * bitten by got named and the next one stayed invisible. + * + * So "I do not recognise this" is now a value the caller must handle, not the + * absence of one. Naming it is the whole point: a fourth occurrence should cost + * one line, not another three-hour diagnosis from a registry diff. + */ +function classifyPublishError(message: string): PublishErrorVerdict { // A settled answer wins over every other signal, so a message that happens // to contain a transport word cannot resurrect it. - if (isTerminalNpmError(message)) return false; + if (isTerminalNpmError(message)) return "terminal"; const lower = message.toLowerCase(); - return ( + if ( isRateLimitedError(message) || isRetryableNpmError(message) || lower.includes("econnreset") || lower.includes("etimedout") || lower.includes("socket hang up") - ); + ) { + return "retry"; + } + return "unknown"; +} + +/** + * An unrecognised error DEFAULTS TO RETRY: a publish that fails once and would + * have worked costs a broken release nobody notices for hours, while a retry + * that was never going to work costs seconds — but only if the budget is small. + * Measured against this repository's own settings (`NPM_MAX_RETRIES=10`, + * base 5 s, cap 300 s) the FULL budget is 11 attempts and 25 minutes of sleep + * per package: at that price a systematic unrecognised code would exhaust the + * 360-minute job after about 12 of 716 packages, turning "two packages missing" + * into "the release stopped". Two retries keeps the premise true — an unknown + * costs ~35 s and still catches the transient blip this exists for. + */ +const UNKNOWN_CODE_MAX_RETRIES = 2; + +function isRetryablePublishError(message: string): boolean { + return classifyPublishError(message) !== "terminal"; } /** @@ -240,7 +290,15 @@ function isRetryablePublishError(message: string): boolean { * Vector 2 is the incident, verbatim in shape: npm prints the tarball shasum on * every attempt, and `838bf765429e…` carries "429" at offset 8. */ -const CLASSIFIER_VECTORS: { name: string; message: string; rateLimited: boolean; terminal: boolean; retryable: boolean }[] = [ +const CLASSIFIER_VECTORS: { + name: string; + message: string; + rateLimited: boolean; + terminal: boolean; + retryable: boolean; + /** Omitted where it follows from `terminal`; REQUIRED for the unknown case. */ + verdict?: PublishErrorVerdict; +}[] = [ { name: "E429 is a rate limit", message: "npm error code E429\nnpm error 429 Too Many Requests", @@ -322,6 +380,42 @@ const CLASSIFIER_VECTORS: { name: string; message: string; rateLimited: boolean; terminal: true, retryable: false, }, + { + name: "IDENTITY_TOKEN_READ_ERROR is retryable — the 5.2.0 incident, verbatim", + message: "npm error code IDENTITY_TOKEN_READ_ERROR\nnpm error error retrieving identity token\nnpm error cause undefined", + rateLimited: false, + terminal: false, + retryable: true, + verdict: "retry", + }, + { + // THE vector the partition kept missing. It is deliberately a code that + // does not exist: the test must fail the day someone makes "unrecognised" + // mean "give up" again, and it cannot do that if it is written against a + // code the classifier already knows. + name: "an unrecognised code is UNKNOWN, and unknown retries", + message: "npm error code EWATERMELON\nnpm error something nobody has seen yet", + rateLimited: false, + terminal: false, + retryable: true, + verdict: "unknown", + }, + { + name: "no code line at all is UNKNOWN, not terminal", + message: "publish exited with status 1", + rateLimited: false, + terminal: false, + retryable: true, + verdict: "unknown", + }, + { + name: "an unknown code must not be resurrected into a rate limit by a shasum", + message: "npm error code EWATERMELON\nnpm notice shasum 838bf765429ea1b2c3d4", + rateLimited: false, + terminal: false, + retryable: true, + verdict: "unknown", + }, ]; function selfTestClassifier(): void { @@ -336,6 +430,13 @@ function selfTestClassifier(): void { if (isRetryablePublishError(v.message) !== v.retryable) { failures.push(`${v.name}: expected retryable=${v.retryable}`); } + // The three-way answer, not the boolean: "unknown" and "retry" both + // retry, so a vector that only checked `retryable` would pass while the + // unknown case quietly collapsed back into one of the named halves. + const expected = v.verdict ?? (v.terminal ? "terminal" : "retry"); + if (classifyPublishError(v.message) !== expected) { + failures.push(`${v.name}: expected verdict=${expected}, got ${classifyPublishError(v.message)}`); + } } if (failures.length > 0) { throw new Error(`retry-classifier self-test FAILED:\n ${failures.join("\n ")}`); @@ -364,7 +465,8 @@ interface RetryConfig { maxRetries: number; baseDelayMs: number; maxDelayMs: number; - shouldRetry: (error: unknown) => boolean; + /** `attempt` is 0-based: the budget a verdict gets can differ per verdict. */ + shouldRetry: (error: unknown, attempt: number) => boolean; onRetry?: (attempt: number, waitMs: number, error: unknown) => void; } @@ -373,7 +475,7 @@ async function withRetry(fn: () => Promise, config: RetryConfig): Promise< try { return await fn(); } catch (error) { - if (attempt < config.maxRetries && config.shouldRetry(error)) { + if (attempt < config.maxRetries && config.shouldRetry(error, attempt)) { const wait = calcBackoffMs(attempt, config.baseDelayMs, config.maxDelayMs); config.onRetry?.(attempt + 1, wait, error); await sleep(wait); @@ -716,6 +818,25 @@ async function publishPackageOnce(pkg: Package, config: Config): Promise { }); } +/** Codes already announced, so one bad release cannot emit 716 identical warnings. */ +const announcedUnknownCodes = new Set(); + +/** + * Kept OUT of `classifyPublishError` on purpose: that function stays pure so the + * startup self-test can run the real decision rather than a copy of it. + */ +function announceUnknownPublishError(message: string, pkg: Package): void { + const code = npmErrorCode(message) ?? "(no npm error code)"; + console.log(`❓ ${pkg.name}@${pkg.version}: unrecognised npm error ${code} — retrying as a precaution`); + if (announcedUnknownCodes.has(code)) return; + announcedUnknownCodes.add(code); + console.log( + `::warning::unrecognised npm error code ${code} — the retry classifier knows neither that it is ` + + "permanent nor that it is transient, so it is being retried. If it is transient, add it to " + + "RETRYABLE_NPM_CODES; if it is permanent, add it to TERMINAL_NPM_CODES.", + ); +} + async function publishPackageWithRetry(pkg: Package, config: Config): Promise { await withRetry( () => publishPackageOnce(pkg, config), @@ -724,7 +845,16 @@ async function publishPackageWithRetry(pkg: Package, config: Config): Promise isRetryablePublishError(err instanceof Error ? err.message : String(err)), + shouldRetry: (err, attempt) => { + const message = err instanceof Error ? err.message : String(err); + const verdict = classifyPublishError(message); + if (verdict !== "unknown") return verdict === "retry"; + // Say so, loudly and by name. The next unrecognised code has to be + // findable by reading the log rather than by diffing 716 package + // versions against the registry, which is how this one was found. + announceUnknownPublishError(message, pkg); + return attempt < UNKNOWN_CODE_MAX_RETRIES; + }, onRetry: (attempt, wait, err) => { const message = err instanceof Error ? err.message : String(err); console.log(`⏳ ${pkg.name}@${pkg.version} retry ${attempt}/${MAX_RETRIES_PUBLISH} in ${wait}ms: ${message}`); diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 60b3d8dde5..aae508af28 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -1,3 +1,28 @@ +# Publishes the ~700 per-namespace `@girs/*` packages from the committed tree. +# +# ⚠️ A MERGE TO `main` PUBLISHES. This workflow triggers on `push: main`, not only on a +# release, so ANY merge — a one-line workflow fix, a typo in a comment — starts a full publish +# sweep of every package in the tree. That is usually harmless, because a package whose version +# npm already serves is reported as already published and skipped, so the sweep is a no-op that +# costs an hour of runner time. It is NOT harmless when the tree carries versions the registry +# does not have yet: then the merge publishes them, and publishing is irreversible. +# +# Measured, so this is not hypothetical: merging PR #20 (a change to `sdk-types.yml`, touching +# nothing else) started run 35017963560, which reported `714 already published, 2 to publish` +# and published `@girs/clutter-7@5.2.0` and `@girs/meta-8@5.2.0`. Those two were genuinely +# missing and the publish was the right outcome — but it was a SIDE EFFECT of a merge, not a +# decision anybody made. Before merging anything here, know which versions in the tree the +# registry is missing; `npm view` will not tell you reliably (see below). +# +# ⚠️ READING npm TELLS YOU WHAT npm CACHED. A packument is served +# `cache-control: public, max-age=300`, the abbreviated form npm installs from included, and +# npm ships `prefer-online=false`, so it does not re-check data it still considers fresh. +# A retry loop polling npm every 30 seconds therefore spends its first ten attempts re-reading +# ONE five-minute-old snapshot: twenty attempts, two real lookups. Anyone writing a retry +# against the registry needs `npm_config_prefer_online=true` (see the `Require a generator that +# can bundle` step in `sdk-types.yml`), and anyone VERIFYING what got published needs a real +# cache buster — `curl -H 'Cache-Control: no-cache' ".../@girs%2fname?cb=$(date +%s)"` — because +# `npm view` and a bare `curl` will both hand back a stale packument and look authoritative. name: Release CI on: From 9a60dca69f3d65f3ceaf852277c48cfd5192ad5e Mon Sep 17 00:00:00 2001 From: Pascal Garber Date: Tue, 15 Sep 2026 22:39:08 +0200 Subject: [PATCH 2/2] docs(release): say why the warnings live here --- .github/workflows/release.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index aae508af28..c6e12bd5a9 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -1,5 +1,12 @@ # Publishes the ~700 per-namespace `@girs/*` packages from the committed tree. # +# The operational warnings below live in a workflow header, not in a README, because this +# repository has no README, CONTRIBUTING or AGENTS.md to put them in: its root is ~700 +# GENERATED namespace directories that the type generator rewrites wholesale on every release. +# `.github/` is the part of the tree the generator does not touch, so it is where prose +# survives. Please do not "tidy" this into a README — there isn't one, and a new one would be +# the first thing a regeneration removes. +# # ⚠️ A MERGE TO `main` PUBLISHES. This workflow triggers on `push: main`, not only on a # release, so ANY merge — a one-line workflow fix, a typo in a comment — starts a full publish # sweep of every package in the tree. That is usually harmless, because a package whose version