Fast-fail on connection errors (#567)

Avoid hanging on connection errors; fail fast by propagating connection
errors.

This also speeds up tests a lot (from >20s to about 3s), since the tests
included some connection timeouts.

This expands on https://github.com/fullstorydev/grpcurl/pull/564 ; it
does the same for plaintext connections.

Fixes #387

grpcurl.go

@@ -568,9 +568,6 @@ func ClientTLSConfig(insecureSkipVerify bool, cacertFile, clientCertFile, client
 // client certs. The serverCertFile and serverKeyFile must both not be blank.
 func ServerTransportCredentials(cacertFile, serverCertFile, serverKeyFile string, requireClientCerts bool) (credentials.TransportCredentials, error) {
 	var tlsConf tls.Config
-	// TODO(jh): Remove this line once https://github.com/golang/go/issues/28779 is fixed
-	// in Go tip. Until then, the recently merged TLS 1.3 support breaks the TLS tests.
-	tlsConf.MaxVersion = tls.VersionTLS12
 
 	// Load the server certificates from disk
 	certificate, err := tls.LoadX509KeyPair(serverCertFile, serverKeyFile)
@@ -734,6 +731,25 @@ func (c *errSignalingCreds) ClientHandshake(ctx context.Context, addr string, ra
 	conn, auth, err := c.TransportCredentials.ClientHandshake(ctx, addr, rawConn)
 	if err != nil {
 		c.writeResult(err)
+		return conn, auth, err
 	}
-	return conn, auth, err
+	// Wrap the connection to capture post-handshake errors, e.g.:
+	// - TLS 1.3 client cert rejection (server sends alert after handshake)
+	// - Plaintext client to TLS server (server closes conn immediately)
+	return &errSignalingConn{Conn: conn, writeResult: c.writeResult}, auth, nil
+}
+
+// errSignalingConn wraps a net.Conn to capture the first read error and
+// report it via writeResult.
+type errSignalingConn struct {
+	net.Conn
+	writeResult func(res interface{})
+}
+
+func (c *errSignalingConn) Read(b []byte) (int, error) {
+	n, err := c.Conn.Read(b)
+	if err != nil {
+		c.writeResult(err)
+	}
+	return n, err
 }

tls_settings_test.go

@@ -2,6 +2,7 @@ package grpcurl_test
 
 import (
 	"context"
+	"crypto/tls"
 	"fmt"
 	"net"
 	"strings"
@@ -10,6 +11,7 @@ import (
 
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/credentials"
+	"google.golang.org/grpc/peer"
 
 	. "github.com/fullstorydev/grpcurl"
 	grpcurl_testing "github.com/fullstorydev/grpcurl/internal/testing"
@@ -101,64 +103,85 @@ func TestRequireClientCertTLS(t *testing.T) {
 	simpleTest(t, e.cc)
 }
 
+func TestTLS12(t *testing.T) {
+	serverCreds, err := ServerTransportCredentials("", "internal/testing/tls/server.crt", "internal/testing/tls/server.key", false)
+	if err != nil {
+		t.Fatalf("failed to create server creds: %v", err)
+	}
+	tlsConf, err := ClientTLSConfig(false, "internal/testing/tls/ca.crt", "", "")
+	if err != nil {
+		t.Fatalf("failed to create client TLS config: %v", err)
+	}
+	tlsConf.MaxVersion = tls.VersionTLS12
+
+	e, err := createTestServerAndClient(serverCreds, credentials.NewTLS(tlsConf))
+	if err != nil {
+		t.Fatalf("failed to setup server and client: %v", err)
+	}
+	defer e.Close()
+
+	tlsVersion := negotiatedTLSVersion(t, e.cc)
+	if tlsVersion != tls.VersionTLS12 {
+		t.Errorf("expected TLS 1.2, got 0x%04x", tlsVersion)
+	}
+}
+
+func TestTLS13(t *testing.T) {
+	serverCreds, err := ServerTransportCredentials("", "internal/testing/tls/server.crt", "internal/testing/tls/server.key", false)
+	if err != nil {
+		t.Fatalf("failed to create server creds: %v", err)
+	}
+	clientCreds, err := ClientTransportCredentials(false, "internal/testing/tls/ca.crt", "", "")
+	if err != nil {
+		t.Fatalf("failed to create client creds: %v", err)
+	}
+
+	e, err := createTestServerAndClient(serverCreds, clientCreds)
+	if err != nil {
+		t.Fatalf("failed to setup server and client: %v", err)
+	}
+	defer e.Close()
+
+	tlsVersion := negotiatedTLSVersion(t, e.cc)
+	if tlsVersion != tls.VersionTLS13 {
+		t.Errorf("expected TLS 1.3, got 0x%04x", tlsVersion)
+	}
+}
+
+func negotiatedTLSVersion(t *testing.T, cc *grpc.ClientConn) uint16 {
+	t.Helper()
+	cl := grpcurl_testing.NewTestServiceClient(cc)
+	ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
+	defer cancel()
+	var p peer.Peer
+	_, err := cl.UnaryCall(ctx, &grpcurl_testing.SimpleRequest{}, grpc.WaitForReady(true), grpc.Peer(&p))
+	if err != nil {
+		t.Fatalf("RPC failed: %v", err)
+	}
+	tlsInfo, ok := p.AuthInfo.(credentials.TLSInfo)
+	if !ok {
+		t.Fatalf("expected TLS auth info, got %T", p.AuthInfo)
+	}
+	return tlsInfo.State.Version
+}
+
 func TestBrokenTLS_ClientPlainText(t *testing.T) {
 	serverCreds, err := ServerTransportCredentials("", "internal/testing/tls/server.crt", "internal/testing/tls/server.key", false)
 	if err != nil {
 		t.Fatalf("failed to create server creds: %v", err)
 	}
 
-	// client connection (usually) succeeds since client is not waiting for TLS handshake
-	// (we try several times, but if we never get a connection and the error message is
-	// a known/expected possibility, we'll just bail)
-	var e testEnv
-	failCount := 0
-	for {
-		e, err = createTestServerAndClient(serverCreds, nil)
-		if err == nil {
-			// success!
-			defer e.Close()
-			break
-		}
-
-		if strings.Contains(err.Error(), "deadline exceeded") ||
-			strings.Contains(err.Error(), "use of closed network connection") {
-			// It is possible that the connection never becomes healthy:
-			//   1) grpc connects successfully
-			//   2) grpc client tries to send HTTP/2 preface and settings frame
-			//   3) server, expecting handshake, closes the connection
-			//   4) in the client, the write fails, so the connection never
-			//      becomes ready
-			// The client will attempt to reconnect on transient errors, so
-			// may eventually bump into the connect time limit. This used to
-			// result in a "deadline exceeded" error, but more recent versions
-			// of the grpc library report any underlying I/O error instead, so
-			// we also check for "use of closed network connection".
-			failCount++
-			if failCount > 5 {
-				return // bail...
-			}
-			// we'll try again
-
-		} else {
-			// some other error occurred, so we'll consider that a test failure
-			t.Fatalf("failed to setup server and client: %v", err)
-		}
-	}
-
-	// but request fails because server closes connection upon seeing request
-	// bytes that are not a TLS handshake
-	cl := grpcurl_testing.NewTestServiceClient(e.cc)
-	_, err = cl.UnaryCall(context.Background(), &grpcurl_testing.SimpleRequest{})
+	// Plaintext client to TLS server: the server expects a TLS handshake,
+	// gets an HTTP/2 preface instead, and closes the connection.
+	e, err := createTestServerAndClient(serverCreds, nil)
 	if err == nil {
-		t.Fatal("expecting failure")
+		e.Close()
+		t.Fatal("expecting failure when connecting plaintext to TLS server")
 	}
-	// various errors possible when server closes connection
-	if !strings.Contains(err.Error(), "transport is closing") &&
-		!strings.Contains(err.Error(), "connection is unavailable") &&
+	if !strings.Contains(err.Error(), "EOF") &&
 		!strings.Contains(err.Error(), "use of closed network connection") &&
-		!strings.Contains(err.Error(), "all SubConns are in TransientFailure") {
-
-		t.Fatalf("expecting transport failure, got: %v", err)
+		!strings.Contains(err.Error(), "connection reset by peer") {
+		t.Fatalf("expecting connection closed error, got: %v", err)
 	}
 }
 
@@ -253,12 +276,14 @@ func TestBrokenTLS_ClientNotTrusted(t *testing.T) {
 		e.Close()
 		t.Fatal("expecting TLS failure setting up server and client")
 	}
-	// Check for either the old error (Go <=1.24) or the new one (Go 1.25+)
-	// Go 1.24: "bad certificate"
-	// Go 1.25: "handshake failure"
+	// The exact TLS alert varies by Go version and TLS version negotiated:
+	// - TLS 1.2: "bad certificate" (Go <=1.24) or "handshake failure" (Go 1.25+)
+	// - TLS 1.3: "certificate required" (server rejects after handshake)
 	errMsg := err.Error()
-	if !strings.Contains(errMsg, "bad certificate") && !strings.Contains(errMsg, "handshake failure") {
-		t.Fatalf("expecting a specific TLS certificate or handshake error, got: %v", err)
+	if !strings.Contains(errMsg, "bad certificate") &&
+		!strings.Contains(errMsg, "handshake failure") &&
+		!strings.Contains(errMsg, "certificate required") {
+		t.Fatalf("expecting a TLS certificate error, got: %v", err)
 	}
 }
 
@@ -297,12 +322,14 @@ func TestBrokenTLS_RequireClientCertButNonePresented(t *testing.T) {
 		e.Close()
 		t.Fatal("expecting TLS failure setting up server and client")
 	}
-	// Check for either the old error (Go <=1.24) or the new one (Go 1.25+)
-	// Go 1.24: "bad certificate"
-	// Go 1.25: "handshake failure"
+	// The exact TLS alert varies by Go version and TLS version negotiated:
+	// - TLS 1.2: "bad certificate" (Go <=1.24) or "handshake failure" (Go 1.25+)
+	// - TLS 1.3: "certificate required" (server rejects after handshake)
 	errMsg := err.Error()
-	if !strings.Contains(errMsg, "bad certificate") && !strings.Contains(errMsg, "handshake failure") {
-		t.Fatalf("expecting a specific TLS certificate or handshake error, got: %v", err)
+	if !strings.Contains(errMsg, "bad certificate") &&
+		!strings.Contains(errMsg, "handshake failure") &&
+		!strings.Contains(errMsg, "certificate required") {
+		t.Fatalf("expecting a TLS certificate error, got: %v", err)
 	}
 }