diff --git a/packages/bolt-connection/test/connection/connection-channel.test.js b/packages/bolt-connection/test/connection/connection-channel.test.js index f5ffbe46d..95f8ee61b 100644 --- a/packages/bolt-connection/test/connection/connection-channel.test.js +++ b/packages/bolt-connection/test/connection/connection-channel.test.js @@ -169,7 +169,7 @@ describe('ChannelConnection', () => { expect(loggerFunction).toHaveBeenCalledWith( 'error', `${connection} experienced a fatal error caused by Neo4jError: some error ` + - '({"code":"C","name":"Neo4jError"})' + '({"code":"C","name":"Neo4jError","retriable":false})' ) }) }) @@ -217,7 +217,7 @@ describe('ChannelConnection', () => { expect(loggerFunction).toHaveBeenCalledWith( 'error', `${connection} experienced a fatal error caused by Neo4jError: current failure ` + - '({"code":"ongoing","name":"Neo4jError"})' + '({"code":"ongoing","name":"Neo4jError","retriable":false})' ) }) }) diff --git a/packages/core/src/error.ts b/packages/core/src/error.ts index 6609c9a30..dcec636af 100644 --- a/packages/core/src/error.ts +++ b/packages/core/src/error.ts @@ -62,6 +62,7 @@ class Neo4jError extends Error { * Optional error code. Will be populated when error originates in the database. */ code: Neo4jErrorCode + retriable: boolean __proto__: Neo4jError /** @@ -76,6 +77,24 @@ class Neo4jError extends Error { this.__proto__ = Neo4jError.prototype this.code = code this.name = 'Neo4jError' + /** + * Indicates if the error is retriable. + * @type {boolean} - true if the error is retriable + */ + this.retriable = _isRetriableCode(code) + } + + /** + * Verifies if the given error is retriable. + * + * @param {object|undefined|null} error the error object + * @returns {boolean} true if the error is retriable + */ + static isRetriable(error?: any | null): boolean { + return error !== null && + error !== undefined && + error instanceof Neo4jError && + error.retriable } } @@ -90,8 +109,62 @@ function newError (message: string, code?: Neo4jErrorCode): Neo4jError { return new Neo4jError(message, code ?? NOT_AVAILABLE) } +/** + * Verifies if the given error is retriable. + * + * @public + * @param {object|undefined|null} error the error object + * @returns {boolean} true if the error is retriable + */ + const isRetriableError = Neo4jError.isRetriable + +/** + * @private + * @param {string} code the error code + * @returns {boolean} true if the error is a retriable error + */ +function _isRetriableCode (code?: Neo4jErrorCode): boolean { + return code === SERVICE_UNAVAILABLE || + code === SESSION_EXPIRED || + _isAuthorizationExpired(code) || + _isRetriableTransientError(code) +} + +/** + * @private + * @param {string} code the error to check + * @return {boolean} true if the error is a transient error + */ +function _isRetriableTransientError (code?: Neo4jErrorCode): 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. + + if (code !== undefined && code.indexOf('TransientError') >= 0) { + if ( + code === 'Neo.TransientError.Transaction.Terminated' || + code === 'Neo.TransientError.Transaction.LockClientStopped' + ) { + return false + } + return true + } + return false +} + +/** + * @private + * @param {string} code the error to check + * @returns {boolean} true if the error is a service unavailable error + */ +function _isAuthorizationExpired (code?: Neo4jErrorCode): boolean { + return code === 'Neo.ClientError.Security.AuthorizationExpired' +} + export { newError, + isRetriableError, Neo4jError, SERVICE_UNAVAILABLE, SESSION_EXPIRED, diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 0310319cb..81e925d6d 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -20,6 +20,7 @@ import { newError, Neo4jError, + isRetriableError, PROTOCOL_ERROR, SERVICE_UNAVAILABLE, SESSION_EXPIRED @@ -93,6 +94,7 @@ const error = { const forExport = { newError, Neo4jError, + isRetriableError, error, Integer, int, @@ -150,6 +152,7 @@ const forExport = { export { newError, Neo4jError, + isRetriableError, error, Integer, int, diff --git a/packages/core/src/internal/index.ts b/packages/core/src/internal/index.ts index fc0762681..00b91f965 100644 --- a/packages/core/src/internal/index.ts +++ b/packages/core/src/internal/index.ts @@ -29,7 +29,6 @@ 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, @@ -43,6 +42,5 @@ export { logger, urlUtil, serverAddress, - resolver, - retryStrategy + resolver } diff --git a/packages/core/src/internal/retry-strategy.ts b/packages/core/src/internal/retry-strategy.ts deleted file mode 100644 index df803f264..000000000 --- a/packages/core/src/internal/retry-strategy.ts +++ /dev/null @@ -1,63 +0,0 @@ -/** - * 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/packages/core/src/internal/transaction-executor.ts b/packages/core/src/internal/transaction-executor.ts index 47d2ff3f0..0b849ddd2 100644 --- a/packages/core/src/internal/transaction-executor.ts +++ b/packages/core/src/internal/transaction-executor.ts @@ -17,10 +17,10 @@ * limitations under the License. */ -import { newError } from '../error' +import { newError, isRetriableError } from '../error' import Transaction from '../transaction' import TransactionPromise from '../transaction-promise' -import { canRetryOn } from './retry-strategy' + const DEFAULT_MAX_RETRY_TIME_MS = 30 * 1000 // 30 seconds const DEFAULT_INITIAL_RETRY_DELAY_MS = 1000 // 1 seconds @@ -107,7 +107,7 @@ export class TransactionExecutor { ): Promise { const elapsedTimeMs = Date.now() - retryStartTime - if (elapsedTimeMs > this._maxRetryTimeMs || !canRetryOn(error)) { + if (elapsedTimeMs > this._maxRetryTimeMs || !isRetriableError(error)) { return Promise.reject(error) } diff --git a/packages/core/test/error.test.ts b/packages/core/test/error.test.ts index 7cbfe0949..36b2ba204 100644 --- a/packages/core/test/error.test.ts +++ b/packages/core/test/error.test.ts @@ -18,6 +18,7 @@ */ import { Neo4jError, + isRetriableError, newError, PROTOCOL_ERROR, SERVICE_UNAVAILABLE, @@ -44,6 +45,18 @@ describe('newError', () => { }) }) +describe('isRetriableError()', () => { + it.each(getRetriableErrorsFixture()) + ('should return true for error with code %s', error => { + expect(isRetriableError(error)).toBe(true) + }) + + it.each(getNonRetriableErrorsFixture()) + ('should return false for error with code %s', error => { + expect(isRetriableError(error)).toBe(false) + }) +}) + describe('Neo4jError', () => { test('should have message', () => { const error = new Neo4jError('message', 'code') @@ -76,4 +89,64 @@ describe('Neo4jError', () => { expect(error.__proto__).toEqual(Neo4jError.prototype) expect(error.constructor).toEqual(Neo4jError) }) + + test.each(getRetriableCodes()) + ('should define retriable as true for error with code %s', code => { + const error = new Neo4jError('message', code) + + expect(error.retriable).toBe(true) + }) + + test.each(getNonRetriableCodes()) + ('should define retriable as false for error with code %s', code => { + const error = new Neo4jError('message', code) + + expect(error.retriable).toBe(false) + }) + + describe('.isRetriable()', () => { + it.each(getRetriableErrorsFixture()) + ('should return true for error with code %s', error => { + expect(Neo4jError.isRetriable(error)).toBe(true) + }) + + it.each(getNonRetriableErrorsFixture()) + ('should return false for error with code %s', error => { + expect(Neo4jError.isRetriable(error)).toBe(false) + }) + }) }) + +function getRetriableErrorsFixture () { + return getRetriableCodes().map(code => [newError('message', code)]) +} + +function getNonRetriableErrorsFixture () { + return [ + null, + undefined, + '', + 'Neo.TransientError.Transaction.DeadlockDetected', + new Error('Neo.ClientError.Security.AuthorizationExpired'), + ...getNonRetriableCodes().map(code => [newError('message', code)]) + ] +} + +function getRetriableCodes () { + return [ + SERVICE_UNAVAILABLE, + SESSION_EXPIRED, + 'Neo.ClientError.Security.AuthorizationExpired', + 'Neo.TransientError.Transaction.DeadlockDetected', + 'Neo.TransientError.Network.CommunicationError' + ] +} + +function getNonRetriableCodes () { + return [ + 'Neo.TransientError.Transaction.Terminated', + 'Neo.DatabaseError.General.UnknownError', + 'Neo.TransientError.Transaction.LockClientStopped', + 'Neo.DatabaseError.General.OutOfMemoryError' + ] +} diff --git a/packages/neo4j-driver-lite/src/index.ts b/packages/neo4j-driver-lite/src/index.ts index 1614a0875..5f28c6b4f 100644 --- a/packages/neo4j-driver-lite/src/index.ts +++ b/packages/neo4j-driver-lite/src/index.ts @@ -21,6 +21,7 @@ import { logging } from './logging' import { Neo4jError, + isRetriableError, error, Integer, inSafeRange, @@ -403,6 +404,7 @@ const forExport = { isDateTime, integer, Neo4jError, + isRetriableError, auth, logging, types, @@ -453,6 +455,7 @@ export { isDateTime, integer, Neo4jError, + isRetriableError, auth, logging, types, diff --git a/packages/neo4j-driver/src/index.js b/packages/neo4j-driver/src/index.js index 62d01bdbe..f8bb7f809 100644 --- a/packages/neo4j-driver/src/index.js +++ b/packages/neo4j-driver/src/index.js @@ -21,6 +21,7 @@ import VERSION from './version' import { Neo4jError, + isRetryableError, error, Integer, inSafeRange, @@ -383,6 +384,7 @@ const forExport = { isDateTime, integer, Neo4jError, + isRetryableError, auth, logging, types, @@ -405,6 +407,7 @@ export { isDateTime, integer, Neo4jError, + isRetryableError, auth, logging, types, diff --git a/packages/neo4j-driver/src/internal/retry-logic-rx.js b/packages/neo4j-driver/src/internal/retry-logic-rx.js index 08ede599e..2ff7968e2 100644 --- a/packages/neo4j-driver/src/internal/retry-logic-rx.js +++ b/packages/neo4j-driver/src/internal/retry-logic-rx.js @@ -17,7 +17,7 @@ * limitations under the License. */ -import { newError, error, internal } from 'neo4j-driver-core' +import { newError, error, internal, isRetriableError } from 'neo4j-driver-core' import { Observable, throwError, of } from 'rxjs' import { retryWhen, flatMap, delay } from 'rxjs/operators' @@ -25,8 +25,7 @@ const { logger: { // eslint-disable-next-line no-unused-vars Logger - }, - retryStrategy: { canRetryOn } + } } = internal const { SERVICE_UNAVAILABLE, SESSION_EXPIRED } = error @@ -81,7 +80,7 @@ export default class RxRetryLogic { return failedWork.pipe( flatMap(err => { - if (!canRetryOn(err)) { + if (!isRetriableError(err)) { return throwError(err) } diff --git a/packages/neo4j-driver/types/index.d.ts b/packages/neo4j-driver/types/index.d.ts index 265ae9686..12b66128d 100644 --- a/packages/neo4j-driver/types/index.d.ts +++ b/packages/neo4j-driver/types/index.d.ts @@ -19,6 +19,7 @@ import { Neo4jError, + isRetriableError, error, Integer, inSafeRange, @@ -174,6 +175,7 @@ declare const forExport: { TrustStrategy: TrustStrategy SessionMode: SessionMode Neo4jError: Neo4jError + isRetriableError: typeof isRetriableError Node: Node Relationship: Relationship UnboundRelationship: UnboundRelationship @@ -234,6 +236,7 @@ export { TrustStrategy, SessionMode, Neo4jError, + isRetriableError, Node, Relationship, UnboundRelationship,