GODRIVER-3298 Handle joined errors correctly in WithTransaction. (#1928)

Author: matthewdale

mongo/errors.go

@@ -166,25 +166,10 @@ func IsTimeout(err error) bool {
 	return false
 }
 
-// unwrap returns the inner error if err implements Unwrap(), otherwise it returns nil.
-func unwrap(err error) error {
-	u, ok := err.(interface {
-		Unwrap() error
-	})
-	if !ok {
-		return nil
-	}
-	return u.Unwrap()
-}
-
 // errorHasLabel returns true if err contains the specified label
 func errorHasLabel(err error, label string) bool {
-	for ; err != nil; err = unwrap(err) {
-		if le, ok := err.(LabeledError); ok && le.HasErrorLabel(label) {
-			return true
-		}
-	}
-	return false
+	var le LabeledError
+	return errors.As(err, &le) && le.HasErrorLabel(label)
 }
 
 // IsNetworkError returns true if err is a network error

mongo/session.go

@@ -107,20 +107,23 @@ func (s *Session) EndSession(ctx context.Context) {
 // parameter already has a Session attached to it, it will be replaced by this
 // session. The fn callback may be run multiple times during WithTransaction due
 // to retry attempts, so it must be idempotent.
-// If a command inside the callback fn fails, it may cause the transaction on the
-// server to be aborted. This situation is normally handled transparently by the
-// driver. However, if the application does not return that error from the fn,
-// the driver will not be able to determine whether the transaction was aborted or
-// not. The driver will then retry the block indefinitely.
-// To avoid this situation, the application MUST NOT silently handle errors within
-// the callback fn. If the application needs to handle errors within the block,
-// it MUST return them after doing so.
-// Non-retryable operation errors or any operation errors that occur after the timeout
-// expires will be returned without retrying. If the callback fails, the driver will call
-// AbortTransaction. Because this method must succeed to ensure that server-side
-// resources are properly cleaned up, context deadlines and cancellations will
-// not be respected during this call. For a usage example, see the
-// Client.StartSession method documentation.
+//
+// If a command inside the callback fn fails, it may cause the transaction on
+// the server to be aborted. This situation is normally handled transparently by
+// the driver. However, if the application does not return that error from the
+// fn, the driver will not be able to determine whether the transaction was
+// aborted or not. The driver will then retry the block indefinitely.
+//
+// To avoid this situation, the application MUST NOT silently handle errors
+// within the callback fn. If the application needs to handle errors within the
+// block, it MUST return them after doing so.
+//
+// Non-retryable operation errors or any operation errors that occur after the
+// timeout expires will be returned without retrying. If the callback fails, the
+// driver will call AbortTransaction. Because this method must succeed to ensure
+// that server-side resources are properly cleaned up, context deadlines and
+// cancellations will not be respected during this call. For a usage example,
+// see the Client.StartSession method documentation.
 func (s *Session) WithTransaction(
 	ctx context.Context,
 	fn func(ctx context.Context) (interface{}, error),

mongo/with_transactions_test.go

@@ -21,6 +21,7 @@ import (
 	"go.mongodb.org/mongo-driver/v2/event"
 	"go.mongodb.org/mongo-driver/v2/internal/assert"
 	"go.mongodb.org/mongo-driver/v2/internal/integtest"
+	"go.mongodb.org/mongo-driver/v2/internal/require"
 	"go.mongodb.org/mongo-driver/v2/mongo/options"
 	"go.mongodb.org/mongo-driver/v2/mongo/readpref"
 	"go.mongodb.org/mongo-driver/v2/mongo/writeconcern"
@@ -576,6 +577,31 @@ func TestConvenientTransactions(t *testing.T) {
 			"expected transaction to be passed within 2s")
 
 	})
+	t.Run("retries correctly for joined errors", func(t *testing.T) {
+		withTransactionTimeout = 500 * time.Millisecond
+
+		sess, err := client.StartSession()
+		require.Nil(t, err, "StartSession error: %v", err)
+		defer sess.EndSession(context.Background())
+
+		count := 0
+		_, _ = sess.WithTransaction(context.Background(), func(context.Context) (interface{}, error) {
+			count++
+			time.Sleep(10 * time.Millisecond)
+
+			// Return a combined error value that is built using both
+			// errors.Join and fmt.Errorf with multiple "%w" verbs, nesting a
+			// retryable CommandError within the joined error tree.
+			return nil, errors.Join(
+				fmt.Errorf("%w, %w",
+					CommandError{Name: "test err 1", Labels: []string{driver.TransientTransactionError}},
+					errors.New("test err 2"),
+				),
+				errors.New("test err 3"),
+			)
+		})
+		assert.Greater(t, count, 1, "expected WithTransaction callback to be retried at least once")
+	})
 }
 
 func setupConvenientTransactions(t *testing.T, extraClientOpts ...*options.ClientOptions) *Client {