From 1cbbbda9a518d2aec04d86b28a43d1b6a15098df Mon Sep 17 00:00:00 2001 From: Qingyang Hu Date: Fri, 25 Apr 2025 18:23:40 -0400 Subject: [PATCH 1/5] GODRIVER-3361 Improve connection error message. --- x/mongo/driver/topology/connection.go | 4 ++-- x/mongo/driver/topology/errors.go | 31 ++++++++++++++++++--------- 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/x/mongo/driver/topology/connection.go b/x/mongo/driver/topology/connection.go index 24ad6a3a51..7712fd3bf0 100644 --- a/x/mongo/driver/topology/connection.go +++ b/x/mongo/driver/topology/connection.go @@ -212,7 +212,7 @@ func (c *connection) connect(ctx context.Context) (err error) { // Assign the result of DialContext to a temporary net.Conn to ensure that c.nc is not set in an error case. tempNc, err := c.config.dialer.DialContext(ctx, c.addr.Network(), c.addr.String()) if err != nil { - return ConnectionError{Wrapped: err, init: true} + return ConnectionError{Wrapped: err, init: true, message: fmt.Sprintf("failed to connect to %s", c.addr)} } c.nc = tempNc @@ -229,7 +229,7 @@ func (c *connection) connect(ctx context.Context) (err error) { tlsNc, err := configureTLS(ctx, c.config.tlsConnectionSource, c.nc, c.addr, tlsConfig, ocspOpts) if err != nil { - return ConnectionError{Wrapped: err, init: true} + return ConnectionError{Wrapped: err, init: true, message: fmt.Sprintf("failed to configure TLS for %s", c.addr)} } c.nc = tlsNc } diff --git a/x/mongo/driver/topology/errors.go b/x/mongo/driver/topology/errors.go index 79f11f7f79..9ad40f391b 100644 --- a/x/mongo/driver/topology/errors.go +++ b/x/mongo/driver/topology/errors.go @@ -10,6 +10,10 @@ import ( "context" "errors" "fmt" + "io" + "net" + "os" + "strings" "time" "go.mongodb.org/mongo-driver/v2/x/mongo/driver/description" @@ -28,21 +32,28 @@ type ConnectionError struct { // Error implements the error interface. func (e ConnectionError) Error() string { - message := e.message + var messages []string if e.init { - fullMsg := "error occurred during connection handshake" - if message != "" { - fullMsg = fmt.Sprintf("%s: %s", fullMsg, message) - } - message = fullMsg + messages = append(messages, "error occurred during connection handshake") } - if e.Wrapped != nil && message != "" { - return fmt.Sprintf("connection(%s) %s: %s", e.ConnectionID, message, e.Wrapped.Error()) + if e.message != "" { + messages = append(messages, e.message) } if e.Wrapped != nil { - return fmt.Sprintf("connection(%s) %s", e.ConnectionID, e.Wrapped.Error()) + if errors.Is(e.Wrapped, io.EOF) { + messages = append(messages, "connection closed unexpectedly by the other side") + } + if errors.Is(e.Wrapped, os.ErrDeadlineExceeded) { + messages = append(messages, "client timed out waiting for server response") + } else if err, ok := e.Wrapped.(net.Error); ok && err.Timeout() { + messages = append(messages, "client timed out waiting for server response") + } + messages = append(messages, e.Wrapped.Error()) + } + if len(messages) > 0 { + return fmt.Sprintf("connection(%s) %s", e.ConnectionID, strings.Join(messages, ": ")) } - return fmt.Sprintf("connection(%s) %s", e.ConnectionID, message) + return fmt.Sprintf("connection(%s)", e.ConnectionID) } // Unwrap returns the underlying error. From 076f0df4a0e84819db10fb7a1f205c5bade2b619 Mon Sep 17 00:00:00 2001 From: Qingyang Hu Date: Mon, 28 Apr 2025 16:41:00 -0400 Subject: [PATCH 2/5] fix tests --- x/mongo/driver/topology/connection_test.go | 4 ++-- x/mongo/driver/topology/pool_test.go | 3 ++- x/mongo/driver/topology/server_test.go | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/x/mongo/driver/topology/connection_test.go b/x/mongo/driver/topology/connection_test.go index f4d0cd9d7d..13c9a7f6e7 100644 --- a/x/mongo/driver/topology/connection_test.go +++ b/x/mongo/driver/topology/connection_test.go @@ -63,8 +63,8 @@ func TestConnection(t *testing.T) { t.Run("connect", func(t *testing.T) { t.Run("dialer error", func(t *testing.T) { err := errors.New("dialer error") - var want error = ConnectionError{Wrapped: err, init: true} - conn := newConnection(address.Address(""), WithDialer(func(Dialer) Dialer { + var want error = ConnectionError{Wrapped: err, init: true, message: "failed to connect to testaddr:27017"} + conn := newConnection(address.Address("testaddr"), WithDialer(func(Dialer) Dialer { return DialerFunc(func(context.Context, string, string) (net.Conn, error) { return nil, err }) })) got := conn.connect(context.Background()) diff --git a/x/mongo/driver/topology/pool_test.go b/x/mongo/driver/topology/pool_test.go index 3d270de2e0..6cc7d54287 100644 --- a/x/mongo/driver/topology/pool_test.go +++ b/x/mongo/driver/topology/pool_test.go @@ -471,6 +471,7 @@ func TestPool_checkOut(t *testing.T) { dialErr := errors.New("create new connection error") p := newPool(poolConfig{ + Address: "testaddr", ConnectTimeout: defaultConnectionTimeout, }, WithDialer(func(Dialer) Dialer { return DialerFunc(func(context.Context, string, string) (net.Conn, error) { @@ -481,7 +482,7 @@ func TestPool_checkOut(t *testing.T) { require.NoError(t, err) _, err = p.checkOut(context.Background()) - var want error = ConnectionError{Wrapped: dialErr, init: true} + var want error = ConnectionError{Wrapped: dialErr, init: true, message: "failed to connect to testaddr:27017"} assert.Equalf(t, want, err, "should return error from calling checkOut()") // If a connection initialization error occurs during checkOut, removing and closing the // failed connection both happen asynchronously with the checkOut. Wait for up to 2s for diff --git a/x/mongo/driver/topology/server_test.go b/x/mongo/driver/topology/server_test.go index 5ab5692840..818a7818f6 100644 --- a/x/mongo/driver/topology/server_test.go +++ b/x/mongo/driver/topology/server_test.go @@ -376,7 +376,7 @@ func TestServer(t *testing.T) { } authErr := ConnectionError{Wrapped: &auth.Error{}, init: true} - netErr := ConnectionError{Wrapped: &net.AddrError{}, init: true} + netErr := ConnectionError{Wrapped: &net.AddrError{}, init: true, message: "failed to connect to localhost:27017"} for _, tt := range serverTestTable { t.Run(tt.name, func(t *testing.T) { var returnConnectionError bool From 9cefb59cad46eb79372931e85badc7a00cf02eaa Mon Sep 17 00:00:00 2001 From: Qingyang Hu Date: Tue, 29 Apr 2025 17:09:54 -0400 Subject: [PATCH 3/5] minor updates --- x/mongo/driver/topology/connection.go | 3 --- x/mongo/driver/topology/errors.go | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/x/mongo/driver/topology/connection.go b/x/mongo/driver/topology/connection.go index 7712fd3bf0..e5f98cfe5c 100644 --- a/x/mongo/driver/topology/connection.go +++ b/x/mongo/driver/topology/connection.go @@ -413,9 +413,6 @@ func (c *connection) readWireMessage(ctx context.Context) ([]byte, error) { c.close() } message := errMsg - if errors.Is(err, io.EOF) { - message = "socket was unexpectedly closed" - } return nil, ConnectionError{ ConnectionID: c.id, Wrapped: transformNetworkError(ctx, err, contextDeadlineUsed), diff --git a/x/mongo/driver/topology/errors.go b/x/mongo/driver/topology/errors.go index 9ad40f391b..6740d54a95 100644 --- a/x/mongo/driver/topology/errors.go +++ b/x/mongo/driver/topology/errors.go @@ -41,7 +41,7 @@ func (e ConnectionError) Error() string { } if e.Wrapped != nil { if errors.Is(e.Wrapped, io.EOF) { - messages = append(messages, "connection closed unexpectedly by the other side") + messages = append(messages, "socket was unexpectedly closed") } if errors.Is(e.Wrapped, os.ErrDeadlineExceeded) { messages = append(messages, "client timed out waiting for server response") From c3a7110047e0516385018ecc0765159b673f034e Mon Sep 17 00:00:00 2001 From: Qingyang Hu Date: Fri, 2 May 2025 17:07:35 -0400 Subject: [PATCH 4/5] minor updates --- x/mongo/driver/topology/connection.go | 5 ++++- x/mongo/driver/topology/errors.go | 4 +++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/x/mongo/driver/topology/connection.go b/x/mongo/driver/topology/connection.go index e5f98cfe5c..28ed3581e8 100644 --- a/x/mongo/driver/topology/connection.go +++ b/x/mongo/driver/topology/connection.go @@ -341,7 +341,10 @@ func transformNetworkError(ctx context.Context, originalError error, contextDead return originalError } if netErr, ok := originalError.(net.Error); ok && netErr.Timeout() { - return fmt.Errorf("%w: %s", context.DeadlineExceeded, originalError.Error()) + return fmt.Errorf("%w: %s: %s", + context.DeadlineExceeded, + "client timed out waiting for server response", + originalError.Error()) } return originalError diff --git a/x/mongo/driver/topology/errors.go b/x/mongo/driver/topology/errors.go index 6740d54a95..dee1941542 100644 --- a/x/mongo/driver/topology/errors.go +++ b/x/mongo/driver/topology/errors.go @@ -19,6 +19,8 @@ import ( "go.mongodb.org/mongo-driver/v2/x/mongo/driver/description" ) +var _ error = ConnectionError{} + // ConnectionError represents a connection error. type ConnectionError struct { ConnectionID string @@ -41,7 +43,7 @@ func (e ConnectionError) Error() string { } if e.Wrapped != nil { if errors.Is(e.Wrapped, io.EOF) { - messages = append(messages, "socket was unexpectedly closed") + messages = append(messages, "connection closed unexpectedly by the other side") } if errors.Is(e.Wrapped, os.ErrDeadlineExceeded) { messages = append(messages, "client timed out waiting for server response") From 482f7461d4fb62e3a3aa3b12155f058643877354 Mon Sep 17 00:00:00 2001 From: Qingyang Hu Date: Fri, 2 May 2025 17:30:00 -0400 Subject: [PATCH 5/5] fix test cases --- x/mongo/driver/topology/pool_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/x/mongo/driver/topology/pool_test.go b/x/mongo/driver/topology/pool_test.go index 6cc7d54287..94d4de285d 100644 --- a/x/mongo/driver/topology/pool_test.go +++ b/x/mongo/driver/topology/pool_test.go @@ -1279,7 +1279,7 @@ func TestBackgroundRead(t *testing.T) { defer cancel() _, err = conn.readWireMessage(ctx) regex := regexp.MustCompile( - `^connection\(.*\[-\d+\]\) incomplete read of message header: context deadline exceeded: read tcp 127.0.0.1:.*->127.0.0.1:.*: i\/o timeout$`, + `^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$`, ) assert.True(t, regex.MatchString(err.Error()), "error %q does not match pattern %q", err, regex) assert.Nil(t, conn.awaitRemainingBytes, "conn.awaitRemainingBytes should be nil") @@ -1319,7 +1319,7 @@ func TestBackgroundRead(t *testing.T) { defer cancel() _, err = conn.readWireMessage(ctx) regex := regexp.MustCompile( - `^connection\(.*\[-\d+\]\) incomplete read of message header: context deadline exceeded: read tcp 127.0.0.1:.*->127.0.0.1:.*: i\/o timeout$`, + `^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$`, ) assert.True(t, regex.MatchString(err.Error()), "error %q does not match pattern %q", err, regex) err = p.checkIn(conn) @@ -1366,7 +1366,7 @@ func TestBackgroundRead(t *testing.T) { defer cancel() _, err = conn.readWireMessage(ctx) regex := regexp.MustCompile( - `^connection\(.*\[-\d+\]\) incomplete read of message header: context deadline exceeded: read tcp 127.0.0.1:.*->127.0.0.1:.*: i\/o timeout$`, + `^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$`, ) assert.True(t, regex.MatchString(err.Error()), "error %q does not match pattern %q", err, regex) err = p.checkIn(conn) @@ -1418,7 +1418,7 @@ func TestBackgroundRead(t *testing.T) { defer cancel() _, err = conn.readWireMessage(ctx) regex := regexp.MustCompile( - `^connection\(.*\[-\d+\]\) incomplete read of message header: context deadline exceeded: read tcp 127.0.0.1:.*->127.0.0.1:.*: i\/o timeout$`, + `^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$`, ) assert.True(t, regex.MatchString(err.Error()), "error %q does not match pattern %q", err, regex) err = p.checkIn(conn) @@ -1472,7 +1472,7 @@ func TestBackgroundRead(t *testing.T) { defer cancel() _, err = conn.readWireMessage(ctx) regex := regexp.MustCompile( - `^connection\(.*\[-\d+\]\) incomplete read of full message: context deadline exceeded: read tcp 127.0.0.1:.*->127.0.0.1:.*: i\/o timeout$`, + `^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$`, ) assert.True(t, regex.MatchString(err.Error()), "error %q does not match pattern %q", err, regex) err = p.checkIn(conn) @@ -1522,7 +1522,7 @@ func TestBackgroundRead(t *testing.T) { defer cancel() _, err = conn.readWireMessage(ctx) regex := regexp.MustCompile( - `^connection\(.*\[-\d+\]\) incomplete read of full message: context deadline exceeded: read tcp 127.0.0.1:.*->127.0.0.1:.*: i\/o timeout$`, + `^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$`, ) assert.True(t, regex.MatchString(err.Error()), "error %q does not match pattern %q", err, regex) err = p.checkIn(conn)