diff --git a/packages/core/src/internal/transaction-executor.ts b/packages/core/src/internal/transaction-executor.ts index adeb5cd38..47d2ff3f0 100644 --- a/packages/core/src/internal/transaction-executor.ts +++ b/packages/core/src/internal/transaction-executor.ts @@ -19,6 +19,7 @@ import { newError } 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 @@ -26,7 +27,7 @@ const DEFAULT_INITIAL_RETRY_DELAY_MS = 1000 // 1 seconds const DEFAULT_RETRY_DELAY_MULTIPLIER = 2.0 const DEFAULT_RETRY_DELAY_JITTER_FACTOR = 0.2 -type TransactionCreator = () => Transaction +type TransactionCreator = () => TransactionPromise type TransactionWork = (tx: Transaction) => T | Promise type Resolve = (value: T | PromiseLike) => void type Reject = (value: any) => void @@ -138,15 +139,15 @@ export class TransactionExecutor { }) } - _executeTransactionInsidePromise( + async _executeTransactionInsidePromise( transactionCreator: TransactionCreator, transactionWork: TransactionWork, resolve: Resolve, reject: Reject - ): void { + ): Promise { let tx: Transaction try { - tx = transactionCreator() + tx = await transactionCreator() } catch (error) { // failed to create a transaction reject(error) diff --git a/packages/core/test/session.test.ts b/packages/core/test/session.test.ts index 7f6601919..bf49bba1b 100644 --- a/packages/core/test/session.test.ts +++ b/packages/core/test/session.test.ts @@ -101,7 +101,7 @@ describe('session', () => { }) it('run should send watermarks to Transaction when fetchsize if defined (writeTransaction)', async () => { - const connection = newFakeConnection() + const connection = mockBeginWithSuccess(newFakeConnection()) const session = newSessionWithConnection(connection, false, 1000) const status = { functionCalled: false } @@ -118,7 +118,7 @@ describe('session', () => { }) it('run should send watermarks to Transaction when fetchsize is fetch all (writeTransaction)', async () => { - const connection = newFakeConnection() + const connection = mockBeginWithSuccess(newFakeConnection()) const session = newSessionWithConnection(connection, false, FETCH_ALL) const status = { functionCalled: false } @@ -135,7 +135,7 @@ describe('session', () => { }) it('run should send watermarks to Transaction when fetchsize if defined (readTransaction)', async () => { - const connection = newFakeConnection() + const connection = mockBeginWithSuccess(newFakeConnection()) const session = newSessionWithConnection(connection, false, 1000) const status = { functionCalled: false } @@ -152,7 +152,7 @@ describe('session', () => { }) it('run should send watermarks to Transaction when fetchsize is fetch all (readTransaction)', async () => { - const connection = newFakeConnection() + const connection = mockBeginWithSuccess(newFakeConnection()) const session = newSessionWithConnection(connection, false, FETCH_ALL) const status = { functionCalled: false } @@ -238,17 +238,8 @@ describe('session', () => { }) it('should resolves a Transaction', async () => { - const connection = newFakeConnection() - const protocol = connection.protocol() - // @ts-ignore - connection.protocol = () => { - return { - ...protocol, - beginTransaction: (params: { afterComplete: () => {} }) => { - params.afterComplete() - } - } - } + const connection = mockBeginWithSuccess(newFakeConnection()) + const session = newSessionWithConnection(connection, false, 1000) const tx: Transaction = await session.beginTransaction() @@ -258,6 +249,20 @@ describe('session', () => { }) }) +function mockBeginWithSuccess(connection: FakeConnection) { + const protocol = connection.protocol() + // @ts-ignore + connection.protocol = () => { + return { + ...protocol, + beginTransaction: (params: { afterComplete: () => {}} ) => { + params.afterComplete() + } + } + } + return connection +} + function newSessionWithConnection( connection: Connection, beginTx: boolean = true, diff --git a/packages/neo4j-driver/test/internal/transaction-executor.test.js b/packages/neo4j-driver/test/internal/transaction-executor.test.js index 54330cef7..021155982 100644 --- a/packages/neo4j-driver/test/internal/transaction-executor.test.js +++ b/packages/neo4j-driver/test/internal/transaction-executor.test.js @@ -195,6 +195,22 @@ describe('#unit TransactionExecutor', () => { ]) }, 30000) + it('should retry when given transaction creator fail on begin once', async () => { + await testRetryWhenTransactionBeginFails([SERVICE_UNAVAILABLE]) + }, 30000) + + it('should retry when given transaction creator throws on begin many times', async () => { + await testRetryWhenTransactionBeginFails([ + SERVICE_UNAVAILABLE, + SESSION_EXPIRED, + TRANSIENT_ERROR_2, + SESSION_EXPIRED, + SERVICE_UNAVAILABLE, + TRANSIENT_ERROR_1, + 'Neo.ClientError.Security.AuthorizationExpired' + ]) + }, 30000) + it('should retry when given transaction work throws once', async () => { await testRetryWhenTransactionWorkThrows([SERVICE_UNAVAILABLE]) }, 30000) @@ -294,6 +310,30 @@ describe('#unit TransactionExecutor', () => { } } + async function testRetryWhenTransactionBeginFails (errorCodes) { + const fakeSetTimeout = setTimeoutMock.install() + try { + const executor = new TransactionExecutor() + const transactionCreator = throwingTransactionCreatorOnBegin( + errorCodes, + new FakeTransaction() + ) + const usedTransactions = [] + + const result = await executor.execute(transactionCreator, tx => { + expect(tx).toBeDefined() + usedTransactions.push(tx) + return Promise.resolve(42) + }) + + expect(usedTransactions.length).toEqual(1) + expect(result).toEqual(42) + verifyRetryDelays(fakeSetTimeout, errorCodes.length) + } finally { + fakeSetTimeout.uninstall() + } + } + async function testRetryWhenTransactionWorkReturnsRejectedPromise ( errorCodes ) { @@ -451,6 +491,17 @@ function throwingTransactionCreator (errorCodes, result) { } } +function throwingTransactionCreatorOnBegin (errorCodes, result) { + const remainingErrorCodes = errorCodes.slice().reverse() + return () => { + if (remainingErrorCodes.length === 0) { + return Promise.resolve(result) + } + const errorCode = remainingErrorCodes.pop() + return Promise.reject(error(errorCode)) + } +} + function throwingTransactionWork (errorCodes, result) { const remainingErrorCodes = errorCodes.slice().reverse() return () => { diff --git a/packages/testkit-backend/src/skipped-tests/common.js b/packages/testkit-backend/src/skipped-tests/common.js index ad9db4d78..ca224c8f7 100644 --- a/packages/testkit-backend/src/skipped-tests/common.js +++ b/packages/testkit-backend/src/skipped-tests/common.js @@ -7,11 +7,6 @@ const skippedTests = [ ifEquals('stub.optimizations.test_optimizations.TestOptimizations.test_uses_implicit_default_arguments_multi_query'), ifEquals('stub.optimizations.test_optimizations.TestOptimizations.test_uses_implicit_default_arguments_multi_query_nested') ), - skip( - 'Eager verification not implemented for tx functions', - ifEquals('stub.tx_run.test_tx_run.TestTxRun.test_eager_begin_on_tx_func_run_with_error_on_begin'), - ifEquals('stub.tx_run.test_tx_run.TestTxRun.test_eager_begin_on_tx_func_run_with_disconnect_on_begin') - ), skip( 'Fail while enable Temporary::ResultKeys', ifEquals('neo4j.test_bookmarks.TestBookmarks.test_can_pass_bookmark_into_next_session'),