Skip to content

Commit

Permalink
Return error if operation is attempted on a closed session (#570)
Browse files Browse the repository at this point in the history
* return error if an operation is attempted on closed session
  • Loading branch information
StephenCathcart authored Feb 23, 2024
1 parent ef1b68f commit 21c64d6
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 0 deletions.
26 changes: 26 additions & 0 deletions neo4j/session_with_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ type sessionWithContext struct {
fetchSize int
config SessionConfig
auth *idb.ReAuthToken
closed bool
}

func newSessionWithContext(
Expand Down Expand Up @@ -276,6 +277,13 @@ func (s *sessionWithContext) LastBookmarks() Bookmarks {
}

func (s *sessionWithContext) BeginTransaction(ctx context.Context, configurers ...func(*TransactionConfig)) (ExplicitTransaction, error) {

if s.closed {
err := &UsageError{Message: "Operation attempted on a closed session"}
s.log.Error(log.Session, s.logId, err)
return nil, err
}

// Guard for more than one transaction per session
if s.explicitTx != nil {
err := &UsageError{Message: "Session already has a pending transaction"}
Expand Down Expand Up @@ -388,6 +396,12 @@ func (s *sessionWithContext) runRetriable(
api telemetry.API,
configurers ...func(*TransactionConfig)) (any, error) {

if s.closed {
err := &UsageError{Message: "Operation attempted on a closed session"}
s.log.Error(log.Session, s.logId, err)
return nil, err
}

// Guard for more than one transaction per session
if s.explicitTx != nil {
err := &UsageError{Message: "Session already has a pending transaction"}
Expand Down Expand Up @@ -580,6 +594,12 @@ func (s *sessionWithContext) retrieveSessionBookmarks(conn idb.Connection) {
func (s *sessionWithContext) Run(ctx context.Context,
cypher string, params map[string]any, configurers ...func(*TransactionConfig)) (ResultWithContext, error) {

if s.closed {
err := &UsageError{Message: "Operation attempted on a closed session"}
s.log.Error(log.Session, s.logId, err)
return nil, err
}

if s.explicitTx != nil {
err := &UsageError{Message: "Trying to run auto-commit transaction while in explicit transaction"}
s.log.Error(log.Session, s.logId, err)
Expand Down Expand Up @@ -654,6 +674,11 @@ func (s *sessionWithContext) Run(ctx context.Context,
}

func (s *sessionWithContext) Close(ctx context.Context) error {
if s.closed {
// Safeguard against closing more than once
return nil
}

var txErr error
if s.explicitTx != nil {
txErr = s.explicitTx.Close(ctx)
Expand All @@ -676,6 +701,7 @@ func (s *sessionWithContext) Close(ctx context.Context) error {
}()
<-poolCleanUpChan
<-routerCleanUpChan
s.closed = true
return txErr
}

Expand Down
40 changes: 40 additions & 0 deletions neo4j/session_with_context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -736,6 +736,46 @@ func TestSession(outer *testing.T) {
AssertErrorMessageContains(t, err, "cannot use this transaction")
AssertIntEqual(t, poolReturnsCalls, 1) // pool.Return must not be called again
})

ct.Run("Run returns error if session is closed", func(t *testing.T) {
_, _, sess := createSession()
sess.Close(context.Background())
_, err := sess.Run(context.Background(), "cypher", nil)
AssertErrorMessageContains(t, err, "Operation attempted on a closed session")
})

ct.Run("BeginTransaction returns error if session is closed", func(t *testing.T) {
_, _, sess := createSession()
sess.Close(context.Background())
_, err := sess.BeginTransaction(context.Background())
AssertErrorMessageContains(t, err, "Operation attempted on a closed session")
})

ct.Run("ExecuteRead returns error if session is closed", func(t *testing.T) {
_, _, sess := createSession()
sess.Close(context.Background())
_, err := sess.ExecuteRead(context.Background(), func(tx ManagedTransaction) (any, error) {
return nil, nil
})
AssertErrorMessageContains(t, err, "Operation attempted on a closed session")
})

ct.Run("ExecuteWrite returns error if session is closed", func(t *testing.T) {
_, _, sess := createSession()
sess.Close(context.Background())
_, err := sess.ExecuteWrite(context.Background(), func(tx ManagedTransaction) (any, error) {
return nil, nil
})
AssertErrorMessageContains(t, err, "Operation attempted on a closed session")
})

ct.Run("Session returns nil if closed multiple times", func(t *testing.T) {
_, _, sess := createSession()
err := sess.Close(context.Background())
AssertNoError(t, err)
err = sess.Close(context.Background())
AssertNoError(t, err)
})
})

}
Expand Down

0 comments on commit 21c64d6

Please sign in to comment.