Skip to content

Reclassify Transaction.LockClientStopped and Transaction.Terminated as ClientError #945

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
Sep 12, 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
21 changes: 20 additions & 1 deletion packages/bolt-connection/src/bolt/response-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Expand Down Expand Up @@ -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
}
74 changes: 74 additions & 0 deletions packages/bolt-connection/test/bolt/response-handler.test.js
Original file line number Diff line number Diff line change
@@ -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 <metadata>

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)
})
})
})
20 changes: 3 additions & 17 deletions packages/core/src/error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,30 +129,16 @@ function _isRetriableCode (code?: Neo4jErrorCode): boolean {
return code === SERVICE_UNAVAILABLE ||
code === SESSION_EXPIRED ||
_isAuthorizationExpired(code) ||
_isRetriableTransientError(code)
_isTransientError(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?.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
}

/**
Expand Down
10 changes: 6 additions & 4 deletions packages/core/test/error.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
]
}
4 changes: 2 additions & 2 deletions packages/neo4j-driver/test/internal/retry-logic-rx.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ describe('#unit-rx retrylogic', () => {
verifyNoRetry(
newError(
'transaction terminated',
'Neo.TransientError.Transaction.Terminated'
'Neo.ClientError.Transaction.Terminated'
)
)
})
Expand All @@ -74,7 +74,7 @@ describe('#unit-rx retrylogic', () => {
verifyNoRetry(
newError(
'lock client stopped',
'Neo.TransientError.Transaction.LockClientStopped'
'Neo.ClientError.Transaction.LockClientStopped'
)
)
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down