Skip to content

Commit

Permalink
Improve transport.Error text (#827)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jonjohnsonjr committed Nov 12, 2020
1 parent 5749037 commit dd33eb2
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 10 deletions.
Empty file added 1
Empty file.
3 changes: 3 additions & 0 deletions pkg/v1/remote/descriptor.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,9 @@ func Get(ref name.Reference, options ...Option) (*Descriptor, error) {

// Head returns a v1.Descriptor for the given reference by issuing a HEAD
// request.
//
// Note that the server response will not have a body, so any errors encountered
// should be retried with Get to get more details.
func Head(ref name.Reference, options ...Option) (*v1.Descriptor, error) {
acceptable := []types.MediaType{
// Just to look at them.
Expand Down
7 changes: 5 additions & 2 deletions pkg/v1/remote/transport/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,12 @@ func (e *Error) responseErr() string {
switch len(e.Errors) {
case 0:
if len(e.rawBody) == 0 {
return fmt.Sprintf("unsupported status code %d", e.StatusCode)
if e.request != nil && e.request.Method == http.MethodHead {
return fmt.Sprintf("unexpected status code %d %s (HEAD responses have no body, use GET for details)", e.StatusCode, http.StatusText(e.StatusCode))
}
return fmt.Sprintf("unexpected status code %d %s", e.StatusCode, http.StatusText(e.StatusCode))
}
return fmt.Sprintf("unsupported status code %d; body: %s", e.StatusCode, e.rawBody)
return fmt.Sprintf("unexpected status code %d %s: %s", e.StatusCode, http.StatusText(e.StatusCode), e.rawBody)
case 1:
return e.Errors[0].String()
default:
Expand Down
20 changes: 16 additions & 4 deletions pkg/v1/remote/transport/error_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,16 +95,16 @@ func TestCheckErrorNotError(t *testing.T) {
}{{
code: http.StatusBadRequest,
body: "",
msg: "unsupported status code 400",
msg: "unexpected status code 400 Bad Request",
}, {
code: http.StatusUnauthorized,
// Valid JSON, but not a structured error -- we should still print the body.
body: `{"details":"incorrect username or password"}`,
msg: `unsupported status code 401; body: {"details":"incorrect username or password"}`,
msg: `unexpected status code 401 Unauthorized: {"details":"incorrect username or password"}`,
}, {
code: http.StatusUnauthorized,
body: "Not JSON",
msg: "GET https://example.com/somepath?access_token=REDACTED&scope=foo&service=bar: unsupported status code 401; body: Not JSON",
msg: "GET https://example.com/somepath?access_token=REDACTED&scope=foo&service=bar: unexpected status code 401 Unauthorized: Not JSON",
request: &http.Request{
Method: http.MethodGet,
URL: &url.URL{
Expand All @@ -118,6 +118,18 @@ func TestCheckErrorNotError(t *testing.T) {
}.Encode(),
},
},
}, {
code: http.StatusUnauthorized,
body: "",
msg: "HEAD https://example.com/somepath: unexpected status code 401 Unauthorized (HEAD responses have no body, use GET for details)",
request: &http.Request{
Method: http.MethodHead,
URL: &url.URL{
Scheme: "https",
Host: "example.com",
Path: "somepath",
},
},
}}

for _, test := range tests {
Expand Down Expand Up @@ -166,7 +178,7 @@ func TestCheckErrorWithError(t *testing.T) {
error: &Error{
StatusCode: 400,
},
msg: "unsupported status code 400",
msg: "unexpected status code 400 Bad Request",
}, {
code: http.StatusBadRequest,
error: &Error{
Expand Down
7 changes: 3 additions & 4 deletions pkg/v1/remote/transport/logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,16 +36,15 @@ func NewLogger(inner http.RoundTripper) http.RoundTripper {

func (t *logTransport) RoundTrip(in *http.Request) (out *http.Response, err error) {
// Inspired by: github.com/motemen/go-loghttp
msg := fmt.Sprintf("--> %s %s", in.Method, in.URL)

// We redact token responses and binary blobs in response/request.
omitBody, reason := redact.FromContext(in.Context())
if omitBody {
msg = fmt.Sprintf("%s [body redacted: %s]", msg, reason)
logs.Debug.Printf("--> %s %s [body redacted: %s]", in.Method, in.URL, reason)
} else {
logs.Debug.Printf("--> %s %s", in.Method, in.URL)
}

logs.Debug.Printf(msg)

// Save these headers so we can redact Authorization.
savedHeaders := in.Header.Clone()
if in.Header != nil {
Expand Down

0 comments on commit dd33eb2

Please sign in to comment.