Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve transport.Error text #827

Merged
merged 1 commit into from
Nov 12, 2020
Merged

Conversation

jonjohnsonjr
Copy link
Collaborator

@jonjohnsonjr jonjohnsonjr commented Nov 12, 2020

Fixes #825

Return "unexpected status code" instead of "unsupported".

Instead of just printing the status code, include the http.StatusText.

While debugging this, I encountered more MISSING stuff in the debug logs
for URLs. Fix that.

Document that remote.Head errors can be retried with remote.Get to get more details.

@codecov-io
Copy link

codecov-io commented Nov 12, 2020

Codecov Report

Merging #827 (ffe4c1e) into master (ef0e499) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #827      +/-   ##
==========================================
+ Coverage   74.63%   74.64%   +0.01%     
==========================================
  Files         103      103              
  Lines        4340     4342       +2     
==========================================
+ Hits         3239     3241       +2     
  Misses        619      619              
  Partials      482      482              
Impacted Files Coverage Δ
pkg/v1/remote/descriptor.go 74.39% <ø> (ø)
pkg/v1/remote/transport/error.go 100.00% <100.00%> (ø)
pkg/v1/remote/transport/logger.go 88.23% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ef0e499...ffe4c1e. Read the comment docs.

jonjohnsonjr added a commit to jonjohnsonjr/rules_docker that referenced this pull request Nov 12, 2020
jonjohnsonjr added a commit to jonjohnsonjr/rules_docker that referenced this pull request Nov 12, 2020
Return "unexpected status code" instead of "unsupported".

Instead of just printing the status code, include the http.StatusText.

While debugging this, I encountered more MISSING stuff in the debug logs
for URLs. Fix that.

Add note to remote.Head about errors.
Copy link
Contributor

@hasheddan hasheddan left a comment

Choose a reason for hiding this comment

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

This will be nice to have 👍

@jonjohnsonjr jonjohnsonjr merged commit dd33eb2 into google:master Nov 12, 2020
@jonjohnsonjr jonjohnsonjr deleted the error-text branch November 12, 2020 23:09
smukherj1 pushed a commit to bazelbuild/rules_docker that referenced this pull request Nov 13, 2020
@dprotaso
Copy link
Contributor

@jonjohnsonjr what's the empty 1 file for?

@imjasonh
Copy link
Collaborator

@jonjohnsonjr what's the empty 1 file for?

🤫

@jonjohnsonjr
Copy link
Collaborator Author

My inability to successfully redirect stderr to stdout the first try, and my arrogance to just "git commit -a --amend" without looking.

@jonjohnsonjr
Copy link
Collaborator Author

PRs welcome.

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.

Unhelpful error message when 403 is returned from the registry
5 participants