Skip to content

Add .retriable to the Neo4jError #901

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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})'
)
})
})
Expand Down Expand Up @@ -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})'
)
})
})
Expand Down
73 changes: 73 additions & 0 deletions packages/core/src/error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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

/**
Expand All @@ -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
}
}

Expand All @@ -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,
Expand Down
3 changes: 3 additions & 0 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import {
newError,
Neo4jError,
isRetriableError,
PROTOCOL_ERROR,
SERVICE_UNAVAILABLE,
SESSION_EXPIRED
Expand Down Expand Up @@ -93,6 +94,7 @@ const error = {
const forExport = {
newError,
Neo4jError,
isRetriableError,
error,
Integer,
int,
Expand Down Expand Up @@ -150,6 +152,7 @@ const forExport = {
export {
newError,
Neo4jError,
isRetriableError,
error,
Integer,
int,
Expand Down
4 changes: 1 addition & 3 deletions packages/core/src/internal/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -43,6 +42,5 @@ export {
logger,
urlUtil,
serverAddress,
resolver,
retryStrategy
resolver
}
63 changes: 0 additions & 63 deletions packages/core/src/internal/retry-strategy.ts

This file was deleted.

6 changes: 3 additions & 3 deletions packages/core/src/internal/transaction-executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -107,7 +107,7 @@ export class TransactionExecutor {
): Promise<T> {
const elapsedTimeMs = Date.now() - retryStartTime

if (elapsedTimeMs > this._maxRetryTimeMs || !canRetryOn(error)) {
if (elapsedTimeMs > this._maxRetryTimeMs || !isRetriableError(error)) {
return Promise.reject(error)
}

Expand Down
73 changes: 73 additions & 0 deletions packages/core/test/error.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
*/
import {
Neo4jError,
isRetriableError,
newError,
PROTOCOL_ERROR,
SERVICE_UNAVAILABLE,
Expand All @@ -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')
Expand Down Expand Up @@ -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'
]
}
3 changes: 3 additions & 0 deletions packages/neo4j-driver-lite/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { logging } from './logging'

import {
Neo4jError,
isRetriableError,
error,
Integer,
inSafeRange,
Expand Down Expand Up @@ -403,6 +404,7 @@ const forExport = {
isDateTime,
integer,
Neo4jError,
isRetriableError,
auth,
logging,
types,
Expand Down Expand Up @@ -453,6 +455,7 @@ export {
isDateTime,
integer,
Neo4jError,
isRetriableError,
auth,
logging,
types,
Expand Down
3 changes: 3 additions & 0 deletions packages/neo4j-driver/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import VERSION from './version'

import {
Neo4jError,
isRetryableError,
error,
Integer,
inSafeRange,
Expand Down Expand Up @@ -383,6 +384,7 @@ const forExport = {
isDateTime,
integer,
Neo4jError,
isRetryableError,
auth,
logging,
types,
Expand All @@ -405,6 +407,7 @@ export {
isDateTime,
integer,
Neo4jError,
isRetryableError,
auth,
logging,
types,
Expand Down
Loading