From 0ebca4b2a7f79d4a8d145ffa381eee2c44d336b1 Mon Sep 17 00:00:00 2001 From: Antonio Barcelos Date: Thu, 9 Jun 2022 11:58:46 +0200 Subject: [PATCH 1/2] Reclassify `Transaction.LockClientStopped` and `Transaction.Terminated` as `ClientError` The server 5.x already fix this classification error, but the driver has re-classify errors from previous version of the server for a more convenience for the user. Co-authored-by: Florent Biville --- .../src/bolt/response-handler.js | 21 +++++- .../test/bolt/response-handler.test.js | 74 +++++++++++++++++++ packages/core/src/error.ts | 20 +---- packages/core/test/error.test.ts | 10 ++- 4 files changed, 103 insertions(+), 22 deletions(-) create mode 100644 packages/bolt-connection/test/bolt/response-handler.test.js diff --git a/packages/bolt-connection/src/bolt/response-handler.js b/packages/bolt-connection/src/bolt/response-handler.js index 25fa2dd58..53238c2a3 100644 --- a/packages/bolt-connection/src/bolt/response-handler.js +++ b/packages/bolt-connection/src/bolt/response-handler.js @@ -117,7 +117,8 @@ export default class ResponseHandler { this._log.debug(`S: FAILURE ${json.stringify(msg)}`) } try { - const error = newError(payload.message, payload.code) + const standardizedCode = _standardizeCode(payload.code) + const error = newError(payload.message, standardizedCode) this._currentFailure = this._observer.onErrorApplyTransformation( error ) @@ -194,3 +195,21 @@ export default class ResponseHandler { this._currentFailure = null } } + +/** + * Standardize error classification that are different between 5.x and previous versions. + * + * The transient error were clean-up for being retrieable and because of this + * `Terminated` and `LockClientStopped` were reclassified as `ClientError`. + * + * @param {string} code + * @returns {string} the standardized error code + */ +function _standardizeCode (code) { + if (code === 'Neo.TransientError.Transaction.Terminated') { + return 'Neo.ClientError.Transaction.Terminated' + } else if (code === 'Neo.TransientError.Transaction.LockClientStopped') { + return 'Neo.ClientError.Transaction.LockClientStopped' + } + return code +} diff --git a/packages/bolt-connection/test/bolt/response-handler.test.js b/packages/bolt-connection/test/bolt/response-handler.test.js new file mode 100644 index 000000000..ba9e525f4 --- /dev/null +++ b/packages/bolt-connection/test/bolt/response-handler.test.js @@ -0,0 +1,74 @@ +/** + * 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 ResponseHandler from '../../src/bolt/response-handler' +import { internal, newError } from 'neo4j-driver-core' + +const { + logger: { Logger } +} = internal + +const FAILURE = 0x7f // 0111 1111 // FAILURE + +describe('response-handler', () => { + describe('.handleResponse()', () => { + it.each([ + { + code: 'Neo.TransientError.Transaction.Terminated', + expectedErrorCode: 'Neo.ClientError.Transaction.Terminated' + }, + { + code: 'Neo.TransientError.Transaction.LockClientStopped', + expectedErrorCode: 'Neo.ClientError.Transaction.LockClientStopped' + }, + { + code: 'Neo.ClientError.Transaction.LockClientStopped', + expectedErrorCode: 'Neo.ClientError.Transaction.LockClientStopped' + }, + { + code: 'Neo.ClientError.Transaction.Terminated', + expectedErrorCode: 'Neo.ClientError.Transaction.Terminated' + }, + { + code: 'Neo.TransientError.Security.NotYourBusiness', + expectedErrorCode: 'Neo.TransientError.Security.NotYourBusiness' + } + ])('should fix wrong classified error codes', ({ code, expectedErrorCode }) => { + const observer = { + capturedErrors: [], + onFailure: error => observer.capturedErrors.push(error) + } + const message = 'Something gets wrong' + const expectedError = newError(message, expectedErrorCode) + const responseHandler = new ResponseHandler({ observer, log: Logger.noOp() }) + responseHandler._queueObserver({}) + + const errorMessage = { + signature: FAILURE, + fields: [{ message, code }] + } + responseHandler.handleResponse(errorMessage) + + expect(observer.capturedErrors.length).toBe(1) + const [receivedError] = observer.capturedErrors + expect(receivedError.message).toBe(expectedError.message) + expect(receivedError.code).toBe(expectedError.code) + }) + }) +}) diff --git a/packages/core/src/error.ts b/packages/core/src/error.ts index 7b565fee6..a2454be8f 100644 --- a/packages/core/src/error.ts +++ b/packages/core/src/error.ts @@ -129,7 +129,7 @@ function _isRetriableCode (code?: Neo4jErrorCode): boolean { return code === SERVICE_UNAVAILABLE || code === SESSION_EXPIRED || _isAuthorizationExpired(code) || - _isRetriableTransientError(code) + _isTransientError(code) } /** @@ -137,22 +137,8 @@ function _isRetriableCode (code?: Neo4jErrorCode): boolean { * @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?.includes('TransientError') === true) { - if ( - code === 'Neo.TransientError.Transaction.Terminated' || - code === 'Neo.TransientError.Transaction.LockClientStopped' - ) { - return false - } - return true - } - return false +function _isTransientError (code?: Neo4jErrorCode): boolean { + return code?.includes('TransientError') === true } /** diff --git a/packages/core/test/error.test.ts b/packages/core/test/error.test.ts index 535a21613..f613f4792 100644 --- a/packages/core/test/error.test.ts +++ b/packages/core/test/error.test.ts @@ -165,15 +165,17 @@ function getRetriableCodes (): string[] { SESSION_EXPIRED, 'Neo.ClientError.Security.AuthorizationExpired', 'Neo.TransientError.Transaction.DeadlockDetected', - 'Neo.TransientError.Network.CommunicationError' + 'Neo.TransientError.Network.CommunicationError', + 'Neo.TransientError.Transaction.Terminated', + 'Neo.TransientError.Transaction.LockClientStopped' ] } function getNonRetriableCodes (): string[] { return [ - 'Neo.TransientError.Transaction.Terminated', 'Neo.DatabaseError.General.UnknownError', - 'Neo.TransientError.Transaction.LockClientStopped', - 'Neo.DatabaseError.General.OutOfMemoryError' + 'Neo.DatabaseError.General.OutOfMemoryError', + 'Neo.ClientError.Transaction.Terminated', + 'Neo.ClientError.Transaction.LockClientStopped' ] } From f812ea47305d84c3831f80fd24f576c72deb8953 Mon Sep 17 00:00:00 2001 From: Antonio Barcelos Date: Fri, 9 Sep 2022 12:45:24 +0200 Subject: [PATCH 2/2] Adjust test --- packages/neo4j-driver/test/internal/retry-logic-rx.test.js | 4 ++-- .../neo4j-driver/test/internal/transaction-executor.test.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/neo4j-driver/test/internal/retry-logic-rx.test.js b/packages/neo4j-driver/test/internal/retry-logic-rx.test.js index eb2817ea0..d806d069e 100644 --- a/packages/neo4j-driver/test/internal/retry-logic-rx.test.js +++ b/packages/neo4j-driver/test/internal/retry-logic-rx.test.js @@ -65,7 +65,7 @@ describe('#unit-rx retrylogic', () => { verifyNoRetry( newError( 'transaction terminated', - 'Neo.TransientError.Transaction.Terminated' + 'Neo.ClientError.Transaction.Terminated' ) ) }) @@ -74,7 +74,7 @@ describe('#unit-rx retrylogic', () => { verifyNoRetry( newError( 'lock client stopped', - 'Neo.TransientError.Transaction.LockClientStopped' + 'Neo.ClientError.Transaction.LockClientStopped' ) ) }) diff --git a/packages/neo4j-driver/test/internal/transaction-executor.test.js b/packages/neo4j-driver/test/internal/transaction-executor.test.js index 98a40690a..adb6bc71c 100644 --- a/packages/neo4j-driver/test/internal/transaction-executor.test.js +++ b/packages/neo4j-driver/test/internal/transaction-executor.test.js @@ -30,9 +30,9 @@ const { SERVICE_UNAVAILABLE, SESSION_EXPIRED } = err const TRANSIENT_ERROR_1 = 'Neo.TransientError.Transaction.DeadlockDetected' const TRANSIENT_ERROR_2 = 'Neo.TransientError.Network.CommunicationError' const UNKNOWN_ERROR = 'Neo.DatabaseError.General.UnknownError' -const TX_TERMINATED_ERROR = 'Neo.TransientError.Transaction.Terminated' +const TX_TERMINATED_ERROR = 'Neo.ClientError.Transaction.Terminated' const LOCKS_TERMINATED_ERROR = - 'Neo.TransientError.Transaction.LockClientStopped' + 'Neo.ClientError.Transaction.LockClientStopped' const OOM_ERROR = 'Neo.DatabaseError.General.OutOfMemoryError' describe('#unit TransactionExecutor', () => {