From eb5958869770b1e8db0e76f477349f88484b1462 Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Sun, 19 Jul 2026 12:46:01 +0100 Subject: [PATCH] fix: stop retrying artifact upload on non-retryable status codes On netstandard2.0 HttpRequestException carries no StatusCode, so IsRetryable fell back to a message heuristic that only excluded 401/403. A GitHub Enterprise Server 404 (the Actions Results artifact API does not exist there) therefore burned the full 5-attempt backoff schedule, adding roughly 25-30s to every run. Introduce ArtifactUploadException carrying the status code on every target framework, so retry decisions are code-driven rather than string-driven. IsRetryable is now an allowlist of transient codes (408, 429, 500, 502, 503, 504); everything else fails on the first attempt. 404 responses gain a hint that the artifact API is unavailable on this host. Non-retryable failures still propagate out of UploadAsync so the CreateArtifact name-dedup loop does not re-issue the same doomed request three times. --- .../Reporters/Html/GitHubArtifactUploader.cs | 47 ++++++++++--------- .../GitHubArtifactUploaderTests.cs | 31 ++++++++++++ 2 files changed, 55 insertions(+), 23 deletions(-) diff --git a/src/TUnit.Engine/Reporters/Html/GitHubArtifactUploader.cs b/src/TUnit.Engine/Reporters/Html/GitHubArtifactUploader.cs index 58ee705c43..d27a307372 100644 --- a/src/TUnit.Engine/Reporters/Html/GitHubArtifactUploader.cs +++ b/src/TUnit.Engine/Reporters/Html/GitHubArtifactUploader.cs @@ -14,7 +14,7 @@ internal static class GitHubArtifactUploader private const int BaseRetryMs = 3000; private const double RetryMultiplier = 1.5; - private static readonly HashSet RetryableStatusCodes = [429, 500, 502, 503, 504]; + private static readonly HashSet RetryableStatusCodes = [408, 429, 500, 502, 503, 504]; private static readonly HttpClient SharedHttpClient = new() { Timeout = TimeSpan.FromSeconds(30) }; internal static async Task UploadAsync( @@ -280,7 +280,7 @@ private static (string? RunId, string? JobId) ExtractBackendIds(string jwt) { return await action(); } - catch (HttpRequestException ex) when (IsRetryable(ex)) + catch (ArtifactUploadException ex) when (IsRetryable(ex.StatusCode)) { if (attempt == MaxRetries - 1) { @@ -312,29 +312,30 @@ private static async Task EnsureSuccessAsync(HttpResponseMessage response, strin // Include response body in the exception message rather than logging per-attempt, // so only the final failure is surfaced by RetryAsync. -#if NET - throw new HttpRequestException($"{step} returned {(int)response.StatusCode}: {body}", null, response.StatusCode); -#else - throw new HttpRequestException($"{step} returned {(int)response.StatusCode}: {body}"); -#endif + var statusCode = (int)response.StatusCode; + var hint = statusCode is 404 + // GitHub Enterprise Server / other non-github.com hosts don't implement the + // Actions Results artifact API, so retrying can only burn wall-clock time. + ? " (the artifact API is unavailable on this host — this is expected on GitHub Enterprise Server)" + : ""; + + throw new ArtifactUploadException($"{step} returned {statusCode}{hint}: {body}", statusCode); } - private static bool IsRetryable(HttpRequestException ex) - { -#if NET - var statusCode = (int?)ex.StatusCode; - return statusCode is not null && RetryableStatusCodes.Contains(statusCode.Value); -#else - // On netstandard2.0, HttpRequestException doesn't have StatusCode. - // Use message heuristic to avoid retrying non-retryable errors like 401/403. - var msg = ex.Message; - if (msg.Contains("401") || msg.Contains("403") || - msg.Contains("Unauthorized") || msg.Contains("Forbidden")) - { - return false; - } + /// + /// Only transient failures are worth retrying. Anything else — an unavailable artifact API (404), + /// a bad token (401/403), a malformed request (400) — fails identically on every attempt, so + /// retrying just adds the full backoff schedule to the run time before the same error surfaces. + /// + internal static bool IsRetryable(int? statusCode) => + statusCode is not null && RetryableStatusCodes.Contains(statusCode.Value); - return true; -#endif + /// + /// Carries the HTTP status code across all target frameworks — + /// only exposes StatusCode on .NET 5+. + /// + private sealed class ArtifactUploadException(string message, int statusCode) : Exception(message) + { + public int StatusCode { get; } = statusCode; } } diff --git a/tests/TUnit.Engine.Tests/GitHubArtifactUploaderTests.cs b/tests/TUnit.Engine.Tests/GitHubArtifactUploaderTests.cs index 3944a4c505..12872c584f 100644 --- a/tests/TUnit.Engine.Tests/GitHubArtifactUploaderTests.cs +++ b/tests/TUnit.Engine.Tests/GitHubArtifactUploaderTests.cs @@ -46,6 +46,37 @@ public void ComputeExpiresAt_Does_Not_Clamp_When_Within_Repository_Maximum() result.ShouldBe("2026-06-24T12:00:00Z"); } + [Test] + [Arguments(408)] + [Arguments(429)] + [Arguments(500)] + [Arguments(502)] + [Arguments(503)] + [Arguments(504)] + public void IsRetryable_Is_True_For_Transient_Status_Codes(int statusCode) + { + GitHubArtifactUploader.IsRetryable(statusCode).ShouldBeTrue(); + } + + [Test] + [Arguments(400)] + [Arguments(401)] + [Arguments(403)] + // 404 is what GitHub Enterprise Server returns — the artifact API doesn't exist there + [Arguments(404)] + [Arguments(422)] + [Arguments(501)] + public void IsRetryable_Is_False_For_Non_Retryable_Status_Codes(int statusCode) + { + GitHubArtifactUploader.IsRetryable(statusCode).ShouldBeFalse(); + } + + [Test] + public void IsRetryable_Is_False_When_No_Status_Code() + { + GitHubArtifactUploader.IsRetryable(null).ShouldBeFalse(); + } + [Test] public void ComputeExpiresAt_Ignores_NonPositive_Repository_Maximum() {