Skip to content

Retry intermediary 403s in artifact-twirp-client#2452

Open
npwolf wants to merge 1 commit into
actions:mainfrom
npwolf:fix/retry-intermediary-403-artifact-client
Open

Retry intermediary 403s in artifact-twirp-client#2452
npwolf wants to merge 1 commit into
actions:mainfrom
npwolf:fix/retry-intermediary-403-artifact-client

Conversation

@npwolf

@npwolf npwolf commented Jul 17, 2026

Copy link
Copy Markdown

Problem

CI intermittently fails an actions/download-artifact@v8 step with:

Downloading single artifact
##[error]Unable to download artifact(s): Failed to ListArtifacts: Received non-retryable error: Failed request: (403) Forbidden: Error from intermediary with HTTP status code 403 "Forbidden"

This happened downloading an artifact that had been uploaded moments earlier in the same workflow run by a sibling job (a Rails CI setup with several parallel matrix shards downloading the same precompiled-assets artifact). Two other parallel shards in the same run downloaded the identical artifact successfully, so the artifact existed and wasn't expired — this was a transient failure on the artifact service's edge/proxy layer, surfaced with no retry.

This matches the currently open, unaddressed report in actions/download-artifact#464 ("intermittent 403s with no retry" — 6/96 parallel jobs failing this way in one run, one comment speculating it's "some sort of throttle perhaps getting incorrectly returned as a 403"). Per this repo's CONTRIBUTING.md and download-artifact's own contributing guide, the retry logic actually lives here in @actions/artifact, not in download-artifact.

Root cause

ArtifactHttpClient.isRetryableHttpStatusCode() in packages/artifact/src/internal/shared/artifact-twirp-client.ts only retries 502/504/500/503/429. A 403 always falls into the non-retryable branch of retryableRequest() and throws immediately (Received non-retryable error: ...), matching the error above exactly.

Why not just add 403 to the retryable list

A 403 from the Results Service can mean two very different things:

  1. A transient edge/proxy hiccup — what "Error from intermediary with HTTP status code 403" explicitly says: the 403 came from an intermediary in front of the backend, not from the backend's own authorization logic.
  2. A genuine, permanent authorization failure (expired/invalid runtime token, artifact out of scope, etc.).

Blindly adding HttpCodes.Forbidden to retryableStatusCodes would retry all 403s for up to maxAttempts (5) with exponential backoff, including genuine permanent failures — masking real auth bugs behind ~30-60s of pointless retrying before still failing.

Fix

isRetryableHttpStatusCode now accepts the constructed error message and retries a 403 only when that message identifies it as coming from an intermediary (/error from intermediary/i). Every other 403 — including the existing UsageError case, which is checked earlier and unaffected — continues to fail fast exactly as before. retryableRequest was reordered slightly so the message (which already includes statusMessage and body.msg) is fully built before the retryability decision is made.

Testing

Added two tests to packages/artifact/__tests__/artifact-http-client.test.ts, following the existing mock/assert pattern in that file:

  • an intermediary-flavored 403 followed by a success retries and returns the artifact (2 post calls)
  • a plain 403 with no intermediary signal still fails immediately on the first attempt (1 post call), matching today's behavior

Confirmed the new "retries" test fails with today's code (reproducing the exact Received non-retryable error: ...: Error from intermediary... message) and passes with the fix. Confirmed the existing 403 → UsageError test is unaffected.

Ran npm run bootstrap, npm run build, npm test -- packages/artifact (133/133 passing), npm run format-check, and npm run lint — all clean.

Scope

Deliberately does not touch packages/artifact/src/internal/find/retry-options.ts, the separate octokit-based retry path used for the public/cross-repo API (listArtifactsPublic), which explicitly exempts 403 for good reason in that context (different threat model, different callers).

Also relevant: actions/download-artifact#339, an older, messier thread with several different root causes tangled together — some resolved by fixing path usage, some by pinning versions — but with multiple commenters reporting the same general category of intermittent, non-path-related download failures.

Happy to adjust the message-matching approach if maintainers have a more reliable signal for "this 403 came from the intermediary layer" (e.g., a header) than string-matching the body.

The artifact Results Service's internal Twirp client treats every HTTP
403 as non-retryable, on the assumption it always reflects a genuine
authorization failure. In practice its edge/proxy layer intermittently
returns its own 403 ("Error from intermediary with HTTP status code
403"), which is a transient infrastructure hiccup rather than an auth
decision - the same artifact download succeeds moments later from a
parallel job in the same run.

isRetryableHttpStatusCode now also retries a 403 when the error message
identifies it as coming from an intermediary, while leaving all other
403s (real backend authorization failures) failing fast as before.
Fixes the failure mode reported in actions/download-artifact#464.
Copilot AI review requested due to automatic review settings July 17, 2026 18:01
@npwolf
npwolf requested a review from a team as a code owner July 17, 2026 18:01

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves the resilience of the internal artifact Twirp HTTP client by adding a targeted retry for a specific transient failure mode: intermediary/proxy-generated HTTP 403 responses that can occur intermittently during artifact operations in CI.

Changes:

  • Reorders error construction in retryableRequest() so the full error message is available when deciding retryability.
  • Extends retry logic to treat HTTP 403 as retryable only when the error message indicates it came “from an intermediary”.
  • Adds unit tests covering both the retryable intermediary-403 case and the fail-fast non-intermediary 403 case.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
packages/artifact/src/internal/shared/artifact-twirp-client.ts Builds the full error message before computing retryability and conditionally retries intermediary-style 403s.
packages/artifact/tests/artifact-http-client.test.ts Adds tests ensuring intermediary 403s retry and non-intermediary 403s remain non-retryable.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants