Skip to content

GODRIVER-3399: PoolClearedError should have TransientTransactionError label appended to it #2114

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.

Sign up for GitHub

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

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

GODRIVER-3399: PoolClearedError should have TransientTransactionError label appended to it #2114

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
bfe0067
add TransientTransactionError label to poolClearedError
zhouselena Jun 24, 2025
f5a2321
Merge branch 'master' of github.com:zhouselena/mongo-go-driver into G…
zhouselena Jun 24, 2025
06308d2
cleanup
zhouselena Jun 24, 2025
f4fc9f4
cleanup
zhouselena Jun 24, 2025
2bf70b3
update to driver.TransientTransactionError
zhouselena Jun 24, 2025
38d2589
wrap error instead of adding errorLabels
zhouselena Jun 24, 2025
d15e4c6
wrap error in driver.error
zhouselena Jun 25, 2025
46de325
wrap all instances of pce
zhouselena Jun 25, 2025
7f6ed4d
pull back idle connections pcerr
zhouselena Jun 25, 2025
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
7 changes: 6 additions & 1 deletion x/mongo/driver/topology/pool.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,12 @@ func (p *pool) checkOut(ctx context.Context) (conn *connection, err error) {
}
return nil, ErrPoolClosed
case poolPaused:
err := poolClearedError{err: p.lastClearErr, address: p.address}
pcErr := poolClearedError{err: p.lastClearErr, address: p.address}
err := driver.Error{
Message: pcErr.Error(),
Labels: []string{driver.TransientTransactionError},
Wrapped: pcErr,
Comment on lines +506 to +510
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically this will add the "TransientTransactionError" to all poolClearedErrors, independent of if they happen during a transaction or not. Considering the spec is fuzzy on the exact expected behavior, I think the current code is good, but it's worth leaving a comment that describes the deviation from other cases where we add "TransientTransactionError".

E.g. recommended comment:

// Wrap poolCleared in a driver.Error so we can add the
// "TransientTransactionError" label. This will add
// "TransientTransactionError" to all poolClearedError instances, not
// just those that happened during transactions. While that behavior is
// different than other places we add "TransientTransactionError", it is
// consistent with the Transactions specification and simplifies the
// code.

}
p.stateMu.RUnlock()

duration := time.Since(start)
Expand Down
24 changes: 24 additions & 0 deletions x/mongo/driver/topology/pool_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"go.mongodb.org/mongo-driver/v2/internal/eventtest"
"go.mongodb.org/mongo-driver/v2/internal/require"
"go.mongodb.org/mongo-driver/v2/mongo/address"
"go.mongodb.org/mongo-driver/v2/x/mongo/driver"
"go.mongodb.org/mongo-driver/v2/x/mongo/driver/operation"
)

Expand Down Expand Up @@ -1584,3 +1585,26 @@ func TestPool_PoolMonitor(t *testing.T) {
"expected ConnectionCheckOutFailed Duration to be set")
})
}

func TestPool_Error(t *testing.T) {
t.Parallel()

t.Run("should have TransientTransactionError", func(t *testing.T) {
t.Parallel()

p := newPool(poolConfig{})
assert.Equalf(t, poolPaused, p.getState(), "expected new pool to be paused")

// Since new pool is paused, checkout should throw PoolClearedError.
_, err := p.checkOut(context.Background())
var le driver.Error
if errors.As(err, &le) {
assert.ErrorIs(t, poolClearedError{}, le.Unwrap(), "expect error to be PoolClearedError")
assert.True(t, le.HasErrorLabel(driver.TransientTransactionError), `expected error to include the "TransientTransactionError" label`)
} else {
t.Errorf("expected labeled error, got %v", err)
}

p.close(context.Background())
})
}
Loading