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 #564 ; it
does the same for plaintext connections.

Fixes #387

Author: bcleenders

grpcurl.go

@@ -733,20 +733,14 @@ func (c *errSignalingCreds) ClientHandshake(ctx context.Context, addr string, ra
 		c.writeResult(err)
 		return conn, auth, err
 	}
-	// Wrap TLS connections to capture post-handshake errors. With TLS 1.3,
-	// client certificate rejection by the server happens after the client
-	// considers the handshake complete. The server's TLS alert surfaces on the
-	// first Read from the connection. Only TLS connections need this (plaintext
-	// connections don't have post-handshake alerts).
-	if _, isTLS := auth.(credentials.TLSInfo); isTLS {
-		conn = &errSignalingConn{Conn: conn, writeResult: c.writeResult}
-	}
-	return conn, auth, nil
+	// 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. This allows BlockingDial to surface post-handshake
-// errors.
+// report it via writeResult.
 type errSignalingConn struct {
 	net.Conn
 	writeResult func(res interface{})

tls_settings_test.go

@@ -171,58 +171,17 @@ func TestBrokenTLS_ClientPlainText(t *testing.T) {
 		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)
 	}
 }