diff --git a/cmd/notation/internal/flag/options.go b/cmd/notation/internal/flag/options.go index 50b693eb9..858561c23 100644 --- a/cmd/notation/internal/flag/options.go +++ b/cmd/notation/internal/flag/options.go @@ -86,7 +86,7 @@ func withExecutableTrace(ctx context.Context) context.Context { }, ExecuteDone: func(executableName, action string, err error) { if err != nil { - logger.Errorf("finished executing credential helper program %s with action %s and got error %w", executableName, action, err) + logger.Errorf("finished executing credential helper program %s with action %s and got error %v", executableName, action, err) } else { logger.Debugf("successfully finished executing credential helper program %s with action %s", executableName, action) } diff --git a/cmd/notation/internal/flag/options_test.go b/cmd/notation/internal/flag/options_test.go index 6ffb2bcdf..e53023062 100644 --- a/cmd/notation/internal/flag/options_test.go +++ b/cmd/notation/internal/flag/options_test.go @@ -14,10 +14,18 @@ package flag import ( + "bytes" + "context" + "errors" "reflect" + "strings" "testing" + "github.com/sirupsen/logrus" "oras.land/oras-go/v2/registry/remote/auth" + credentialstrace "oras.land/oras-go/v2/registry/remote/credentials/trace" + + "github.com/notaryproject/notation-go/log" ) func TestSecureFlagOpts_Credential(t *testing.T) { @@ -72,3 +80,21 @@ func TestSecureFlagOpts_Credential(t *testing.T) { }) } } + +func TestWithExecutableTrace_ExecuteDoneLogsError(t *testing.T) { + var out bytes.Buffer + logger := logrus.New() + logger.SetOutput(&out) + logger.SetLevel(logrus.DebugLevel) + + ctx := withExecutableTrace(log.WithLogger(context.Background(), logger)) + credentialstrace.ContextExecutableTrace(ctx).ExecuteDone("docker-credential-test", "get", errors.New("boom")) + + got := out.String() + if !strings.Contains(got, "and got error boom") { + t.Errorf("the logged error is missing the error message, got %q", got) + } + if strings.Contains(got, "%!") { + t.Errorf("the logged error contains a formatting error, got %q", got) + } +} diff --git a/internal/trace/transport.go b/internal/trace/transport.go index 76ca9c751..1c0f15dc4 100644 --- a/internal/trace/transport.go +++ b/internal/trace/transport.go @@ -75,7 +75,7 @@ func (t *Transport) RoundTrip(req *http.Request) (resp *http.Response, err error // log the response resp, err = t.RoundTripper.RoundTrip(req) if err != nil { - e.Errorf("Error in getting response: %w", err) + e.Errorf("Error in getting response: %v", err) } else if resp == nil { e.Errorf("No response obtained for request %s %q", req.Method, req.URL) } else { diff --git a/internal/trace/transport_test.go b/internal/trace/transport_test.go index 58e6dfbc2..b65fdeb74 100644 --- a/internal/trace/transport_test.go +++ b/internal/trace/transport_test.go @@ -30,9 +30,16 @@ limitations under the License. package trace import ( + "bytes" + "context" "errors" "net/http" + "strings" "testing" + + "github.com/sirupsen/logrus" + + "github.com/notaryproject/notation-go/log" ) type TransportMock struct { @@ -87,3 +94,31 @@ func TestTransport_RoundTrip(t *testing.T) { } }) } + +func TestTransport_RoundTrip_LogsResponseError(t *testing.T) { + var out bytes.Buffer + logger := logrus.New() + logger.SetOutput(&out) + logger.SetLevel(logrus.DebugLevel) + + ctx := log.WithLogger(context.Background(), logger) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, "https://example.com", nil) + if err != nil { + t.Fatal(err) + } + transport := NewTransport(&TransportMock{respErr: errors.New("boom")}) + resp, err := transport.RoundTrip(req) + if resp != nil { + resp.Body.Close() + } + if err == nil { + t.Fatal("expected an error") + } + + if got := out.String(); !strings.Contains(got, "Error in getting response: boom") { + t.Errorf("the logged error is missing the error message, got %q", got) + } + if got := out.String(); strings.Contains(got, "%!") { + t.Errorf("the logged error contains a formatting error, got %q", got) + } +}