From 21c64d6d34129227170299c230ffba5c3080103f Mon Sep 17 00:00:00 2001
From: Stephen Cathcart <stephen.cathcart@neotechnology.com>
Date: Fri, 23 Feb 2024 13:32:32 +0000
Subject: [PATCH] Return error if operation is attempted on a closed session
 (#570)

* return error if an operation is attempted on closed session
---
 neo4j/session_with_context.go      | 26 +++++++++++++++++++
 neo4j/session_with_context_test.go | 40 ++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+)

diff --git a/neo4j/session_with_context.go b/neo4j/session_with_context.go
index 18b42b22..aebc5e6d 100644
--- a/neo4j/session_with_context.go
+++ b/neo4j/session_with_context.go
@@ -206,6 +206,7 @@ type sessionWithContext struct {
 	fetchSize     int
 	config        SessionConfig
 	auth          *idb.ReAuthToken
+	closed        bool
 }
 
 func newSessionWithContext(
@@ -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"}
@@ -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"}
@@ -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)
@@ -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)
@@ -676,6 +701,7 @@ func (s *sessionWithContext) Close(ctx context.Context) error {
 	}()
 	<-poolCleanUpChan
 	<-routerCleanUpChan
+	s.closed = true
 	return txErr
 }
 
diff --git a/neo4j/session_with_context_test.go b/neo4j/session_with_context_test.go
index 108cd977..156f3de2 100644
--- a/neo4j/session_with_context_test.go
+++ b/neo4j/session_with_context_test.go
@@ -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)
+		})
 	})
 
 }