From 74ca5bbfdd2bfec9afe1f31da6f0e104322f35c7 Mon Sep 17 00:00:00 2001 From: Brian Joerger Date: Tue, 7 Apr 2026 12:50:30 -0700 Subject: [PATCH] Remove extra new line in client error messages (#65206) * Fix formatErrorWriter adding duplicate new lines. * Address comments. * Address claude comment. --- lib/utils/cli.go | 36 ++++++++++++++++++++---------------- tool/tsh/common/tsh.go | 8 ++++---- 2 files changed, 24 insertions(+), 20 deletions(-) diff --git a/lib/utils/cli.go b/lib/utils/cli.go index 02f72d34917..0945c7f4266 100644 --- a/lib/utils/cli.go +++ b/lib/utils/cli.go @@ -137,7 +137,7 @@ func InitLogger(purpose LoggingPurpose, level slog.Level, opts ...LoggerOption) // FatalError is for CLI front-ends: it detects gravitational/trace debugging // information, sends it to the logger, strips it off and prints a clean message to stderr func FatalError(err error) { - fmt.Fprintln(os.Stderr, UserMessageFromError(err)) + fmt.Fprint(os.Stderr, UserMessageFromError(err)) os.Exit(1) } @@ -158,13 +158,17 @@ func GetIterations() int { // UserMessageFromError returns user-friendly error message from error. // The error message will be formatted for output depending on the debug -// flag +// flag and will always end with a new line. func UserMessageFromError(err error) string { if err == nil { return "" } if slog.Default().Enabled(context.Background(), slog.LevelDebug) { - return trace.DebugReport(err) + msg := trace.DebugReport(err) + if !strings.HasSuffix(msg, "\n") { + msg += "\n" + } + return msg } var buf bytes.Buffer if runtime.GOOS == constants.WindowsOS { @@ -181,37 +185,37 @@ func UserMessageFromError(err error) string { } // FormatErrorWithNewline returns user friendly error message from error. -// The error message is escaped if necessary. A newline is added if the error text -// does not end with a newline. +// The message will always end with a new line. func FormatErrorWithNewline(err error) string { var buf bytes.Buffer formatErrorWriter(err, &buf) - message := buf.String() - if !strings.HasSuffix(message, "\n") { - message = message + "\n" - } - return message + return buf.String() } // formatErrorWriter formats the specified error into the provided writer. -// The error message is escaped if necessary +// The error message is escaped if necessary. A newline is added if the +// error text does not end with a newline. func formatErrorWriter(err error, w io.Writer) { if err == nil { return } - if certErr := formatCertError(err); certErr != "" { - fmt.Fprintln(w, certErr) - return - } msg := trace.UserMessage(err) + if certErr := formatCertError(err); certErr != "" { + msg = certErr + } + // Error can be of type trace.proxyError where error message didn't get captured. if msg == "" { fmt.Fprintln(w, "please check Teleport's log for more details") return } - fmt.Fprintln(w, AllowWhitespace(msg)) + msg = AllowWhitespace(msg) + fmt.Fprint(w, msg) + if !strings.HasSuffix(msg, "\n") { + w.Write([]byte("\n")) + } } func formatCertError(err error) string { diff --git a/tool/tsh/common/tsh.go b/tool/tsh/common/tsh.go index 36175cfc24f..2fa2dcbf600 100644 --- a/tool/tsh/common/tsh.go +++ b/tool/tsh/common/tsh.go @@ -4214,7 +4214,7 @@ func retryWithAccessRequest( } // Print and log the original AccessDenied error. - fmt.Fprintln(os.Stderr, utils.UserMessageFromError(origErr)) + fmt.Fprint(os.Stderr, utils.UserMessageFromError(origErr)) fmt.Fprintf(os.Stdout, "You do not currently have access to %q, attempting to request access.\n\n", resource) if err := promptUserForAccessRequestDetails(cf, req); err != nil { return trace.Wrap(err) @@ -4599,7 +4599,7 @@ func convertSSHExitCode(tc *client.TeleportClient, err error) error { } // For unexpected errors, print the error message before converting to an exitCodeError. - fmt.Fprintln(tc.Stderr, utils.UserMessageFromError(err)) + fmt.Fprint(tc.Stderr, utils.UserMessageFromError(err)) return trace.Wrap(&common.ExitCodeError{Code: status}) } return trace.Wrap(err) @@ -4618,7 +4618,7 @@ func onBenchmark(cf *CLIConf, suite benchmark.Suite) error { result, err := cnf.Benchmark(cf.Context, tc, suite) if err != nil { - fmt.Fprintln(os.Stderr, utils.UserMessageFromError(err)) + fmt.Fprint(os.Stderr, utils.UserMessageFromError(err)) return trace.Wrap(&common.ExitCodeError{Code: 255}) } fmt.Fprintf(cf.Stdout(), "\n") @@ -4642,7 +4642,7 @@ func onBenchmark(cf *CLIConf, suite benchmark.Suite) error { if cf.BenchExport { path, err := benchmark.ExportLatencyProfile(cf.Context, cf.BenchExportPath, result.Histogram, cf.BenchTicks, cf.BenchValueScale) if err != nil { - fmt.Fprintf(cf.Stderr(), "failed exporting latency profile: %s\n", utils.UserMessageFromError(err)) + fmt.Fprintf(cf.Stderr(), "failed exporting latency profile: %s\n", trace.UserMessage(err)) } else { fmt.Fprintf(cf.Stdout(), "latency profile saved: %v\n", path) }