Skip to content

Commit 5db858a

Browse files
committed
Handle context cancellation and improve comment correctness.
1 parent c573b2b commit 5db858a

File tree

3 files changed

+22
-14
lines changed

3 files changed

+22
-14
lines changed

x/mongo/driver/topology/connection_options.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ type connectionConfig struct {
5555
zstdLevel *int
5656
ocspCache ocsp.Cache
5757
disableOCSPEndpointCheck bool
58-
errorHandlingCallback func(error, error, uint64, *primitive.ObjectID)
58+
errorHandlingCallback func(err, ctxErr error, startGenNum uint64, svcID *primitive.ObjectID)
5959
tlsConnectionSource tlsConnectionSource
6060
loadBalanced bool
6161
getGenerationFn generationNumberFn
@@ -92,7 +92,7 @@ func withTLSConnectionSource(fn func(tlsConnectionSource) tlsConnectionSource) C
9292
}
9393
}
9494

95-
func withErrorHandlingCallback(fn func(error, error, uint64, *primitive.ObjectID)) ConnectionOption {
95+
func withErrorHandlingCallback(fn func(err, ctxErr error, startGenNum uint64, svcID *primitive.ObjectID)) ConnectionOption {
9696
return func(c *connectionConfig) error {
9797
c.errorHandlingCallback = fn
9898
return nil

x/mongo/driver/topology/connection_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ func TestConnection(t *testing.T) {
129129
return &net.TCPConn{}, nil
130130
})
131131
}),
132-
withErrorHandlingCallback(func(err, ctxErr error, _ uint64, _ *primitive.ObjectID) {
132+
withErrorHandlingCallback(func(err, _ error, _ uint64, _ *primitive.ObjectID) {
133133
got = err
134134
}),
135135
)

x/mongo/driver/topology/server.go

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,7 @@ func (s *Server) Connection(ctx context.Context) (driver.Connection, error) {
273273
}
274274

275275
// ProcessHandshakeError implements SDAM error handling for errors that occur before a connection finishes handshaking.
276-
// ctxErr is any error caused by the context passed to Server#Connection() and is used to determine whether or not an
276+
// ctxErr is any error caused by the context passed to Server.Connection() and is used to determine whether or not an
277277
// operation-scoped context deadline or cancellation was the cause of the handshake error.
278278
func (s *Server) ProcessHandshakeError(err, ctxErr error, startingGenerationNumber uint64, serviceID *primitive.ObjectID) {
279279
// Ignore the error if the server is behind a load balancer but the service ID is unknown. This indicates that the
@@ -292,29 +292,37 @@ func (s *Server) ProcessHandshakeError(err, ctxErr error, startingGenerationNumb
292292
return
293293
}
294294

295-
isTimeout := func(err error) bool {
295+
isTimeoutOrCanceled := func(err error) bool {
296296
for err != nil {
297+
// Check for errors that implement the "net.Error" interface and self-report as timeout
298+
// errors. Includes some "*net.OpError" errors and "context.DeadlineExceeded".
297299
if netErr, ok := err.(net.Error); ok && netErr.Timeout() {
298300
return true
299301
}
300-
// Handle the case where an error has been replaced by "net.errCanceled", which isn't
301-
// exported and can't be compared directly. In this case, just compare the error message.
302-
if err.Error() == "operation was canceled" {
302+
// Check for context cancellation. Also handle the case where the cancellation error has
303+
// been replaced by "net.errCanceled" (which isn't exported and can't be compared
304+
// directly) by checking the error message.
305+
if err == context.Canceled || err.Error() == "operation was canceled" {
303306
return true
304307
}
305-
if wrapper, ok := err.(interface{ Unwrap() error }); ok {
306-
err = wrapper.Unwrap()
307-
} else {
308+
309+
wrapper, ok := err.(interface{ Unwrap() error })
310+
if !ok {
308311
break
309312
}
313+
err = wrapper.Unwrap()
310314
}
311315

312316
return false
313317
}
314318

315-
// Ignore errors that indicate a client-side timeout occurred when using an operation-scoped
316-
// deadline (i.e. not using connectTimeoutMS as the connection timeout).
317-
if (ctxErr == context.DeadlineExceeded || ctxErr == context.Canceled) && isTimeout(wrappedConnErr) {
319+
// Ignore errors that indicate a client-side timeout occurred when the context passed into an
320+
// operation timed out or was canceled (i.e. errors caused by an operation-scoped timeout).
321+
// Timeouts caused by reaching connectTimeoutMS or other non-operation-scoped timeouts should
322+
// still clear the pool.
323+
// TODO(GODRIVER-2038): Remove this condition when connections are no longer created with an
324+
// operation-scoped timeout.
325+
if ctxErr != nil && isTimeoutOrCanceled(wrappedConnErr) {
318326
return
319327
}
320328

0 commit comments

Comments
 (0)