From 731600801a23f5601654295682dcfccce0240411 Mon Sep 17 00:00:00 2001 From: romanett Date: Wed, 7 Oct 2026 18:19:19 +0200 Subject: [PATCH] Release: stop retrying 404s in the feed gates; keep GitHub Release notes under the size limit (#4696) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description `Release to nuget.org` [run 37591540644](https://github.com/OPCFoundation/UA-.NETStandard/actions/runs/37591540644) was the stable 2.0.0 promotion. It spent more than an hour in **Verify already-published package identity**. It pushed every package to both feeds and created the `2.0.0` tag, then [failed](https://github.com/OPCFoundation/UA-.NETStandard/actions/runs/37591540644/job/112693955843) in **Tag published package candidate** before the GitHub Release existed. This PR fixes both problems. ## 1. Feed gates retried every 404 PowerShell 7's `Invoke-WebRequest -MaximumRetryCount` retries every status from 400 to 599, and that includes 404. `-SkipHttpErrorCheck` stops the error from being thrown but doesn't stop the retries. Both feed gates ask once per package whether an id/version is published, and 404 is the normal "not yet" answer. Each absent package therefore cost 3 retries × 5 s, about 15 s. A stable set is never pushed automatically, so for 2.0.0 every package, `.Debug` included, was absent on **both** feeds. - `package-version-policy.ps1`: adds `Invoke-FeedRequest`. It retries only network failures, 408, 429 and 5xx, and returns every other response straight away. After the last retry it returns the final response, or rethrows the network error. - `assert-published-packages-match.ps1` and `validate-preview-package-ordering.ps1` call it instead of `Invoke-WebRequest -MaximumRetryCount 3 -RetryIntervalSec 5`. The status handling in each caller is unchanged, so the gates still fail closed: anything other than 200 or 404 still blocks. - `PackageVersionPolicyTests.GetGitHubPackagesPublishedVersionPagesResultsAndFailsClosedAsync` now dot-sources the policy script so the extracted function can find `Invoke-FeedRequest`. The `Invoke-WebRequest` stub still intercepts the call. - New stub-based tests check that `Invoke-FeedRequest` makes one request for 200/400/403/404 and retries 408/429/5xx exactly `MaximumRetryCount` times. They also check that it stops once a retry is not transient, rethrows an exhausted network failure and passes `-OutFile` with `-PassThru`. | Response | Before | After | |---|---|---| | 404 | 4 requests, 15 s | 1 request, immediate (live nuget.org: 0.58 s) | | 403 | 4 requests | 1 request | | 503 / 429 | retried 3× | retried 3× (unchanged) | | connection refused | — | retried, then rethrown | ## 2. GitHub Release body too long ``` HTTP 422: Validation Failed (https://api.github.com/repos/OPCFoundation/UA-.NETStandard/releases) body is too long (maximum is 125000 characters) ``` `gh release create --generate-notes` diffs against the latest release when no previous tag is given. Here that was `1.5.378.182`, which is on another branch, so the generated notes for 2.0.0 came to 191,048 characters. The step now calls `POST /releases/generate-notes` with `previous_tag_name` from the new `Get-PreviousReleaseTag` helper in `package-version-policy.ps1`. It picks the greatest same-major semantic version strictly lower than the tag, ignoring non-semver tags like 1.5.378.x, so a servicing 2.0.1 published after 2.1.0 still diffs against 2.0.0. For 2.0.0 that is `2.0.0-preview.2`, and the notes come to 31,408 characters (measured against the real repo; that call has no side effects). If a body still exceeds 120,000 characters, it is cut at a line boundary and a compare link is appended. The tag and release checks are idempotent, and the pushes use `--skip-duplicate` and verify identity, so re-running the promotion finishes the release. **For run 37591540644 to pass:** the dispatch runs `release.yml` from `release/2.0`, so these changes have to reach `release/2.0` before the promotion is re-run. ## Verification - All 3 scripts parse with 0 errors. The modified workflow step also parses, and the YAML loads. - Live nuget.org: absent version → 404 in 0.58 s; package index → 200. httpbin 503 with `-RetryIntervalSec 1`: retried, 503 after 4.7 s. - `dotnet test tests/Opc.Ua.Tools.Tests`: 939 passed (1 skipped) on net10.0, plus net8/9/48. ## Related Issues - No issue; found investigating run 37591540644. ## Checklist - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [ ] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [ ] I have added all necessary documentation. - [ ] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 --- .../assert-published-packages-match.ps1 | 3 +- .azurepipelines/package-version-policy.ps1 | 102 ++++++++++++ .../validate-preview-package-ordering.ps1 | 6 +- .github/workflows/release.yml | 44 +++++- .../PackageVersionPolicyTests.cs | 146 ++++++++++++++++++ 5 files changed, 294 insertions(+), 7 deletions(-) diff --git a/.azurepipelines/assert-published-packages-match.ps1 b/.azurepipelines/assert-published-packages-match.ps1 index 5796f41689..6c6337cdad 100644 --- a/.azurepipelines/assert-published-packages-match.ps1 +++ b/.azurepipelines/assert-published-packages-match.ps1 @@ -95,8 +95,7 @@ function Get-PublishedPackageBytes { $deadline = [DateTimeOffset]::UtcNow.AddSeconds($TimeoutSeconds) while ($true) { - $response = Invoke-WebRequest -Uri $Uri -Headers $Headers -SkipHttpErrorCheck ` - -MaximumRetryCount 3 -RetryIntervalSec 5 -OutFile $Destination -PassThru + $response = Invoke-FeedRequest -Uri $Uri -Headers $Headers -OutFile $Destination if ($response.StatusCode -eq 404) { if (Test-Path -LiteralPath $Destination) { Remove-Item -LiteralPath $Destination -Force diff --git a/.azurepipelines/package-version-policy.ps1 b/.azurepipelines/package-version-policy.ps1 index 0e80a9cf12..131bb19297 100644 --- a/.azurepipelines/package-version-policy.ps1 +++ b/.azurepipelines/package-version-policy.ps1 @@ -250,6 +250,108 @@ function Get-NuGetPackageContentDigest { } } +function Invoke-FeedRequest { + <# + .SYNOPSIS + Invoke-WebRequest that retries transient failures only and returns + the final response for the caller to judge by status code. + + .DESCRIPTION + Invoke-WebRequest's own -MaximumRetryCount retries every status from + 400 to 599, so each 404 - the normal "this id/version is not + published" answer the release gates ask for once per package - cost + three pointless retries and ~15 s. Across a full package set on two + feeds that turned a read-only check into an hour-long step. + + Only a network failure, 408, 429 or 5xx is retried here. Every other + status is returned straight away; after the last retry the final + response is returned (or the network error rethrown), so callers keep + failing closed on anything they do not recognise. + #> + param( + [Parameter(Mandatory)][string]$Uri, + [hashtable]$Headers = @{}, + [string]$OutFile, + [ValidateRange(0, 10)][int]$MaximumRetryCount = 3, + [ValidateRange(0, 60)][int]$RetryIntervalSec = 5 + ) + + $parameters = @{ + Uri = $Uri + Headers = $Headers + SkipHttpErrorCheck = $true + } + if ($OutFile) { + $parameters['OutFile'] = $OutFile + $parameters['PassThru'] = $true + } + + for ($attempt = 0; ; $attempt++) { + try { + $response = Invoke-WebRequest @parameters + } + catch { + # -SkipHttpErrorCheck means only failures without an HTTP + # status (DNS, connection reset, timeout) reach this block. + if ($attempt -ge $MaximumRetryCount) { + throw + } + Start-Sleep -Seconds $RetryIntervalSec + continue + } + + $status = [int]$response.StatusCode + $transient = $status -eq 408 -or $status -eq 429 -or $status -ge 500 + if (-not $transient -or $attempt -ge $MaximumRetryCount) { + return $response + } + Start-Sleep -Seconds $RetryIntervalSec + } +} + +function Get-PreviousReleaseTag { + <# + .SYNOPSIS + Returns the release tag GitHub should generate release notes from: + the greatest semantic version of the same major that is strictly + lower than $Tag, or $null when there is none. + + .DESCRIPTION + Without a previous tag GitHub diffs against the latest release, which + can be a 1.5.378 maintenance release on another branch; the 2.0.0 + notes generated that way exceeded the 125000-character body limit. + Choosing by version instead of publication time keeps a servicing + promotion (2.0.1 after 2.1.0) on its own line. Tags that are not + semantic versions, such as the four-component 1.5.378.x, are ignored. + #> + param( + [Parameter(Mandatory)][string]$Tag, + [AllowEmptyCollection()][string[]]$ReleaseTags = @() + ) + + $target = $null + if (-not [System.Management.Automation.SemanticVersion]::TryParse($Tag, [ref]$target)) { + throw "Release tag '$Tag' is not a semantic version." + } + + $best = $null + $bestTag = $null + foreach ($candidateTag in $ReleaseTags) { + $candidate = $null + if (-not [System.Management.Automation.SemanticVersion]::TryParse($candidateTag, [ref]$candidate)) { + continue + } + if ($candidate.Major -ne $target.Major -or $candidate -ge $target) { + continue + } + if ($null -eq $best -or $candidate -gt $best) { + $best = $candidate + $bestTag = $candidateTag + } + } + return $bestTag +} + function Test-StablePackageVersion { <# .SYNOPSIS diff --git a/.azurepipelines/validate-preview-package-ordering.ps1 b/.azurepipelines/validate-preview-package-ordering.ps1 index 04e0324bc4..130c7ed0cb 100644 --- a/.azurepipelines/validate-preview-package-ordering.ps1 +++ b/.azurepipelines/validate-preview-package-ordering.ps1 @@ -41,8 +41,7 @@ function Get-NuGetOrgPublishedVersion { [Parameter(Mandatory)][string]$Uri ) - $response = Invoke-WebRequest -Uri $Uri -SkipHttpErrorCheck ` - -MaximumRetryCount 3 -RetryIntervalSec 5 + $response = Invoke-FeedRequest -Uri $Uri if ($response.StatusCode -eq 404) { # The package has never been published to nuget.org. return @() @@ -79,8 +78,7 @@ function Get-GitHubPackagesPublishedVersion { $maxPages = 100 while ($true) { $uri = "$ApiUrl/orgs/$Owner/packages/nuget/$PackageId/versions?per_page=100&page=$page" - $response = Invoke-WebRequest -Uri $uri -Headers $Headers -SkipHttpErrorCheck ` - -MaximumRetryCount 3 -RetryIntervalSec 5 + $response = Invoke-FeedRequest -Uri $uri -Headers $Headers if ($response.StatusCode -eq 404) { # The package has never been published to GitHub Packages. return @() diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index decfb1573f..ea39ddf4a8 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -464,7 +464,49 @@ jobs: } } elseif ($release -match 'HTTP 404') { - $arguments = @('release', 'create', $tag, '--target', $env:RELEASE_COMMIT, '--generate-notes') + # GitHub generates notes against the latest release when no + # previous tag is given. That is a 1.5.378 maintenance release + # on another branch, so the 2.0.0 notes listed every commit + # since the branches split and exceeded the 125000-character + # body limit (HTTP 422). Start from the greatest lower release + # of the same major (see Get-PreviousReleaseTag) and cap the + # body regardless. + . ./.azurepipelines/package-version-policy.ps1 + $releaseTags = @(gh api --paginate "repos/$env:GITHUB_REPOSITORY/releases?per_page=100" ` + --jq '.[] | select(.draft | not) | .tag_name') + if ($LASTEXITCODE -ne 0) { + throw 'Unable to list existing GitHub Releases.' + } + $previousTag = Get-PreviousReleaseTag -Tag $tag -ReleaseTags $releaseTags + + $notesRequest = @('--method', 'POST', "repos/$env:GITHUB_REPOSITORY/releases/generate-notes", + '-f', "tag_name=$tag", '-f', "target_commitish=$env:RELEASE_COMMIT") + if ($previousTag) { + $notesRequest += @('-f', "previous_tag_name=$previousTag") + } + $notes = gh api @notesRequest | ConvertFrom-Json + if ($LASTEXITCODE -ne 0) { + throw "Failed to generate release notes for '$tag'." + } + + $body = [string]$notes.body + $maxBodyLength = 120000 + if ($body.Length -gt $maxBodyLength) { + $compare = if ($previousTag) { + "https://github.com/$env:GITHUB_REPOSITORY/compare/$previousTag...$tag" + } + else { + "https://github.com/$env:GITHUB_REPOSITORY/commits/$tag" + } + $cut = $body.LastIndexOf("`n", $maxBodyLength) + $body = $body.Substring(0, [Math]::Max($cut, 0)) + + "`n`n_The generated notes were truncated to fit the GitHub Release size limit. Full changelog: $($compare)_`n" + } + $notesFile = Join-Path $env:RUNNER_TEMP 'release-notes.md' + Set-Content -LiteralPath $notesFile -Value $body -Encoding utf8NoBOM + + $arguments = @('release', 'create', $tag, '--target', $env:RELEASE_COMMIT, + '--title', [string]$notes.name, '--notes-file', $notesFile) if ($isPrerelease) { $arguments += '--prerelease' } diff --git a/tests/Opc.Ua.Tools.Tests/PackageVersionPolicyTests.cs b/tests/Opc.Ua.Tools.Tests/PackageVersionPolicyTests.cs index 7b3ccefc58..bc07705e15 100644 --- a/tests/Opc.Ua.Tools.Tests/PackageVersionPolicyTests.cs +++ b/tests/Opc.Ua.Tools.Tests/PackageVersionPolicyTests.cs @@ -758,8 +758,11 @@ namespace Opc.Ua.Tools.Tests // read the organization's packages" (403). Pin that contract: // silently treating a 403 as "nothing published" would let a // stale preview number reach an immutable feed. + // The policy script supplies Invoke-FeedRequest, which resolves + // the Invoke-WebRequest stub below at call time. JsonElement result = await RunPolicyScriptAsync( $$""" + . '{{PolicyScriptPath}}' $ast = [System.Management.Automation.Language.Parser]::ParseFile( '{{OrderingScriptPath}}', [ref]$null, [ref]$null) $fn = $ast.FindAll({ @@ -842,6 +845,149 @@ namespace Opc.Ua.Tools.Tests }); } + [TestCase(200, 1, Description = "Success is returned at once")] + [TestCase(404, 1, Description = "Not published is the normal answer, never retried")] + [TestCase(403, 1, Description = "Permission failures are not transient")] + [TestCase(400, 1, Description = "Other client errors are not transient")] + [TestCase(408, 4, Description = "Request timeout is retried")] + [TestCase(429, 4, Description = "Throttling is retried")] + [TestCase(500, 4, Description = "Server error is retried")] + [TestCase(503, 4, Description = "Unavailable is retried")] + public async Task InvokeFeedRequestRetriesOnlyTransientStatusesAsync(int status, int expectedRequests) + { + // Invoke-WebRequest -MaximumRetryCount retries every 4xx, which + // made each absent package in the release gates cost ~15 s. The + // gates must still get the final response back to fail closed. + JsonElement result = await RunPolicyScriptAsync( + $$""" + . '{{PolicyScriptPath}}' + $script:requests = 0 + function Invoke-WebRequest { + param([string]$Uri, [hashtable]$Headers, [switch]$SkipHttpErrorCheck) + $script:requests++ + return [pscustomobject]@{ StatusCode = {{status}} } + } + + $response = Invoke-FeedRequest -Uri 'https://stub/pkg' -MaximumRetryCount 3 -RetryIntervalSec 0 + @{ requests = $script:requests; status = [int]$response.StatusCode } | ConvertTo-Json + """).ConfigureAwait(false); + + Assert.Multiple(() => + { + Assert.That(result.GetProperty("requests").GetInt32(), Is.EqualTo(expectedRequests)); + Assert.That(result.GetProperty("status").GetInt32(), Is.EqualTo(status)); + }); + } + + [Test] + public async Task InvokeFeedRequestStopsRetryingOnceTheResponseIsNotTransientAsync() + { + JsonElement result = await RunPolicyScriptAsync( + $$""" + . '{{PolicyScriptPath}}' + $script:requests = 0 + function Invoke-WebRequest { + param([string]$Uri, [hashtable]$Headers, [switch]$SkipHttpErrorCheck) + $script:requests++ + $status = if ($script:requests -lt 3) { 503 } else { 404 } + return [pscustomobject]@{ StatusCode = $status } + } + + $response = Invoke-FeedRequest -Uri 'https://stub/pkg' -MaximumRetryCount 3 -RetryIntervalSec 0 + @{ requests = $script:requests; status = [int]$response.StatusCode } | ConvertTo-Json + """).ConfigureAwait(false); + + Assert.Multiple(() => + { + Assert.That(result.GetProperty("requests").GetInt32(), Is.EqualTo(3)); + Assert.That(result.GetProperty("status").GetInt32(), Is.EqualTo(404)); + }); + } + + [Test] + public async Task InvokeFeedRequestRethrowsNetworkFailureAfterTheLastRetryAsync() + { + JsonElement result = await RunPolicyScriptAsync( + $$""" + . '{{PolicyScriptPath}}' + $script:requests = 0 + function Invoke-WebRequest { + param([string]$Uri, [hashtable]$Headers, [switch]$SkipHttpErrorCheck) + $script:requests++ + throw [System.Net.Http.HttpRequestException]::new('connection refused') + } + + $threw = $false + $message = '' + try { + [void](Invoke-FeedRequest -Uri 'https://stub/pkg' -MaximumRetryCount 2 -RetryIntervalSec 0) + } + catch { + $threw = $true + $message = $_.Exception.Message + } + @{ requests = $script:requests; threw = $threw; message = $message } | ConvertTo-Json + """).ConfigureAwait(false); + + Assert.Multiple(() => + { + Assert.That(result.GetProperty("requests").GetInt32(), Is.EqualTo(3)); + Assert.That(result.GetProperty("threw").GetBoolean(), Is.True); + Assert.That(result.GetProperty("message").GetString(), Is.EqualTo("connection refused")); + }); + } + + [Test] + public async Task InvokeFeedRequestPassesOutFileThroughAsync() + { + JsonElement result = await RunPolicyScriptAsync( + $$""" + . '{{PolicyScriptPath}}' + function Invoke-WebRequest { + param([string]$Uri, [hashtable]$Headers, [switch]$SkipHttpErrorCheck, [string]$OutFile, [switch]$PassThru) + return [pscustomobject]@{ StatusCode = 200; OutFile = $OutFile; PassThru = [bool]$PassThru } + } + + $response = Invoke-FeedRequest -Uri 'https://stub/pkg' -OutFile 'pkg.nupkg' -RetryIntervalSec 0 + @{ outFile = $response.OutFile; passThru = $response.PassThru } | ConvertTo-Json + """).ConfigureAwait(false); + + Assert.Multiple(() => + { + Assert.That(result.GetProperty("outFile").GetString(), Is.EqualTo("pkg.nupkg")); + Assert.That( + result.GetProperty("passThru").GetBoolean(), + Is.True, + "Without -PassThru, -OutFile returns no response and the caller cannot read the status."); + }); + } + + [TestCase("2.0.0", "2.0.0-preview.10", Description = "Stable follows its greatest preview, not preview.2")] + [TestCase("2.0.1", "2.0.0", Description = "Servicing ignores the newer 2.1.0 minor line")] + [TestCase("2.1.0", "2.0.1", Description = "New minor follows the greatest lower release")] + [TestCase("2.0.0-preview.10", "2.0.0-preview.2", Description = "Preview numbers compare numerically")] + [TestCase("2.0.0-preview.2", null, Description = "No lower 2.x release: 1.5.378 is ignored")] + public async Task GetPreviousReleaseTagPicksGreatestLowerSameMajorVersionAsync( + string tag, + string? expected) + { + // Published out of version order on purpose: selection must not + // depend on publication time, and the four-component 1.5.378 + // maintenance tags must never become the baseline. + JsonElement result = await RunPolicyScriptAsync( + $$""" + . '{{PolicyScriptPath}}' + $tags = @('1.5.378.182', '2.1.0', '2.0.1', '2.0.0', '2.0.0-preview.10', + '2.0.0-preview.2', '1.5.378.176', 'not-a-version') + @{ actual = (Get-PreviousReleaseTag -Tag '{{tag}}' -ReleaseTags $tags) } | ConvertTo-Json + """).ConfigureAwait(false); + + JsonElement actual = result.GetProperty("actual"); + Assert.That( + actual.ValueKind == JsonValueKind.Null ? null : actual.GetString(), + Is.EqualTo(expected)); + } + [Test] public async Task GetNuGetPackageContentDigestIgnoresTheSignaturePartAsync() {