client: option to surface connection errors to callers (#3430)

sethp-nr · web-flow · commit b02de00073bf · 2020-04-09T13:45:13.000-07:00
This commit allows blocking clients to receive a more informative error
message than "context deadline exceeded", which is especially helpful in
tracking down persistent client misconfiguration (such as an invalid TLS
certificate, an invalid server that's refusing connections, etc.)
diff --git a/clientconn.go b/clientconn.go
@@ -217,7 +217,14 @@ func DialContext(ctx context.Context, target string, opts ...DialOption) (conn *
 	defer func() {
 		select {
 		case <-ctx.Done():
-			conn, err = nil, ctx.Err()
+			switch {
+			case ctx.Err() == err:
+				conn = nil
+			case err == nil || !cc.dopts.returnLastError:
+				conn, err = nil, ctx.Err()
+			default:
+				conn, err = nil, fmt.Errorf("%v: %v", ctx.Err(), err)
+			}
 		default:
 		}
 	}()
@@ -322,6 +329,9 @@ func DialContext(ctx context.Context, target string, opts ...DialOption) (conn *
 			}
 			if !cc.WaitForStateChange(ctx, s) {
 				// ctx got timeout or canceled.
+				if err = cc.blockingpicker.connectionError(); err != nil && cc.dopts.returnLastError {
+					return nil, err
+				}
 				return nil, ctx.Err()
 			}
 		}
diff --git a/clientconn_test.go b/clientconn_test.go
@@ -221,16 +221,17 @@ func (s) TestDialWaitsForServerSettingsAndFails(t *testing.T) {
 	client, err := DialContext(ctx,
 		lis.Addr().String(),
 		WithInsecure(),
-		WithBlock(),
+		WithReturnConnectionError(),
 		withBackoff(noBackoff{}),
 		withMinConnectDeadline(func() time.Duration { return time.Second / 4 }))
 	lis.Close()
 	if err == nil {
 		client.Close()
 		t.Fatalf("Unexpected success (err=nil) while dialing")
 	}
-	if err != context.DeadlineExceeded {
-		t.Fatalf("DialContext(_) = %v; want context.DeadlineExceeded", err)
+	expectedMsg := "server handshake"
+	if !strings.Contains(err.Error(), context.DeadlineExceeded.Error()) || !strings.Contains(err.Error(), expectedMsg) {
+		t.Fatalf("DialContext(_) = %v; want a message that includes both %q and %q", err, context.DeadlineExceeded.Error(), expectedMsg)
 	}
 	<-done
 	if numConns < 2 {
diff --git a/dialoptions.go b/dialoptions.go
@@ -46,16 +46,17 @@ type dialOptions struct {
 	chainUnaryInts  []UnaryClientInterceptor
 	chainStreamInts []StreamClientInterceptor
 
-	cp          Compressor
-	dc          Decompressor
-	bs          internalbackoff.Strategy
-	block       bool
-	insecure    bool
-	timeout     time.Duration
-	scChan      <-chan ServiceConfig
-	authority   string
-	copts       transport.ConnectOptions
-	callOptions []CallOption
+	cp              Compressor
+	dc              Decompressor
+	bs              internalbackoff.Strategy
+	block           bool
+	returnLastError bool
+	insecure        bool
+	timeout         time.Duration
+	scChan          <-chan ServiceConfig
+	authority       string
+	copts           transport.ConnectOptions
+	callOptions     []CallOption
 	// This is used by v1 balancer dial option WithBalancer to support v1
 	// balancer, and also by WithBalancerName dial option.
 	balancerBuilder             balancer.Builder
@@ -299,6 +300,19 @@ func WithBlock() DialOption {
 	})
 }
 
+// WithReturnConnectionError returns a DialOption which makes the client connection
+// return a string containing both the last connection error that occurred and
+// the context.DeadlineExceeded error.
+// Implies WithBlock()
+//
+// This API is EXPERIMENTAL.
+func WithReturnConnectionError() DialOption {
+	return newFuncDialOption(func(o *dialOptions) {
+		o.block = true
+		o.returnLastError = true
+	})
+}
+
 // WithInsecure returns a DialOption which disables transport security for this
 // ClientConn. Note that transport security is required unless WithInsecure is
 // set.
