From 465f3ff28b2a940a1d194dd30221c9489157e48e Mon Sep 17 00:00:00 2001 From: Noah Stride Date: Thu, 9 Nov 2023 19:20:22 +0000 Subject: [PATCH 1/4] Use existing error formatting logic from `trace` --- lib/utils/cli.go | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/lib/utils/cli.go b/lib/utils/cli.go index 8735c10a7554c..aab188a6c2865 100644 --- a/lib/utils/cli.go +++ b/lib/utils/cli.go @@ -203,22 +203,15 @@ func formatErrorWriter(err error, w io.Writer) { fmt.Fprintln(w, certErr) return } - // If the error is a trace error, check if it has a user message embedded in - // it, if it does, print it, otherwise escape and print the original error. - if traceErr, ok := err.(*trace.TraceErr); ok { - for _, message := range traceErr.Messages { - fmt.Fprintln(w, AllowWhitespace(message)) - } - fmt.Fprintln(w, AllowWhitespace(trace.Unwrap(traceErr).Error())) - return - } - strErr := err.Error() + + msg := trace.UserMessage(err) // Error can be of type trace.proxyError where error message didn't get captured. - if strErr == "" { + if msg == "" { fmt.Fprintln(w, "please check Teleport's log for more details") - } else { - fmt.Fprintln(w, AllowWhitespace(err.Error())) + return } + + fmt.Fprintln(w, AllowWhitespace(err.Error())) } func formatCertError(err error) string { From 55fc865f553aeb37c75ec8d54258b88536771dde Mon Sep 17 00:00:00 2001 From: Noah Stride Date: Thu, 9 Nov 2023 19:52:43 +0000 Subject: [PATCH 2/4] Remove unnecessary layer of indirection --- lib/utils/cli.go | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/lib/utils/cli.go b/lib/utils/cli.go index aab188a6c2865..c7ed8bb77e6a9 100644 --- a/lib/utils/cli.go +++ b/lib/utils/cli.go @@ -178,21 +178,15 @@ func UserMessageFromError(err error) string { // The error message is escaped if necessary. A newline is added if the error text // does not end with a newline. func FormatErrorWithNewline(err error) string { - message := formatError(err) + var buf bytes.Buffer + formatErrorWriter(err, &buf) + message := buf.String() if !strings.HasSuffix(message, "\n") { message = message + "\n" } return message } -// formatError returns user friendly error message from error. -// The error message is escaped if necessary -func formatError(err error) string { - var buf bytes.Buffer - formatErrorWriter(err, &buf) - return buf.String() -} - // formatErrorWriter formats the specified error into the provided writer. // The error message is escaped if necessary func formatErrorWriter(err error, w io.Writer) { @@ -211,7 +205,7 @@ func formatErrorWriter(err error, w io.Writer) { return } - fmt.Fprintln(w, AllowWhitespace(err.Error())) + fmt.Fprintln(w, AllowWhitespace(msg)) } func formatCertError(err error) string { From 6fa5715c16d4b49e293a4938ab655b8628193036 Mon Sep 17 00:00:00 2001 From: Noah Stride Date: Mon, 20 Nov 2023 11:13:35 +0000 Subject: [PATCH 3/4] Add test for UserMessageFromError --- lib/utils/cli_test.go | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/lib/utils/cli_test.go b/lib/utils/cli_test.go index 69fa3528c11b6..b74fbae074b91 100644 --- a/lib/utils/cli_test.go +++ b/lib/utils/cli_test.go @@ -19,7 +19,7 @@ package utils import ( "crypto/x509" "fmt" - "strings" + "github.com/sirupsen/logrus" "testing" "github.com/gravitational/trace" @@ -27,9 +27,13 @@ import ( ) func TestUserMessageFromError(t *testing.T) { - t.Parallel() + // Behaviour is different in debug + priorLevel := logrus.GetLevel() + logrus.SetLevel(logrus.InfoLevel) + t.Cleanup(func() { + logrus.SetLevel(priorLevel) + }) - t.Skip("Enable after https://drone.gravitational.io/gravitational/teleport/3517 is merged.") tests := []struct { comment string inError error @@ -47,14 +51,14 @@ func TestUserMessageFromError(t *testing.T) { }, { comment: "outputs user message as provided", - inError: trace.Errorf("\x1b[1mWARNING\x1b[0m"), - outString: `error: "\x1b[1mWARNING\x1b[0m"`, + inError: trace.Errorf("bad thing occurred"), + outString: "\x1b[31mERROR: \x1b[0mbad thing occurred", }, } for _, tt := range tests { message := UserMessageFromError(tt.inError) - require.True(t, strings.HasPrefix(message, tt.outString), tt.comment) + require.Contains(t, message, tt.outString) } } From 15118f0aaf3140f9c24cd9d1d9394295e6b60488 Mon Sep 17 00:00:00 2001 From: Noah Stride Date: Mon, 20 Nov 2023 11:23:11 +0000 Subject: [PATCH 4/4] Fix imports --- lib/utils/cli_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/utils/cli_test.go b/lib/utils/cli_test.go index b74fbae074b91..7bc2fa212723f 100644 --- a/lib/utils/cli_test.go +++ b/lib/utils/cli_test.go @@ -19,15 +19,15 @@ package utils import ( "crypto/x509" "fmt" - "github.com/sirupsen/logrus" "testing" "github.com/gravitational/trace" + "github.com/sirupsen/logrus" "github.com/stretchr/testify/require" ) func TestUserMessageFromError(t *testing.T) { - // Behaviour is different in debug + // Behavior is different in debug priorLevel := logrus.GetLevel() logrus.SetLevel(logrus.InfoLevel) t.Cleanup(func() {