Skip to content

Commit 12598be

Browse files
qingyang-huqingyang-hu
authored
GODRIVER-3361 Improve connection error message. (#2027)
1 parent e918e14 commit 12598be

File tree

5 files changed

+101
-26
lines changed

5 files changed

+101
-26
lines changed

‎x/mongo/driver/topology/connection.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ func (c *connection) connect(ctx context.Context) (err error) {
212212
// Assign the result of DialContext to a temporary net.Conn to ensure that c.nc is not set in an error case.
213213
tempNc, err := c.config.dialer.DialContext(ctx, c.addr.Network(), c.addr.String())
214214
if err != nil {
215-
return ConnectionError{Wrapped: err, init: true}
215+
return ConnectionError{Wrapped: err, init: true, message: fmt.Sprintf("failed to connect to %s", c.addr)}
216216
}
217217
c.nc = tempNc
218218

@@ -229,7 +229,7 @@ func (c *connection) connect(ctx context.Context) (err error) {
229229
tlsNc, err := configureTLS(ctx, c.config.tlsConnectionSource, c.nc, c.addr, tlsConfig, ocspOpts)
230230

231231
if err != nil {
232-
return ConnectionError{Wrapped: err, init: true}
232+
return ConnectionError{Wrapped: err, init: true, message: fmt.Sprintf("failed to configure TLS for %s", c.addr)}
233233
}
234234
c.nc = tlsNc
235235
}
@@ -341,7 +341,10 @@ func transformNetworkError(ctx context.Context, originalError error, contextDead
341341
return originalError
342342
}
343343
if netErr, ok := originalError.(net.Error); ok && netErr.Timeout() {
344-
return fmt.Errorf("%w: %s", context.DeadlineExceeded, originalError.Error())
344+
return fmt.Errorf("%w: %s: %s",
345+
context.DeadlineExceeded,
346+
"client timed out waiting for server response",
347+
originalError.Error())
345348
}
346349

347350
return originalError
@@ -413,9 +416,6 @@ func (c *connection) readWireMessage(ctx context.Context) ([]byte, error) {
413416
c.close()
414417
}
415418
message := errMsg
416-
if errors.Is(err, io.EOF) {
417-
message = "socket was unexpectedly closed"
418-
}
419419
return nil, ConnectionError{
420420
ConnectionID: c.id,
421421
Wrapped: transformNetworkError(ctx, err, contextDeadlineUsed),

‎x/mongo/driver/topology/connection_test.go

Lines changed: 63 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,8 @@ func TestConnection(t *testing.T) {
6363
t.Run("connect", func(t *testing.T) {
6464
t.Run("dialer error", func(t *testing.T) {
6565
err := errors.New("dialer error")
66-
var want error = ConnectionError{Wrapped: err, init: true}
67-
conn := newConnection(address.Address(""), WithDialer(func(Dialer) Dialer {
66+
var want error = ConnectionError{Wrapped: err, init: true, message: "failed to connect to testaddr:27017"}
67+
conn := newConnection(address.Address("testaddr"), WithDialer(func(Dialer) Dialer {
6868
return DialerFunc(func(context.Context, string, string) (net.Conn, error) { return nil, err })
6969
}))
7070
got := conn.connect(context.Background())
@@ -1101,6 +1101,67 @@ func (tcl *testCancellationListener) assertCalledOnce(t *testing.T) {
11011101
assert.Equal(t, 1, tcl.numStopListening, "expected StopListening to be called once, got %d", tcl.numListen)
11021102
}
11031103

1104+
type testContext struct {
1105+
context.Context
1106+
deadline time.Time
1107+
}
1108+
1109+
func (tc *testContext) Deadline() (time.Time, bool) {
1110+
return tc.deadline, false
1111+
}
1112+
1113+
func TestConnectionError(t *testing.T) {
1114+
t.Parallel()
1115+
1116+
t.Run("EOF", func(t *testing.T) {
1117+
t.Parallel()
1118+
1119+
addr := bootstrapConnections(t, 1, func(nc net.Conn) {
1120+
_ = nc.Close()
1121+
})
1122+
1123+
p := newPool(
1124+
poolConfig{Address: address.Address(addr.String())},
1125+
)
1126+
defer p.close(context.Background())
1127+
err := p.ready()
1128+
require.NoError(t, err, "unexpected close error")
1129+
1130+
conn, err := p.checkOut(context.Background())
1131+
require.NoError(t, err, "unexpected checkOut error")
1132+
1133+
_, err = conn.readWireMessage(context.Background())
1134+
assert.ErrorContains(t, err, "connection closed unexpectedly by the other side: EOF")
1135+
})
1136+
t.Run("timeout", func(t *testing.T) {
1137+
t.Parallel()
1138+
1139+
timeout := 10 * time.Millisecond
1140+
1141+
addr := bootstrapConnections(t, 1, func(nc net.Conn) {
1142+
time.Sleep(timeout * 2)
1143+
_ = nc.Close()
1144+
})
1145+
1146+
p := newPool(
1147+
poolConfig{Address: address.Address(addr.String())},
1148+
)
1149+
defer p.close(context.Background())
1150+
err := p.ready()
1151+
require.NoError(t, err)
1152+
1153+
conn, err := p.checkOut(context.Background())
1154+
require.NoError(t, err)
1155+
1156+
ctx := &testContext{
1157+
Context: context.Background(),
1158+
deadline: time.Now().Add(timeout),
1159+
}
1160+
_, err = conn.readWireMessage(ctx)
1161+
assert.ErrorContains(t, err, "client timed out waiting for server response")
1162+
})
1163+
}
1164+
11041165
func TestConnection_IsAlive(t *testing.T) {
11051166
t.Parallel()
11061167

‎x/mongo/driver/topology/errors.go

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,17 @@ import (
1010
"context"
1111
"errors"
1212
"fmt"
13+
"io"
14+
"net"
15+
"os"
16+
"strings"
1317
"time"
1418

1519
"go.mongodb.org/mongo-driver/v2/x/mongo/driver/description"
1620
)
1721

22+
var _ error = ConnectionError{}
23+
1824
// ConnectionError represents a connection error.
1925
type ConnectionError struct {
2026
ConnectionID string
@@ -28,21 +34,28 @@ type ConnectionError struct {
2834

2935
// Error implements the error interface.
3036
func (e ConnectionError) Error() string {
31-
message := e.message
37+
var messages []string
3238
if e.init {
33-
fullMsg := "error occurred during connection handshake"
34-
if message != "" {
35-
fullMsg = fmt.Sprintf("%s: %s", fullMsg, message)
36-
}
37-
message = fullMsg
39+
messages = append(messages, "error occurred during connection handshake")
3840
}
39-
if e.Wrapped != nil && message != "" {
40-
return fmt.Sprintf("connection(%s) %s: %s", e.ConnectionID, message, e.Wrapped.Error())
41+
if e.message != "" {
42+
messages = append(messages, e.message)
4143
}
4244
if e.Wrapped != nil {
43-
return fmt.Sprintf("connection(%s) %s", e.ConnectionID, e.Wrapped.Error())
45+
if errors.Is(e.Wrapped, io.EOF) {
46+
messages = append(messages, "connection closed unexpectedly by the other side")
47+
}
48+
if errors.Is(e.Wrapped, os.ErrDeadlineExceeded) {
49+
messages = append(messages, "client timed out waiting for server response")
50+
} else if err, ok := e.Wrapped.(net.Error); ok && err.Timeout() {
51+
messages = append(messages, "client timed out waiting for server response")
52+
}
53+
messages = append(messages, e.Wrapped.Error())
54+
}
55+
if len(messages) > 0 {
56+
return fmt.Sprintf("connection(%s) %s", e.ConnectionID, strings.Join(messages, ": "))
4457
}
45-
return fmt.Sprintf("connection(%s) %s", e.ConnectionID, message)
58+
return fmt.Sprintf("connection(%s)", e.ConnectionID)
4659
}
4760

4861
// Unwrap returns the underlying error.

‎x/mongo/driver/topology/pool_test.go

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -471,6 +471,7 @@ func TestPool_checkOut(t *testing.T) {
471471

472472
dialErr := errors.New("create new connection error")
473473
p := newPool(poolConfig{
474+
Address: "testaddr",
474475
ConnectTimeout: defaultConnectionTimeout,
475476
}, WithDialer(func(Dialer) Dialer {
476477
return DialerFunc(func(context.Context, string, string) (net.Conn, error) {
@@ -481,7 +482,7 @@ func TestPool_checkOut(t *testing.T) {
481482
require.NoError(t, err)
482483

483484
_, err = p.checkOut(context.Background())
484-
var want error = ConnectionError{Wrapped: dialErr, init: true}
485+
var want error = ConnectionError{Wrapped: dialErr, init: true, message: "failed to connect to testaddr:27017"}
485486
assert.Equalf(t, want, err, "should return error from calling checkOut()")
486487
// If a connection initialization error occurs during checkOut, removing and closing the
487488
// failed connection both happen asynchronously with the checkOut. Wait for up to 2s for
@@ -1278,7 +1279,7 @@ func TestBackgroundRead(t *testing.T) {
12781279
defer cancel()
12791280
_, err = conn.readWireMessage(ctx)
12801281
regex := regexp.MustCompile(
1281-
`^connection\(.*\[-\d+\]\) incomplete read of message header: context deadline exceeded: read tcp 127.0.0.1:.*->127.0.0.1:.*: i\/o timeout$`,
1282+
`^connection\(.*\[-\d+\]\) incomplete read of message header: context deadline exceeded: client timed out waiting for server response: read tcp 127.0.0.1:.*->127.0.0.1:.*: i\/o timeout$`,
12821283
)
12831284
assert.True(t, regex.MatchString(err.Error()), "error %q does not match pattern %q", err, regex)
12841285
assert.Nil(t, conn.awaitRemainingBytes, "conn.awaitRemainingBytes should be nil")
@@ -1318,7 +1319,7 @@ func TestBackgroundRead(t *testing.T) {
13181319
defer cancel()
13191320
_, err = conn.readWireMessage(ctx)
13201321
regex := regexp.MustCompile(
1321-
`^connection\(.*\[-\d+\]\) incomplete read of message header: context deadline exceeded: read tcp 127.0.0.1:.*->127.0.0.1:.*: i\/o timeout$`,
1322+
`^connection\(.*\[-\d+\]\) incomplete read of message header: context deadline exceeded: client timed out waiting for server response: read tcp 127.0.0.1:.*->127.0.0.1:.*: i\/o timeout$`,
13221323
)
13231324
assert.True(t, regex.MatchString(err.Error()), "error %q does not match pattern %q", err, regex)
13241325
err = p.checkIn(conn)
@@ -1365,7 +1366,7 @@ func TestBackgroundRead(t *testing.T) {
13651366
defer cancel()
13661367
_, err = conn.readWireMessage(ctx)
13671368
regex := regexp.MustCompile(
1368-
`^connection\(.*\[-\d+\]\) incomplete read of message header: context deadline exceeded: read tcp 127.0.0.1:.*->127.0.0.1:.*: i\/o timeout$`,
1369+
`^connection\(.*\[-\d+\]\) incomplete read of message header: context deadline exceeded: client timed out waiting for server response: read tcp 127.0.0.1:.*->127.0.0.1:.*: i\/o timeout$`,
13691370
)
13701371
assert.True(t, regex.MatchString(err.Error()), "error %q does not match pattern %q", err, regex)
13711372
err = p.checkIn(conn)
@@ -1417,7 +1418,7 @@ func TestBackgroundRead(t *testing.T) {
14171418
defer cancel()
14181419
_, err = conn.readWireMessage(ctx)
14191420
regex := regexp.MustCompile(
1420-
`^connection\(.*\[-\d+\]\) incomplete read of message header: context deadline exceeded: read tcp 127.0.0.1:.*->127.0.0.1:.*: i\/o timeout$`,
1421+
`^connection\(.*\[-\d+\]\) incomplete read of message header: context deadline exceeded: client timed out waiting for server response: read tcp 127.0.0.1:.*->127.0.0.1:.*: i\/o timeout$`,
14211422
)
14221423
assert.True(t, regex.MatchString(err.Error()), "error %q does not match pattern %q", err, regex)
14231424
err = p.checkIn(conn)
@@ -1471,7 +1472,7 @@ func TestBackgroundRead(t *testing.T) {
14711472
defer cancel()
14721473
_, err = conn.readWireMessage(ctx)
14731474
regex := regexp.MustCompile(
1474-
`^connection\(.*\[-\d+\]\) incomplete read of full message: context deadline exceeded: read tcp 127.0.0.1:.*->127.0.0.1:.*: i\/o timeout$`,
1475+
`^connection\(.*\[-\d+\]\) incomplete read of full message: context deadline exceeded: client timed out waiting for server response: read tcp 127.0.0.1:.*->127.0.0.1:.*: i\/o timeout$`,
14751476
)
14761477
assert.True(t, regex.MatchString(err.Error()), "error %q does not match pattern %q", err, regex)
14771478
err = p.checkIn(conn)
@@ -1521,7 +1522,7 @@ func TestBackgroundRead(t *testing.T) {
15211522
defer cancel()
15221523
_, err = conn.readWireMessage(ctx)
15231524
regex := regexp.MustCompile(
1524-
`^connection\(.*\[-\d+\]\) incomplete read of full message: context deadline exceeded: read tcp 127.0.0.1:.*->127.0.0.1:.*: i\/o timeout$`,
1525+
`^connection\(.*\[-\d+\]\) incomplete read of full message: context deadline exceeded: client timed out waiting for server response: read tcp 127.0.0.1:.*->127.0.0.1:.*: i\/o timeout$`,
15251526
)
15261527
assert.True(t, regex.MatchString(err.Error()), "error %q does not match pattern %q", err, regex)
15271528
err = p.checkIn(conn)

‎x/mongo/driver/topology/server_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -376,7 +376,7 @@ func TestServer(t *testing.T) {
376376
}
377377

378378
authErr := ConnectionError{Wrapped: &auth.Error{}, init: true}
379-
netErr := ConnectionError{Wrapped: &net.AddrError{}, init: true}
379+
netErr := ConnectionError{Wrapped: &net.AddrError{}, init: true, message: "failed to connect to localhost:27017"}
380380
for _, tt := range serverTestTable {
381381
t.Run(tt.name, func(t *testing.T) {
382382
var returnConnectionError bool

0 commit comments

Comments
 (0)