diff --git a/core/src/internal/index.ts b/core/src/internal/index.ts index fbea886f9..9e3150329 100644 --- a/core/src/internal/index.ts +++ b/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 } diff --git a/core/src/internal/retry-strategy.ts b/core/src/internal/retry-strategy.ts new file mode 100644 index 000000000..df803f264 --- /dev/null +++ b/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 } diff --git a/core/src/internal/transaction-executor.ts b/core/src/internal/transaction-executor.ts index a52b997b4..b8ea214df 100644 --- a/core/src/internal/transaction-executor.ts +++ b/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 { 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) diff --git a/src/internal/retry-logic-rx.js b/src/internal/retry-logic-rx.js index 27a0ab285..08ede599e 100644 --- a/src/internal/retry-logic-rx.js +++ b/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) { diff --git a/test/internal/retry-logic-rx.test.js b/test/internal/retry-logic-rx.test.js index d38ed6e3e..8752a2619 100644 --- a/test/internal/retry-logic-rx.test.js +++ b/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 }) diff --git a/test/internal/transaction-executor.test.js b/test/internal/transaction-executor.test.js index a11e5b6d0..54330cef7 100644 --- a/test/internal/transaction-executor.test.js +++ b/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 ], diff --git a/testkit-backend/src/skipped-tests.js b/testkit-backend/src/skipped-tests.js index ba7ae730d..de5a60583 100644 --- a/testkit-backend/src/skipped-tests.js +++ b/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',