Transaction functions should retry on AuthorizationExpired error (#730)

`Neo.ClientError.Security.AuthorizationExpired` is a transient error should be object of retry in transaction function scenarios since it could be resolve by get another connection.

Author: bigmontz

core/src/internal/index.ts

@@ -30,6 +30,7 @@ import * as logger from './logger'
 import * as urlUtil from './url-util'
 import * as serverAddress from './server-address'
 import * as resolver from './resolver'
+import * as retryStrategy from './retry-strategy'
 
 export {
   util,
@@ -44,5 +45,6 @@ export {
   logger,
   urlUtil,
   serverAddress,
-  resolver
+  resolver,
+  retryStrategy
 }

core/src/internal/retry-strategy.ts

@@ -0,0 +1,63 @@
+/**
+ * Copyright (c) "Neo4j"
+ * Neo4j Sweden AB [http://neo4j.com]
+ *
+ * This file is part of Neo4j.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { Neo4jError, SERVICE_UNAVAILABLE, SESSION_EXPIRED } from '../error'
+
+/**
+ * Verified error and returns if it could be retried or not
+ *
+ * @param _error The error
+ * @returns If the transaction could be retried.
+ */
+function canRetryOn (_error: any): boolean {
+  return (
+    _error &&
+    _error instanceof Neo4jError &&
+    _error.code &&
+    (_error.code === SERVICE_UNAVAILABLE ||
+      _error.code === SESSION_EXPIRED ||
+      _isAuthorizationExpired(_error) ||
+      _isTransientError(_error))
+  )
+}
+
+function _isTransientError (error: Neo4jError): boolean {
+  // 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
+}
+
+function _isAuthorizationExpired (error: Neo4jError): boolean {
+  return error.code === 'Neo.ClientError.Security.AuthorizationExpired'
+}
+
+export { canRetryOn }

core/src/internal/transaction-executor.ts

@@ -17,13 +17,9 @@
  * limitations under the License.
  */
 
-import {
-  newError,
-  Neo4jError,
-  SERVICE_UNAVAILABLE,
-  SESSION_EXPIRED
-} from '../error'
+import { newError } from '../error'
 import Transaction from '../transaction'
+import { canRetryOn } from './retry-strategy'
 
 const DEFAULT_MAX_RETRY_TIME_MS = 30 * 1000 // 30 seconds
 const DEFAULT_INITIAL_RETRY_DELAY_MS = 1000 // 1 seconds
@@ -109,10 +105,7 @@ export class TransactionExecutor {
   ): Promise<T> {
     const elapsedTimeMs = Date.now() - retryStartTime
 
-    if (
-      elapsedTimeMs > this._maxRetryTimeMs ||
-      !TransactionExecutor._canRetryOn(error)
-    ) {
+    if (elapsedTimeMs > this._maxRetryTimeMs || !canRetryOn(error)) {
       return Promise.reject(error)
     }
 
@@ -229,34 +222,6 @@ export class TransactionExecutor {
     return Math.random() * (max - min) + min
   }
 
-  static _canRetryOn(error: any): boolean {
-    return (error &&
-      error instanceof Neo4jError &&
-      error.code &&
-      (error.code === SERVICE_UNAVAILABLE ||
-        error.code === SESSION_EXPIRED ||
-        this._isTransientError(error))) as boolean
-  }
-
-  static _isTransientError(error: Neo4jError): boolean {
-    // 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() {
     if (this._maxRetryTimeMs < 0) {
       throw newError('Max retry time should be >= 0: ' + this._maxRetryTimeMs)

src/internal/retry-logic-rx.js

@@ -25,7 +25,8 @@ const {
   logger: {
     // eslint-disable-next-line no-unused-vars
     Logger
-  }
+  },
+  retryStrategy: { canRetryOn }
 } = internal
 
 const { SERVICE_UNAVAILABLE, SESSION_EXPIRED } = error
@@ -80,7 +81,7 @@ export default class RxRetryLogic {
 
         return failedWork.pipe(
           flatMap(err => {
-            if (!RxRetryLogic._canRetryOn(err)) {
+            if (!canRetryOn(err)) {
               return throwError(err)
             }
 
@@ -119,35 +120,6 @@ export default class RxRetryLogic {
     const jitter = delay * this._delayJitter
     return delay - jitter + 2 * jitter * Math.random()
   }
-
-  static _canRetryOn (error) {
-    return (
-      error &&
-      error.code &&
-      (error.code === SERVICE_UNAVAILABLE ||
-        error.code === SESSION_EXPIRED ||
-        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
-  }
 }
 
 function valueOrDefault (value, defaultValue) {

test/internal/retry-logic-rx.test.js

@@ -109,6 +109,15 @@ describe('#unit-rx retrylogic', () => {
       verifyRetry(newError('service unavailable', SERVICE_UNAVAILABLE))
     })
 
+    it('a Neo.ClientError.Security.AuthorizationExpired error', () => {
+      verifyRetry(
+        newError(
+          'service unavailable',
+          'Neo.ClientError.Security.AuthorizationExpired'
+        )
+      )
+    })
+
     function verifyRetry (error) {
       scheduler.run(helpers => {
         const retryLogic = new RxRetryLogic({ maxRetryTimeout: 5000 })

test/internal/transaction-executor.test.js

@@ -190,7 +190,8 @@ describe('#unit TransactionExecutor', () => {
       TRANSIENT_ERROR_2,
       SESSION_EXPIRED,
       SERVICE_UNAVAILABLE,
-      TRANSIENT_ERROR_1
+      TRANSIENT_ERROR_1,
+      'Neo.ClientError.Security.AuthorizationExpired'
     ])
   }, 30000)
 
@@ -203,7 +204,8 @@ describe('#unit TransactionExecutor', () => {
       SERVICE_UNAVAILABLE,
       TRANSIENT_ERROR_2,
       TRANSIENT_ERROR_2,
-      SESSION_EXPIRED
+      SESSION_EXPIRED,
+      'Neo.ClientError.Security.AuthorizationExpired'
     ])
   }, 30000)
 
@@ -214,7 +216,8 @@ describe('#unit TransactionExecutor', () => {
       TRANSIENT_ERROR_2,
       SESSION_EXPIRED,
       TRANSIENT_ERROR_1,
-      SESSION_EXPIRED
+      SESSION_EXPIRED,
+      'Neo.ClientError.Security.AuthorizationExpired'
     ])
   }, 30000)
 
@@ -230,7 +233,8 @@ describe('#unit TransactionExecutor', () => {
       TRANSIENT_ERROR_1,
       SESSION_EXPIRED,
       SERVICE_UNAVAILABLE,
-      TRANSIENT_ERROR_2
+      TRANSIENT_ERROR_2,
+      'Neo.ClientError.Security.AuthorizationExpired'
     ])
   }, 30000)
 
@@ -239,6 +243,7 @@ describe('#unit TransactionExecutor', () => {
       [
         SERVICE_UNAVAILABLE,
         TRANSIENT_ERROR_2,
+        'Neo.ClientError.Security.AuthorizationExpired',
         SESSION_EXPIRED,
         SESSION_EXPIRED
       ],

testkit-backend/src/skipped-tests.js

@@ -33,7 +33,8 @@ const skippedTests = [
     ),
     ifEndsWith(
       'test_should_retry_write_until_success_with_leader_change_using_tx_function'
-    )
+    ),
+    ifEndsWith('test_should_retry_on_auth_expired_on_begin_using_tx_function')
   ),
   skip(
     'Driver is creating a dedicated connection to check the MultiDBSupport',