Deprecate isRetriableError and ensure isRetryableError functions (#1305)

Author: MaxAake

packages/bolt-connection/src/connection-provider/connection-provider-pooled.js

@@ -249,7 +249,8 @@ export default class PooledConnectionProvider extends ConnectionProvider {
     const handled = this._authenticationProvider.handleError({ connection, code: error.code })
 
     if (handled) {
-      error.retriable = true
+      error.retriable = true // remove in 7.0
+      error.retryable = true
     }
 
     if (error.code === 'Neo.ClientError.Security.AuthorizationExpired') {

packages/bolt-connection/test/connection-provider/connection-provider-direct.test.js

@@ -205,7 +205,7 @@ it('should call authenticationAuthProvider.handleError when TokenExpired happens
   expect(handleError).toBeCalledWith({ connection: conn, code: 'Neo.ClientError.Security.TokenExpired' })
 })
 
-it('should change error to retriable when error when TokenExpired happens and staticAuthTokenManager is not being used', async () => {
+it('should change error to retryable when error when TokenExpired happens and staticAuthTokenManager is not being used', async () => {
   const address = ServerAddress.fromUrl('localhost:123')
   const pool = newPool()
   const connectionProvider = newDirectConnectionProvider(address, pool, authTokenManagers.bearer({ tokenProvider: () => null }))
@@ -223,9 +223,10 @@ it('should change error to retriable when error when TokenExpired happens and st
   const error = conn.handleAndTransformError(expectedError, address)
 
   expect(error.retriable).toBe(true)
+  expect(error.retryable).toBe(true)
 })
 
-it('should not change error to retriable when error when TokenExpired happens and staticAuthTokenManager is being used', async () => {
+it('should not change error to retryable when error when TokenExpired happens and staticAuthTokenManager is being used', async () => {
   const address = ServerAddress.fromUrl('localhost:123')
   const pool = newPool()
   const connectionProvider = newDirectConnectionProvider(address, pool, staticAuthTokenManager({ authToken: null }))
@@ -243,9 +244,10 @@ it('should not change error to retriable when error when TokenExpired happens an
   const error = conn.handleAndTransformError(expectedError, address)
 
   expect(error.retriable).toBe(false)
+  expect(error.retryable).toBe(false)
 })
 
-it('should not change error to retriable when error when TokenExpired happens and authTokenManagers.basic is being used', async () => {
+it('should not change error to retryable when error when TokenExpired happens and authTokenManagers.basic is being used', async () => {
   const address = ServerAddress.fromUrl('localhost:123')
   const pool = newPool()
   const connectionProvider = newDirectConnectionProvider(address, pool, authTokenManagers.basic({ tokenProvider: () => null }))
@@ -263,6 +265,7 @@ it('should not change error to retriable when error when TokenExpired happens an
   const error = conn.handleAndTransformError(expectedError, address)
 
   expect(error.retriable).toBe(false)
+  expect(error.retryable).toBe(false)
 })
 
 describe('constructor', () => {

packages/bolt-connection/test/connection-provider/connection-provider-routing.test.js

@@ -1704,7 +1704,7 @@ describe.each([
     expect(error).toBe(expectedError)
   })
 
-  it.each(usersDataSet)('should change error to retriable when error when TokenExpired happens and staticAuthTokenManager is not being used [user=%s]', async (user) => {
+  it.each(usersDataSet)('should change error to retryable when error when TokenExpired happens and staticAuthTokenManager is not being used [user=%s]', async (user) => {
     const pool = newPool()
     const connectionProvider = newRoutingConnectionProvider(
       [
@@ -1741,10 +1741,12 @@ describe.each([
     const error2 = server2Connection.handleAndTransformError(error, server2)
 
     expect(error1.retriable).toBe(true)
+    expect(error1.retryable).toBe(true)
     expect(error2.retriable).toBe(true)
+    expect(error2.retryable).toBe(true)
   })
 
-  it.each(usersDataSet)('should not change error to retriable when error when TokenExpired happens and staticAuthTokenManager is being used [user=%s]', async (user) => {
+  it.each(usersDataSet)('should not change error to retryable when error when TokenExpired happens and staticAuthTokenManager is being used [user=%s]', async (user) => {
     const pool = newPool()
     const connectionProvider = newRoutingConnectionProvider(
       [
@@ -1782,10 +1784,12 @@ describe.each([
     const error2 = server2Connection.handleAndTransformError(error, server2)
 
     expect(error1.retriable).toBe(false)
+    expect(error1.retryable).toBe(false)
     expect(error2.retriable).toBe(false)
+    expect(error2.retryable).toBe(false)
   })
 
-  it.each(usersDataSet)('should not change error to retriable when error when TokenExpired happens and authTokenManagers.basic is being used [user=%s]', async (user) => {
+  it.each(usersDataSet)('should not change error to retryable when error when TokenExpired happens and authTokenManagers.basic is being used [user=%s]', async (user) => {
     const pool = newPool()
     const connectionProvider = newRoutingConnectionProvider(
       [
@@ -1823,7 +1827,9 @@ describe.each([
     const error2 = server2Connection.handleAndTransformError(error, server2)
 
     expect(error1.retriable).toBe(false)
+    expect(error1.retryable).toBe(false)
     expect(error2.retriable).toBe(false)
+    expect(error2.retryable).toBe(false)
   })
 
   it.each(usersDataSet)('should use resolved seed router after accepting table with no writers [user=%s]', (user, done) => {

packages/bolt-connection/test/connection/connection-channel.test.js

@@ -371,7 +371,7 @@ describe('ChannelConnection', () => {
         expect(loggerFunction).toHaveBeenCalledWith(
           'error',
           `${connection} experienced a fatal error caused by Neo4jError: some error ` +
-            '({"gqlStatus":"50N42","gqlStatusDescription":"error: general processing exception - unexpected error. some error","diagnosticRecord":{"OPERATION":"","OPERATION_CODE":"0","CURRENT_SCHEMA":"/"},"classification":"UNKNOWN","name":"Neo4jError","code":"C","retriable":false})'
+            '({"gqlStatus":"50N42","gqlStatusDescription":"error: general processing exception - unexpected error. some error","diagnosticRecord":{"OPERATION":"","OPERATION_CODE":"0","CURRENT_SCHEMA":"/"},"classification":"UNKNOWN","name":"Neo4jError","code":"C","retriable":false,"retryable":false})'
         )
       })
     })
@@ -419,7 +419,7 @@ describe('ChannelConnection', () => {
         expect(loggerFunction).toHaveBeenCalledWith(
           'error',
           `${connection} experienced a fatal error caused by Neo4jError: current failure ` +
-            '({"gqlStatus":"50N42","gqlStatusDescription":"error: general processing exception - unexpected error. current failure","diagnosticRecord":{"OPERATION":"","OPERATION_CODE":"0","CURRENT_SCHEMA":"/"},"classification":"UNKNOWN","name":"Neo4jError","code":"ongoing","retriable":false})'
+            '({"gqlStatus":"50N42","gqlStatusDescription":"error: general processing exception - unexpected error. current failure","diagnosticRecord":{"OPERATION":"","OPERATION_CODE":"0","CURRENT_SCHEMA":"/"},"classification":"UNKNOWN","name":"Neo4jError","code":"ongoing","retriable":false,"retryable":false})'
         )
       })
     })

packages/core/src/driver.ts

@@ -536,7 +536,7 @@ class Driver {
   }
 
   /**
-   * Executes a query in a retriable context and returns a {@link EagerResult}.
+   * Executes a query in a retryable context and returns a {@link EagerResult}.
    *
    * This method is a shortcut for a {@link Session#executeRead} and {@link Session#executeWrite}.
    *

packages/core/src/error.ts

@@ -174,9 +174,17 @@ class Neo4jError extends GQLError {
   /**
    * Whether the request that caused this error can be safely retried without duplicate commits on the server.
    * This does not apply when running auto-commit transactions using {@link Session#run}
+   *
+   * @deprecated members using the spelling 'retriable' will be removed in 7.0. Use {@link retryable} instead.
    */
   retriable: boolean
 
+  /**
+   * Whether the request that caused this error can be safely retried without duplicate commits on the server.
+   * This does not apply when running auto-commit transactions using {@link Session#run}
+   */
+  retryable: boolean
+
   /**
    * @constructor
    * @param {string} message - the error message
@@ -207,27 +215,49 @@ class Neo4jError extends GQLError {
      */
     this.name = 'Neo4jError'
 
+    const isRetryableCode = _isRetryableCode(code)
     /**
      * If the error is considered retriable.
      * This does not apply when running auto-commit transactions using {@link Session#run}
      *
+     * @deprecated members using the spelling 'retriable' will be removed in 7.0. Use {@link retryable} instead.
+     * @type {boolean}
+     * @public
+     */
+    this.retriable = isRetryableCode
+
+    /**
+     * If the error is considered retryable.
+     * This does not apply when running auto-commit transactions using {@link Session#run}
+     *
      * @type {boolean}
      * @public
      */
-    this.retriable = _isRetriableCode(code)
+    this.retryable = isRetryableCode
   }
 
   /**
    * Verifies if the given error is retriable.
    *
+   * @deprecated members using the spelling 'retriable' will be removed in 7.0. Use {@link isRetryable} instead.
    * @param {object|undefined|null} error the error object
    * @returns {boolean} true if the error is retriable
    */
   static isRetriable (error?: any | null): boolean {
+    return this.isRetryable(error)
+  }
+
+  /**
+   * Verifies if the given error is retryable.
+   *
+   * @param {object|undefined|null} error the error object
+   * @returns {boolean} true if the error is retryable
+   */
+  static isRetryable (error?: any | null): boolean {
     return error !== null &&
       error !== undefined &&
       error instanceof Neo4jError &&
-      error.retriable
+      error.retryable
   }
 }
 
@@ -263,18 +293,28 @@ function newGQLError (message: string, cause?: Error, gqlStatus?: string, gqlSta
 /**
  * Verifies if the given error is retriable.
  *
+ * @deprecated members using the spelling 'retriable' will be removed in 7.0. Use {@link isRetryableError} instead.
  * @public
  * @param {object|undefined|null} error the error object
  * @returns {boolean} true if the error is retriable
  */
 const isRetriableError = Neo4jError.isRetriable
 
+/**
+ * Verifies if the given error is retryable.
+ *
+ * @public
+ * @param {object|undefined|null} error the error object
+ * @returns {boolean} true if the error is retryable
+ */
+const isRetryableError = Neo4jError.isRetryable
+
 /**
  * @private
  * @param {string} code the error code
  * @returns {boolean} true if the error is a retriable error
  */
-function _isRetriableCode (code?: Neo4jErrorCode): boolean {
+function _isRetryableCode (code?: Neo4jErrorCode): boolean {
   return code === SERVICE_UNAVAILABLE ||
     code === SESSION_EXPIRED ||
     _isAuthorizationExpired(code) ||
@@ -313,6 +353,7 @@ export {
   newError,
   newGQLError,
   isRetriableError,
+  isRetryableError,
   Neo4jError,
   GQLError,
   SERVICE_UNAVAILABLE,

packages/core/src/index.ts

@@ -20,6 +20,7 @@ import {
   Neo4jError,
   newGQLError,
   GQLError,
+  isRetryableError,
   isRetriableError,
   PROTOCOL_ERROR,
   SERVICE_UNAVAILABLE,
@@ -121,6 +122,7 @@ const forExport = {
   Neo4jError,
   newGQLError,
   GQLError,
+  isRetryableError,
   isRetriableError,
   error,
   Integer,
@@ -196,6 +198,7 @@ export {
   Neo4jError,
   newGQLError,
   GQLError,
+  isRetryableError,
   isRetriableError,
   error,
   Integer,

packages/core/src/internal/transaction-executor.ts

@@ -15,7 +15,7 @@
  * limitations under the License.
  */
 /* eslint-disable @typescript-eslint/promise-function-async */
-import { newError, isRetriableError } from '../error'
+import { newError, isRetryableError } from '../error'
 import Transaction, { NonAutoCommitApiTelemetryConfig, NonAutoCommitTelemetryApis } from '../transaction'
 import TransactionPromise from '../transaction-promise'
 import { TELEMETRY_APIS } from './constants'
@@ -151,7 +151,7 @@ export class TransactionExecutor {
   ): Promise<T> {
     const elapsedTimeMs = Date.now() - retryStartTime
 
-    if (elapsedTimeMs > this._maxRetryTimeMs || !isRetriableError(error)) {
+    if (elapsedTimeMs > this._maxRetryTimeMs || !isRetryableError(error)) {
       return Promise.reject(error)
     }
 

packages/core/test/__snapshots__/json.test.ts.snap

@@ -102,11 +102,11 @@ exports[`json .stringify should handle object with custom toString in list 1`] =
 
 exports[`json .stringify should handle object with custom toString in object 1`] = `"{"key":{"identity":"1"}}"`;
 
-exports[`json .stringify should handle objects created with createBrokenObject 1`] = `"{"__isBrokenObject__":true,"__reason__":{"gqlStatus":"50N42","gqlStatusDescription":"error: general processing exception - unexpected error. some error","diagnosticRecord":{"OPERATION":"","OPERATION_CODE":"0","CURRENT_SCHEMA":"/"},"classification":"UNKNOWN","name":"Neo4jError","code":"N/A","retriable":false}}"`;
+exports[`json .stringify should handle objects created with createBrokenObject 1`] = `"{"__isBrokenObject__":true,"__reason__":{"gqlStatus":"50N42","gqlStatusDescription":"error: general processing exception - unexpected error. some error","diagnosticRecord":{"OPERATION":"","OPERATION_CODE":"0","CURRENT_SCHEMA":"/"},"classification":"UNKNOWN","name":"Neo4jError","code":"N/A","retriable":false,"retryable":false}}"`;
 
-exports[`json .stringify should handle objects created with createBrokenObject in list 1`] = `"[{"__isBrokenObject__":true,"__reason__":{"gqlStatus":"50N42","gqlStatusDescription":"error: general processing exception - unexpected error. some error","diagnosticRecord":{"OPERATION":"","OPERATION_CODE":"0","CURRENT_SCHEMA":"/"},"classification":"UNKNOWN","name":"Neo4jError","code":"N/A","retriable":false}}]"`;
+exports[`json .stringify should handle objects created with createBrokenObject in list 1`] = `"[{"__isBrokenObject__":true,"__reason__":{"gqlStatus":"50N42","gqlStatusDescription":"error: general processing exception - unexpected error. some error","diagnosticRecord":{"OPERATION":"","OPERATION_CODE":"0","CURRENT_SCHEMA":"/"},"classification":"UNKNOWN","name":"Neo4jError","code":"N/A","retriable":false,"retryable":false}}]"`;
 
-exports[`json .stringify should handle objects created with createBrokenObject inside other object 1`] = `"{"number":1,"broken":{"__isBrokenObject__":true,"__reason__":{"gqlStatus":"50N42","gqlStatusDescription":"error: general processing exception - unexpected error. some error","diagnosticRecord":{"OPERATION":"","OPERATION_CODE":"0","CURRENT_SCHEMA":"/"},"classification":"UNKNOWN","name":"Neo4jError","code":"N/A","retriable":false}}}"`;
+exports[`json .stringify should handle objects created with createBrokenObject inside other object 1`] = `"{"number":1,"broken":{"__isBrokenObject__":true,"__reason__":{"gqlStatus":"50N42","gqlStatusDescription":"error: general processing exception - unexpected error. some error","diagnosticRecord":{"OPERATION":"","OPERATION_CODE":"0","CURRENT_SCHEMA":"/"},"classification":"UNKNOWN","name":"Neo4jError","code":"N/A","retriable":false,"retryable":false}}}"`;
 
 exports[`json .stringify should handle string 1`] = `""my string""`;
 

packages/core/test/error.test.ts

@@ -16,7 +16,7 @@
  */
 import {
   Neo4jError,
-  isRetriableError,
+  isRetryableError,
   newError,
   newGQLError,
   PROTOCOL_ERROR,
@@ -123,13 +123,13 @@ describe('newError', () => {
   })
 })
 
-describe('isRetriableError()', () => {
-  it.each(getRetriableErrorsFixture())('should return true for error with code %s', error => {
-    expect(isRetriableError(error)).toBe(true)
+describe('isRetryableError()', () => {
+  it.each(getRetryableErrorsFixture())('should return true for error with code %s', error => {
+    expect(isRetryableError(error)).toBe(true)
   })
 
-  it.each(getNonRetriableErrorsFixture())('should return false for error with code %s', error => {
-    expect(isRetriableError(error)).toBe(false)
+  it.each(getNonRetryableErrorsFixture())('should return false for error with code %s', error => {
+    expect(isRetryableError(error)).toBe(false)
   })
 })
 
@@ -178,45 +178,47 @@ describe('Neo4jError', () => {
     expect(error.constructor).toEqual(Neo4jError)
   })
 
-  test.each(getRetriableCodes())('should define retriable as true for error with code %s', code => {
+  test.each(getRetryableCodes())('should define retryable as true for error with code %s', code => {
     const error = new Neo4jError('message', code, 'gqlStatus', 'gqlStatusDescription')
 
     expect(error.retriable).toBe(true)
+    expect(error.retryable).toBe(true)
   })
 
-  test.each(getNonRetriableCodes())('should define retriable as false for error with code %s', code => {
+  test.each(getNonRetryableCodes())('should define retryable as false for error with code %s', code => {
     const error = new Neo4jError('message', code, 'gqlStatus', 'gqlStatusDescription')
 
     expect(error.retriable).toBe(false)
+    expect(error.retryable).toBe(false)
   })
 
-  describe('.isRetriable()', () => {
-    it.each(getRetriableErrorsFixture())('should return true for error with code %s', error => {
-      expect(Neo4jError.isRetriable(error)).toBe(true)
+  describe('.isRetryable()', () => {
+    it.each(getRetryableErrorsFixture())('should return true for error with code %s', error => {
+      expect(Neo4jError.isRetryable(error)).toBe(true)
     })
 
-    it.each(getNonRetriableErrorsFixture())('should return false for error with code %s', error => {
-      expect(Neo4jError.isRetriable(error)).toBe(false)
+    it.each(getNonRetryableErrorsFixture())('should return false for error with code %s', error => {
+      expect(Neo4jError.isRetryable(error)).toBe(false)
     })
   })
 })
 
-function getRetriableErrorsFixture (): Array<[Neo4jError]> {
-  return getRetriableCodes().map(code => [newError('message', code)])
+function getRetryableErrorsFixture (): Array<[Neo4jError]> {
+  return getRetryableCodes().map(code => [newError('message', code)])
 }
 
-function getNonRetriableErrorsFixture (): any[] {
+function getNonRetryableErrorsFixture (): any[] {
   return [
     null,
     undefined,
     '',
     'Neo.TransientError.Transaction.DeadlockDetected',
     new Error('Neo.ClientError.Security.AuthorizationExpired'),
-    ...getNonRetriableCodes().map(code => [newError('message', code)])
+    ...getNonRetryableCodes().map(code => [newError('message', code)])
   ]
 }
 
-function getRetriableCodes (): string[] {
+function getRetryableCodes (): string[] {
   return [
     SERVICE_UNAVAILABLE,
     SESSION_EXPIRED,
@@ -228,7 +230,7 @@ function getRetriableCodes (): string[] {
   ]
 }
 
-function getNonRetriableCodes (): string[] {
+function getNonRetryableCodes (): string[] {
   return [
     'Neo.DatabaseError.General.UnknownError',
     'Neo.DatabaseError.General.OutOfMemoryError',