Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

client: option to surface connection errors to callers #3430

Merged
merged 3 commits into from
Apr 9, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion clientconn.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,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)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be better for the second %v to be %w here, so the error is wrapped and thus available for introspection?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}
default:
}
}()
Expand Down Expand Up @@ -321,6 +328,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()
}
}
Expand Down
7 changes: 4 additions & 3 deletions clientconn_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
34 changes: 24 additions & 10 deletions dialoptions.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -298,6 +299,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.
Expand Down