Skip to content

transport: restore resp.Body in retryError so CheckError can parse it#2264

Merged
Subserial merged 2 commits into
google:mainfrom
alliasgher:fix-retry-error-restores-body
Apr 16, 2026
Merged

transport: restore resp.Body in retryError so CheckError can parse it#2264
Subserial merged 2 commits into
google:mainfrom
alliasgher:fix-retry-error-restores-body

Conversation

@alliasgher

Copy link
Copy Markdown
Contributor

Description

Fixes #2125.

retryError consumed resp.Body with io.ReadAll but never restored it. When the retry loop exhausted its attempts and fell through to CheckError, that function tried to read the same body and got nothing — producing a generic "unexpected status code 404" instead of the structured registry error (MANIFEST_UNKNOWN: manifest unknown; ...).

Fix

After reading the body, restore it with io.NopCloser(bytes.NewReader(b)) so subsequent callers see the same bytes. This is a one-line addition matching the same pattern used elsewhere in the codebase.

Before

With 404 retry:
GET …/manifests/0.0.46-bad-version: unexpected status code 404 Not Found

After

With 404 retry:
GET …/manifests/0.0.46-bad-version: MANIFEST_UNKNOWN: manifest unknown; unknown tag=0.0.46-bad-version

Testing

Added TestRetryErrorRestoresBody: calls retryError then CheckError on the same response and asserts the structured MANIFEST_UNKNOWN error comes through. All existing tests pass.

@codecov-commenter

codecov-commenter commented Apr 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 52.93%. Comparing base (8b3c303) to head (b69fc3e).
⚠️ Report is 91 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (8b3c303) and HEAD (b69fc3e). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (8b3c303) HEAD (b69fc3e)
2 1
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #2264       +/-   ##
===========================================
- Coverage   71.67%   52.93%   -18.75%     
===========================================
  Files         123      165       +42     
  Lines        9935    11208     +1273     
===========================================
- Hits         7121     5933     -1188     
- Misses       2115     4561     +2446     
- Partials      699      714       +15     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

retryError consumed resp.Body with io.ReadAll but discarded the bytes,
leaving the body closed. When the retry loop exhausted its attempts and
called CheckError, that function tried to read the same body and found
nothing, producing a generic "unexpected status code" error instead of
the structured registry error (e.g. MANIFEST_UNKNOWN).

After reading the body, restore it with io.NopCloser(bytes.NewReader(b))
so subsequent callers see the same bytes.

Fixes google#2125

Signed-off-by: alliasgher <alliasgher123@gmail.com>
Signed-off-by: alliasgher <alliasgher123@gmail.com>
@Subserial Subserial force-pushed the fix-retry-error-restores-body branch from 4223d14 to b69fc3e Compare April 16, 2026 18:16
@Subserial Subserial merged commit 1bdf519 into google:main Apr 16, 2026
17 checks passed
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.

ggcr: retryError consumes HTTP response body

3 participants