Retry transient download errors

[kichristensen]

What does this change

downloadFile (used for the mixin/plugin atom.xml feed and binary
downloads) already retried some transient errors, but its allowlist
only matched io.EOF/io.ErrUnexpectedEOF/"TLS handshake timeout".
It did not match "connection reset by peer", which is what actually
fails in CI, and any non-2xx status returned immediately with zero
retries.

• Added pkg/pkgmgmt/retry.go: IsRetryableError (adds ECONNRESET,
  ECONNREFUSED, ETIMEDOUT, net.Error timeouts) and IsRetryableStatus
  (429/5xx).
• downloadFile (pkg/pkgmgmt/client/install.go) now uses these and
  retries on transient statuses instead of failing immediately.
• GetPackageListings (pkg/pkgmgmt/search.go) gets the same treatment
  for consistency — same bare-http.Get-no-retry bug, used by
  mixin search/plugin search.

No new dependency; widened the existing hand-rolled retry loop instead
of adopting a retry library.

Follow-up commit, addressing Copilot review + a CI failure on this PR:

• install.go/search.go: close resp.Body when Do/Get returns a
  non-nil response alongside an error, drain the body before closing on
  non-2xx (connection reuse), and bind the download request to ctx
  via NewRequestWithContext.
• magefile.go's GetMixins now retries porter mixin install up to
  3x with a 5s backoff. CI hit a 404 for terraform-linux-amd64 caused
  by a race with the terraform-mixin's own canary release publish (the
  CDN asset's last-modified matched the failure timestamp exactly).
  Kept IsRetryableStatus fail-fast on 404 (existing tests rely on
  this for missing versions/typos); scoped the retry to the CI
  bootstrap step where the race actually happens instead.

What issue does it fix

Closes #3678

Notes for the reviewer

• Non-retryable statuses (404, 400, etc.) still fail immediately,
  preserving existing test expectations.
• search.go's GetPackageListings fix is same bug class but not
  what Flaky CI: mixin download from cdn.porter.sh fails intermittently in integration test build #3678's CI failure actually hits — included for consistency,
  happy to split out if preferred.

Checklist

• Did you write tests?
• Did you write documentation?
• Did you change porter.yaml or a storage document record? Update the corresponding schema file.
• If this is your first pull request, please add your name to the bottom of our Contributors list. Thank you for making Porter better! 🙇‍♀️

[Copilot]

🟡 Changes recommended

The new retry paths can leak HTTP response bodies on error and don’t consistently drain/close bodies (and bind requests to context), which can cause resource issues and flaky behavior.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR improves network resilience in Porter’s package management layer by adding shared retry helpers and applying them to mixin/plugin feed and binary downloads, addressing flaky CI failures caused by transient network issues.

Changes:

• Added shared retry classification helpers for transient network errors and transient HTTP statuses (429/5xx).
• Updated downloadFile to retry on retryable network errors and transient HTTP statuses instead of failing immediately.
• Updated GetPackageListings to retry on the same transient conditions and added tests covering retry behavior.

File summaries

File	Description
pkg/pkgmgmt/search.go	Adds retry/backoff to package listing fetches and uses shared retry predicates.
pkg/pkgmgmt/search_test.go	Adds tests validating retry-on-503 and give-up behavior for listings.
pkg/pkgmgmt/retry.go	Introduces shared IsRetryableError / IsRetryableStatus helpers.
pkg/pkgmgmt/retry_test.go	Adds unit tests for the shared retry helpers.
pkg/pkgmgmt/client/install.go	Uses shared retry predicates and retries on transient HTTP statuses during downloads.
pkg/pkgmgmt/client/install_test.go	Adds tests validating retry-on-503 and give-up behavior for downloads.

Review details

Suppressed comments (2)

pkg/pkgmgmt/search.go:75

• When retrying on a non-200 response, closing the body without draining it prevents HTTP connection reuse. Draining the body before closing helps avoid extra TCP handshakes during retries and in subsequent calls.

		if resp.StatusCode != http.StatusOK {
			statusCode := resp.StatusCode
			resp.Body.Close()
			resp = nil

pkg/pkgmgmt/client/install.go:222

• On non-200 responses that are retried, the body is closed without being drained, which prevents connection reuse and can add avoidable latency/handshakes during retries.

		if resp.StatusCode != 200 {
			statusCode := resp.StatusCode
			statusErr := fmt.Errorf("bad status returned when downloading %s (%d) %s", url.String(), resp.StatusCode, resp.Status)
			resp.Body.Close()
			resp = nil

• Files reviewed: 6/6 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.