dial: better error handling if ssh command exits with non-zero exit status

SSHError.Error() relied on go-rwccmd behavior of returning io.EOF if the
ssh binary exited with status code 0.

We no longe ruse go-rwccmd => capture Stderr ourselves using zrepl's
circlog (depending on zrepl is not pretty, but since this package is supposedly
only used by zrepl ATM, this is fine)

refs zrepl/zrepl#237

Author: problame

dial.go

@@ -10,6 +10,8 @@ import (
 	"sync"
 	"syscall"
 	"time"
+
+	"github.com/zrepl/zrepl/util/circlog"
 )
 
 type Endpoint struct {
@@ -168,7 +170,7 @@ func (conn *SSHConn) shutdownProcess() *shutdownResult {
 		conn.shutdownResult = &shutdownResult{waitErr}
 	case <-timeout.C:
 		conn.cmdCancel()
-		waitErr := <- wait // reuse existing Wait invocation, must not call twice
+		waitErr := <-wait // reuse existing Wait invocation, must not call twice
 		conn.shutdownResult = &shutdownResult{waitErr}
 	}
 	return conn.shutdownResult
@@ -220,11 +222,6 @@ type SSHError struct {
 // Error() will try to present a one-line error message unless ssh stderr output is longer than one line
 func (e *SSHError) Error() string {
 
-	if e.RWCError == io.EOF {
-		// rwccmd returns io.EOF on exit status 0, but we do not expect ssh to do that
-		return fmt.Sprintf("ssh exited unexpectedly with exit status 0")
-	}
-
 	exitErr, ok := e.RWCError.(*exec.ExitError)
 	if !ok {
 		return fmt.Sprintf("ssh: %s", e.RWCError)
@@ -286,18 +283,31 @@ func Dial(dialCtx context.Context, endpoint Endpoint) (*SSHConn, error) {
 	if err != nil {
 		return nil, err
 	}
-	// stderr is required for *exec.ExitErr
+
+	stderrBuf, err := circlog.NewCircularLog(1 << 15)
+	if err != nil {
+		panic(err) // wrong API usage
+	}
+	cmd.Stderr = stderrBuf
 
 	if err = cmd.Start(); err != nil {
 		return nil, err
 	}
+	cmdWaitErrOrIOErr := func(ioErr error, what string) *SSHError {
+		werr := cmd.Wait()
+		if werr, ok := werr.(*exec.ExitError); ok {
+			werr.Stderr = []byte(stderrBuf.String())
+			return &SSHError{werr, what}
+		}
+		return &SSHError{ioErr, what}
+	}
 
 	confErrChan := make(chan error, 1)
 	go func() {
 		defer close(confErrChan)
 		var buf bytes.Buffer
 		if _, err := io.CopyN(&buf, stdout, int64(len(banner_msg))); err != nil {
-			confErrChan <- &SSHError{err, "read banner"}
+			confErrChan <- cmdWaitErrOrIOErr(err, "read banner")
 			return
 		}
 		resp := buf.Bytes()
@@ -314,7 +324,7 @@ func Dial(dialCtx context.Context, endpoint Endpoint) (*SSHConn, error) {
 		buf.Reset()
 		buf.Write(begin_msg)
 		if _, err := io.Copy(stdin, &buf); err != nil {
-			confErrChan <- &SSHError{err, "send begin message"}
+			confErrChan <- cmdWaitErrOrIOErr(err, "send begin message")
 			return
 		}
 	}()
@@ -332,6 +342,9 @@ func Dial(dialCtx context.Context, endpoint Endpoint) (*SSHConn, error) {
 		for _ = range confErrChan {
 		}
 
+		// TODO collect stderr in this case
+		// can probably extend *SSHError for this but need to implement net.Error
+
 		return nil, dialCtx.Err()
 
 	case err := <-confErrChan: