[v9] Capture stderr from "tsh db connect" and reformat redis error (#13843)

docs/pages/database-access/guides/redis-cluster.mdx

@@ -33,7 +33,7 @@ This guide will help you to:
 
 - Redis version `6.0` or newer.
 
-- `redis-cli` installed and added to your system's `PATH` environment variable.
+- `redis-cli` version `6.2` or newer installed and added to your system's `PATH` environment variable.
 
 - A host where you will run the Teleport Database Service. Teleport version 9.0
   or newer must be installed.

lib/client/db/dbcmd/error.go

@@ -0,0 +1,56 @@
+/*
+Copyright 2022 Gravitational, Inc.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package dbcmd
+
+import (
+	"os/exec"
+	"path/filepath"
+	"strings"
+
+	"github.com/gravitational/trace"
+)
+
+// ConvertCommandError translates some common errors to more user friendly
+// messages.
+//
+// This helps in situations where the user does not have the full context to
+// decipher errors when the database command is executed internally (e.g.
+// command executed through "tsh db connect").
+func ConvertCommandError(cmd *exec.Cmd, err error, peakStderr string) error {
+	switch filepath.Base(cmd.Path) {
+	case redisBin:
+		// This redis-cli "Unrecognized option ..." error can be confusing to
+		// users that they may think it is the `tsh` binary that is not
+		// recognizing the flag.
+		if strings.HasPrefix(peakStderr, "Unrecognized option or bad number of args for") {
+			// TLS support starting 6.0. "--insecure" flag starting 6.2.
+			return trace.BadParameter(
+				"'%s' has exited with the above error. Please make sure '%s' with version 6.2 or newer is installed.",
+				cmd.Path,
+				redisBin,
+			)
+		}
+	}
+
+	return trace.Wrap(err)
+}
+
+const (
+	// PeakStderrSize is the recommended size for capturing stderr that is used
+	// for ConvertCommandError.
+	PeakStderrSize = 100
+)

lib/utils/writer.go

@@ -0,0 +1,53 @@
+/*
+Copyright 2022 Gravitational, Inc.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package utils
+
+// CaptureNBytesWriter is an io.Writer thats captures up to first n bytes
+// of the incoming data in memory, and then it ignores the rest of the incoming
+// data.
+type CaptureNBytesWriter struct {
+	capture      []byte
+	maxRemaining int
+}
+
+// NewCaptureNBytesWriter creates a new CaptureNBytesWriter.
+func NewCaptureNBytesWriter(max int) *CaptureNBytesWriter {
+	return &CaptureNBytesWriter{
+		maxRemaining: max,
+	}
+}
+
+// Write implements io.Writer.
+func (w *CaptureNBytesWriter) Write(p []byte) (int, error) {
+	if w.maxRemaining > 0 {
+		capture := p[:]
+		if len(capture) > w.maxRemaining {
+			capture = capture[:w.maxRemaining]
+		}
+
+		w.capture = append(w.capture, capture...)
+		w.maxRemaining -= len(capture)
+	}
+
+	// Always pretend to be successful.
+	return len(p), nil
+}
+
+// Bytes returns all captured bytes.
+func (w CaptureNBytesWriter) Bytes() []byte {
+	return w.capture
+}

lib/utils/writer_test.go

@@ -0,0 +1,46 @@
+/*
+Copyright 2022 Gravitational, Inc.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package utils
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestCaptureNBytesWriter(t *testing.T) {
+	data := []byte("abcdef")
+	w := NewCaptureNBytesWriter(10)
+
+	// Write 6 bytes. Captured 6 bytes in total.
+	n, err := w.Write(data)
+	require.Equal(t, 6, n)
+	require.NoError(t, err)
+	require.Equal(t, "abcdef", string(w.Bytes()))
+
+	// Write 6 bytes. Captured 10 bytes in total.
+	n, err = w.Write(data)
+	require.Equal(t, 6, n)
+	require.NoError(t, err)
+	require.Equal(t, "abcdefabcd", string(w.Bytes()))
+
+	// Write 6 bytes. Captured 10 bytes in total.
+	n, err = w.Write(data)
+	require.Equal(t, 6, n)
+	require.NoError(t, err)
+	require.Equal(t, "abcdefabcd", string(w.Bytes()))
+}

tool/tsh/db.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"encoding/base64"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"net"
 	"os"
@@ -686,12 +687,19 @@ func onDatabaseConnect(cf *CLIConf) error {
 		return trace.Wrap(err)
 	}
 	log.Debug(cmd.String())
+
 	cmd.Stdout = os.Stdout
-	cmd.Stderr = os.Stderr
 	cmd.Stdin = os.Stdin
+
+	// Use io.MultiWriter to duplicate stderr to the capture writer. The
+	// captured stderr can be used for diagnosing command failures. The capture
+	// writer captures up to a fixed number to limit memory usage.
+	peakStderr := utils.NewCaptureNBytesWriter(dbcmd.PeakStderrSize)
+	cmd.Stderr = io.MultiWriter(os.Stderr, peakStderr)
+
 	err = cmd.Run()
 	if err != nil {
-		return trace.Wrap(err)
+		return dbcmd.ConvertCommandError(cmd, err, string(peakStderr.Bytes()))
 	}
 	return nil
 }