Do not retry transaction when it is terminated

This commit makes transaction functions `Session.readTransaction()` and
`Session#writeTransaction()` not retry when executed transaction was
explicitly terminated by the user. Such termination can happen when
`Session#close()` is called or `killQuery` procedure is used.

Transaction termination can result in two different status codes on the
server, both classified as transient errors which is not entirely correct.
So we need additional status code checks in the retying code.

Author: lutovich

src/v1/internal/transaction-executor.js

@@ -122,7 +122,23 @@ export default class TransactionExecutor {
     return error && error.code &&
       (error.code === SERVICE_UNAVAILABLE ||
       error.code === SESSION_EXPIRED ||
-      error.code.indexOf('TransientError') >= 0);
+      this._isTransientError(error));
+  }
+
+  static _isTransientError(error) {
+    // Retries should not happen when transaction was explicitly terminated by the user.
+    // Termination of transaction might result in two different error codes depending on where it was
+    // terminated. These are really client errors but classification on the server is not entirely correct and
+    // they are classified as transient.
+    
+    const code = error.code;
+    if (code.indexOf('TransientError') >= 0) {
+      if (code === 'Neo.TransientError.Transaction.Terminated' || code === 'Neo.TransientError.Transaction.LockClientStopped') {
+        return false;
+      }
+      return true;
+    }
+    return false;
   }
 
   _verifyAfterConstruction() {

test/internal/transaction-executor.test.js

@@ -24,6 +24,8 @@ import {hijackNextDateNowCall, setTimeoutMock} from './timers-util';
 const TRANSIENT_ERROR_1 = 'Neo.TransientError.Transaction.DeadlockDetected';
 const TRANSIENT_ERROR_2 = 'Neo.TransientError.Network.CommunicationError';
 const UNKNOWN_ERROR = 'Neo.DatabaseError.General.UnknownError';
+const TX_TERMINATED_ERROR = 'Neo.TransientError.Transaction.Terminated';
+const LOCKS_TERMINATED_ERROR = 'Neo.TransientError.Transaction.LockClientStopped';
 const OOM_ERROR = 'Neo.DatabaseError.General.OutOfMemoryError';
 
 describe('TransactionExecutor', () => {
@@ -62,6 +64,14 @@ describe('TransactionExecutor', () => {
     testNoRetryOnUnknownError([UNKNOWN_ERROR], 1, done);
   });
 
+  it('should not retry when transaction work returns promise rejected with transaction termination error', done => {
+    testNoRetryOnUnknownError([TX_TERMINATED_ERROR], 1, done);
+  });
+
+  it('should not retry when transaction work returns promise rejected with locks termination error', done => {
+    testNoRetryOnUnknownError([LOCKS_TERMINATED_ERROR], 1, done);
+  });
+
   it('should stop retrying when time expires', done => {
     const executor = new TransactionExecutor();
     let workInvocationCounter = 0;

test/v1/session.test.js

@@ -796,6 +796,120 @@ describe('session', () => {
     });
   });
 
+  it('should interrupt query waiting on a lock when closed', done => {
+    if (!serverIs31OrLater(done)) {
+      // locks are transaction termination aware by default only in 3.1+
+      return;
+    }
+
+    session.run('CREATE ()').then(() => {
+      session.close(() => {
+        const session1 = driver.session();
+        const session2 = driver.session();
+        const tx1 = session1.beginTransaction();
+
+        tx1.run('MATCH (n) SET n.id = 1 RETURN 42 AS answer').then(result => {
+          // node is now locked by tx1
+          expect(result.records.length).toEqual(1);
+          expect(result.records[0].get(0).toNumber()).toEqual(42);
+
+          // this query should get stuck waiting for the lock
+          session2.run('MATCH (n) SET n.id = 2 RETURN 42 AS answer').catch(error => {
+            expectTransactionTerminatedError(error);
+            tx1.commit().then(() => {
+              readAllNodeIds().then(ids => {
+                expect(ids).toEqual([1]);
+                done();
+              });
+            });
+          });
+
+          setTimeout(() => {
+            // close session after a while
+            session2.close();
+          }, 1000);
+        });
+      });
+    });
+  });
+
+  it('should interrupt transaction waiting on a lock when closed', done => {
+    if (!serverIs31OrLater(done)) {
+      // locks are transaction termination aware by default only in 3.1+
+      return;
+    }
+
+    session.run('CREATE ()').then(() => {
+      session.close(() => {
+        const session1 = driver.session();
+        const session2 = driver.session();
+        const tx1 = session1.beginTransaction();
+        const tx2 = session2.beginTransaction();
+
+        tx1.run('MATCH (n) SET n.id = 1 RETURN 42 AS answer').then(result => {
+          // node is now locked by tx1
+          expect(result.records.length).toEqual(1);
+          expect(result.records[0].get(0).toNumber()).toEqual(42);
+
+          // this query should get stuck waiting for the lock
+          tx2.run('MATCH (n) SET n.id = 2 RETURN 42 AS answer').catch(error => {
+            expectTransactionTerminatedError(error);
+            tx1.commit().then(() => {
+              readAllNodeIds().then(ids => {
+                expect(ids).toEqual([1]);
+                done();
+              });
+            });
+          });
+
+          setTimeout(() => {
+            // close session after a while
+            session2.close();
+          }, 1000);
+        });
+      });
+    });
+  });
+
+  it('should interrupt transaction function waiting on a lock when closed', done => {
+    if (!serverIs31OrLater(done)) {
+      // locks are transaction termination aware by default only in 3.1+
+      return;
+    }
+
+    session.run('CREATE ()').then(() => {
+      session.close(() => {
+        const session1 = driver.session();
+        const session2 = driver.session();
+        const tx1 = session1.beginTransaction();
+
+        tx1.run('MATCH (n) SET n.id = 1 RETURN 42 AS answer').then(result => {
+          // node is now locked by tx1
+          expect(result.records.length).toEqual(1);
+          expect(result.records[0].get(0).toNumber()).toEqual(42);
+
+          session2.writeTransaction(tx2 => {
+            // this query should get stuck waiting for the lock
+            return tx2.run('MATCH (n) SET n.id = 2 RETURN 42 AS answer').catch(error => {
+              expectTransactionTerminatedError(error);
+              tx1.commit().then(() => {
+                readAllNodeIds().then(ids => {
+                  expect(ids).toEqual([1]);
+                  done();
+                });
+              });
+            });
+          });
+
+          setTimeout(() => {
+            // close session after a while
+            session2.close();
+          }, 1000);
+        });
+      });
+    });
+  });
+
   function serverIs31OrLater(done) {
     // lazy way of checking the version number
     // if server has been set we know it is at least 3.1
@@ -840,4 +954,21 @@ describe('session', () => {
     expect(bookmark).toBeDefined();
     expect(bookmark).not.toBeNull();
   }
+
+  function expectTransactionTerminatedError(error) {
+    expect(error.message.toLowerCase().indexOf('transaction terminated') >= 0).toBeTruthy();
+  }
+
+  function readAllNodeIds() {
+    return new Promise((resolve, reject) => {
+      const session = driver.session();
+      session.run('MATCH (n) RETURN n.id').then(result => {
+        const ids = result.records.map(record => record.get(0).toNumber());
+        session.close();
+        resolve(ids);
+      }).catch(error => {
+        reject(error);
+      });
+    });
+  }
 });