tsh: selectively render error messages in color

lib/config/configuration.go

@@ -2138,7 +2138,7 @@ func applyMetricsConfig(fc *FileConfig, cfg *servicecfg.Config) error {
 		if !utils.IsSelfSigned(certificateChain) {
 			if err := utils.VerifyCertificateChain(certificateChain); err != nil {
 				return trace.BadParameter("unable to verify the metrics service certificate chain in %v: %s",
-					p.Certificate, utils.UserMessageFromError(err))
+					p.Certificate, utils.UserMessageFromError(err, false))
 			}
 		}
 

lib/utils/cli.go

@@ -129,6 +129,8 @@ func InitLogger(purpose LoggingPurpose, level slog.Level, opts ...LoggerOption)
 	)
 	switch o.format {
 	case LogFormatText, "":
+		// TODO(zmb3): tsh hits this twice?
+		//
 		textFormatter := logutils.NewDefaultTextFormatter(enableColors)
 
 		// Calling CheckAndSetDefaults enables the timestamp field to
@@ -216,7 +218,12 @@ type Logger interface {
 // 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))
+	// TODO(timothyb89): Due to complications with globally enabling +
+	// properly resetting Windows terminal ANSI processing, for now we just
+	// disable color output. Otherwise, raw ANSI escapes will be visible to
+	// users.
+	enableColors := runtime.GOOS != constants.WindowsOS && IsTerminal(os.Stderr)
+	fmt.Fprintln(os.Stderr, UserMessageFromError(err, enableColors))
 	os.Exit(1)
 }
 
@@ -238,23 +245,20 @@ func GetIterations() int {
 // UserMessageFromError returns user-friendly error message from error.
 // The error message will be formatted for output depending on the debug
 // flag
-func UserMessageFromError(err error) string {
+func UserMessageFromError(err error, enableColors bool) string {
 	if err == nil {
 		return ""
 	}
 	if slog.Default().Enabled(context.Background(), slog.LevelDebug) {
 		return trace.DebugReport(err)
 	}
 	var buf bytes.Buffer
-	if runtime.GOOS == constants.WindowsOS {
-		// TODO(timothyb89): Due to complications with globally enabling +
-		// properly resetting Windows terminal ANSI processing, for now we just
-		// disable color output. Otherwise, raw ANSI escapes will be visible to
-		// users.
-		fmt.Fprint(&buf, "ERROR: ")
-	} else {
+	if enableColors {
 		fmt.Fprint(&buf, Color(Red, "ERROR: "))
+	} else {
+		fmt.Fprint(&buf, "ERROR: ")
 	}
+
 	formatErrorWriter(err, &buf)
 	return buf.String()
 }

lib/utils/cli_test.go

@@ -63,7 +63,7 @@ func TestUserMessageFromError(t *testing.T) {
 	}
 
 	for _, tt := range tests {
-		message := UserMessageFromError(tt.inError)
+		message := UserMessageFromError(tt.inError, true)
 		require.Contains(t, message, tt.outString)
 	}
 }

tool/tsh/common/tsh.go

@@ -3625,7 +3625,7 @@ func retryWithAccessRequest(
 	}
 
 	// Print and log the original AccessDenied error.
-	fmt.Fprintln(os.Stderr, utils.UserMessageFromError(origErr))
+	fmt.Fprintln(os.Stderr, utils.UserMessageFromError(origErr, shouldEnableColors()))
 	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)
@@ -3924,7 +3924,7 @@ func onSSH(cf *CLIConf) error {
 		if err != nil {
 			if errors.Is(err, teleport.ErrNodeIsAmbiguous) ||
 				// TODO(tross) DELETE IN v20.0.0
-				strings.Contains(utils.UserMessageFromError(err), teleport.NodeIsAmbiguous) {
+				strings.Contains(utils.UserMessageFromError(err, false), teleport.NodeIsAmbiguous) {
 				clt, err := tc.ConnectToCluster(cf.Context)
 				if err != nil {
 					return trace.Wrap(err)
@@ -3959,14 +3959,28 @@ func onSSH(cf *CLIConf) error {
 		}
 		if err != nil {
 			// Print the error here so we don't lose it when returning the exitCodeError.
-			fmt.Fprintln(tc.Stderr, utils.UserMessageFromError(err))
+			fmt.Fprintln(tc.Stderr, utils.UserMessageFromError(err, utils.IsTerminal(os.Stderr)))
 		}
 		err = &common.ExitCodeError{Code: tc.ExitStatus}
 		return trace.Wrap(err)
 	}
 	return trace.Wrap(err)
 }
 
+// shouldEnableColors determines whether tsh should use ANSI escape
+// sequences to render colored error messages.
+func shouldEnableColors() bool {
+	// TODO(timothyb89): Due to complications with globally enabling +
+	// properly resetting Windows terminal ANSI processing, for now we just
+	// disable color output. Otherwise, raw ANSI escapes will be visible to
+	// users.
+	if runtime.GOOS == constants.WindowsOS {
+		return false
+	}
+
+	return utils.IsTerminal(os.Stderr)
+}
+
 // onBenchmark executes benchmark
 func onBenchmark(cf *CLIConf, suite benchmark.Suite) error {
 	tc, err := makeClient(cf)
@@ -3978,9 +3992,11 @@ func onBenchmark(cf *CLIConf, suite benchmark.Suite) error {
 		Rate:          cf.BenchRate,
 	}
 
+	enableColors := shouldEnableColors()
+
 	result, err := cnf.Benchmark(cf.Context, tc, suite)
 	if err != nil {
-		fmt.Fprintln(os.Stderr, utils.UserMessageFromError(err))
+		fmt.Fprintln(os.Stderr, utils.UserMessageFromError(err, enableColors))
 		return trace.Wrap(&common.ExitCodeError{Code: 255})
 	}
 	fmt.Fprintf(cf.Stdout(), "\n")
@@ -4004,7 +4020,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", utils.UserMessageFromError(err, enableColors))
 		} else {
 			fmt.Fprintf(cf.Stdout(), "latency profile saved: %v\n", path)
 		}