Skip to content

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

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

Merged
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
19 changes: 2 additions & 17 deletions mongo/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
31 changes: 17 additions & 14 deletions mongo/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
26 changes: 26 additions & 0 deletions mongo/with_transactions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down
Loading