From e2ac2fab565230424e7f1429e0942f7d8dfa2e53 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Thu, 8 Aug 2024 10:54:12 -0400 Subject: [PATCH 1/3] test(NODE-6318): utf runner withTransaction callback propagates errors from operations --- .../unified_test_format.test.ts | 74 +++++++++++++++++++ test/tools/unified-spec-runner/operations.ts | 33 ++++----- test/tools/unified-spec-runner/schema.ts | 1 + 3 files changed, 91 insertions(+), 17 deletions(-) create mode 100644 test/integration/unified-test-format/unified_test_format.test.ts diff --git a/test/integration/unified-test-format/unified_test_format.test.ts b/test/integration/unified-test-format/unified_test_format.test.ts new file mode 100644 index 0000000000..3cc4a8f9b5 --- /dev/null +++ b/test/integration/unified-test-format/unified_test_format.test.ts @@ -0,0 +1,74 @@ +import { type FailPoint, TestBuilder, UnifiedTestSuiteBuilder } from '../../tools/utils'; + +describe('Unified Test Runner', () => { + UnifiedTestSuiteBuilder.describe('withTransaction error propagation') + .runOnRequirement({ topologies: ['replicaset'], minServerVersion: '4.4.0' }) + .createEntities([ + { + client: { + id: 'client', + useMultipleMongoses: true, + uriOptions: { appName: 'bob' }, + observeEvents: ['commandStartedEvent', 'commandSucceededEvent', 'commandFailedEvent'] + } + }, + { database: { id: 'database', client: 'client', databaseName: 'test' } }, + { collection: { id: 'collection', database: 'database', collectionName: 'coll' } }, + { session: { id: 'session', client: 'client' } }, + + { client: { id: 'failPointClient', useMultipleMongoses: false } } + ]) + .test( + TestBuilder.it('should propagation the error to the withTransaction API') + .operation({ + name: 'failPoint', + object: 'testRunner', + arguments: { + client: 'failPointClient', + failPoint: { + configureFailPoint: 'failCommand', + mode: { times: 1 }, + data: { failCommands: ['insert'], errorCode: 50, appName: 'bob' } + } as FailPoint + } + }) + .operation({ + name: 'withTransaction', + object: 'session', + arguments: { + callback: [ + { + name: 'insertOne', + object: 'collection', + arguments: { session: 'session', document: { _id: 1 } }, + expectError: { isClientError: false } + } + ] + }, + expectError: { isClientError: false } + }) + .expectEvents({ + client: 'client', + events: [ + { + commandStartedEvent: { + commandName: 'insert', + databaseName: 'test', + command: { insert: 'coll' } + } + }, + { commandFailedEvent: { commandName: 'insert' } }, + { + commandStartedEvent: { + commandName: 'abortTransaction', + databaseName: 'admin', + command: { abortTransaction: 1 } + } + }, + { commandFailedEvent: { commandName: 'abortTransaction' } } + ] + }) + .toJSON() + ) + .run(); +}); diff --git a/test/tools/unified-spec-runner/operations.ts b/test/tools/unified-spec-runner/operations.ts index 045016c08e..51d458a185 100644 --- a/test/tools/unified-spec-runner/operations.ts +++ b/test/tools/unified-spec-runner/operations.ts @@ -1,4 +1,3 @@ -/* eslint-disable @typescript-eslint/no-unused-vars */ /* eslint-disable @typescript-eslint/no-non-null-assertion */ import { Readable } from 'node:stream'; @@ -7,16 +6,13 @@ import { pipeline } from 'node:stream/promises'; import { AssertionError, expect } from 'chai'; import { - AbstractCursor, type ChangeStream, Collection, CommandStartedEvent, Db, type Document, - type GridFSFile, type MongoClient, MongoError, - type ObjectId, ReadConcern, ReadPreference, SERVER_DESCRIPTION_CHANGED, @@ -25,7 +21,7 @@ import { type TopologyType, WriteConcern } from '../../mongodb'; -import { getSymbolFrom, sleep } from '../../tools/utils'; +import { sleep } from '../../tools/utils'; import { type TestConfiguration } from '../runner/config'; import { EntitiesMap } from './entities'; import { expectErrorCheck, resultCheck } from './match'; @@ -153,27 +149,27 @@ operations.set('assertSameLsidOnLastTwoCommands', async ({ entities, operation } expect(last.command.lsid.id.buffer.equals(secondLast.command.lsid.id.buffer)).to.be.true; }); -operations.set('assertSessionDirty', async ({ entities, operation }) => { +operations.set('assertSessionDirty', async ({ operation }) => { const session = operation.arguments!.session; expect(session.serverSession.isDirty).to.be.true; }); -operations.set('assertSessionNotDirty', async ({ entities, operation }) => { +operations.set('assertSessionNotDirty', async ({ operation }) => { const session = operation.arguments!.session; expect(session.serverSession.isDirty).to.be.false; }); -operations.set('assertSessionPinned', async ({ entities, operation }) => { +operations.set('assertSessionPinned', async ({ operation }) => { const session = operation.arguments!.session; expect(session.isPinned, 'session should be pinned').to.be.true; }); -operations.set('assertSessionUnpinned', async ({ entities, operation }) => { +operations.set('assertSessionUnpinned', async ({ operation }) => { const session = operation.arguments!.session; expect(session.isPinned, 'session should be unpinned').to.be.false; }); -operations.set('assertSessionTransactionState', async ({ entities, operation }) => { +operations.set('assertSessionTransactionState', async ({ operation }) => { const session = operation.arguments!.session; const transactionStateTranslation = { @@ -240,7 +236,7 @@ operations.set('commitTransaction', async ({ entities, operation }) => { operations.set('createChangeStream', async ({ entities, operation }) => { const watchable = entities.get(operation.object); - if (watchable == null || !('watch' in watchable)) { + if (watchable == null || typeof watchable !== 'object' || !('watch' in watchable)) { throw new AssertionError(`Entity ${operation.object} must be watchable`); } @@ -292,7 +288,7 @@ operations.set('dropCollection', async ({ entities, operation }) => { // TODO(NODE-4243): dropCollection should suppress namespace not found errors try { - return await db.dropCollection(collection, opts); + await db.dropCollection(collection, opts); } catch (err) { if (!/ns not found/.test(err.message)) { throw err; @@ -544,10 +540,10 @@ operations.set('upload', async ({ entities, operation }) => { const bucket = entities.getEntity('bucket', operation.object); const { filename, source, ...options } = operation.arguments ?? {}; - const stream = bucket.openUploadStream(operation.arguments!.filename, options); - const filestream = Readable.from(Buffer.from(operation.arguments!.source.$$hexBytes, 'hex')); + const stream = bucket.openUploadStream(filename, options); + const fileStream = Readable.from(Buffer.from(source.$$hexBytes, 'hex')); - await pipeline(filestream, stream); + await pipeline(fileStream, stream); return stream.gridFSFile?._id; }); @@ -720,7 +716,7 @@ operations.set('withTransaction', async ({ entities, operation, client, testConf await session.withTransaction(async () => { for (const callbackOperation of operation.arguments!.callback) { - await executeOperationAndCheck(callbackOperation, entities, client, testConfig); + await executeOperationAndCheck(callbackOperation, entities, client, testConfig, true); } }, options); }); @@ -935,7 +931,8 @@ export async function executeOperationAndCheck( operation: OperationDescription, entities: EntitiesMap, client: MongoClient, - testConfig: TestConfiguration + testConfig: TestConfiguration, + rethrow = false ): Promise { const opFunc = operations.get(operation.name); expect(opFunc, `Unknown operation: ${operation.name}`).to.exist; @@ -956,10 +953,12 @@ export async function executeOperationAndCheck( } catch (error) { if (operation.expectError) { expectErrorCheck(error, operation.expectError, entities); + if (rethrow) throw error; return; } else if (!operation.ignoreResultAndError || error instanceof MalformedOperationError) { throw error; } + if (rethrow) throw error; } // We check the positive outcome here so the try-catch above doesn't catch our chai assertions diff --git a/test/tools/unified-spec-runner/schema.ts b/test/tools/unified-spec-runner/schema.ts index 81b8172463..ce722b2e70 100644 --- a/test/tools/unified-spec-runner/schema.ts +++ b/test/tools/unified-spec-runner/schema.ts @@ -386,6 +386,7 @@ export interface StoreEventsAsEntity { } export interface ExpectedError { isError?: true; + isTimeoutError?: boolean; isClientError?: boolean; errorContains?: string; errorCode?: number; From 22e1bbb4f46a16bf852155f7d9a2982a74c72d2d Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Mon, 12 Aug 2024 13:26:13 -0400 Subject: [PATCH 2/3] chore: test name --- .../integration/unified-test-format/unified_test_format.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/unified-test-format/unified_test_format.test.ts b/test/integration/unified-test-format/unified_test_format.test.ts index 3cc4a8f9b5..cb3b40f9f2 100644 --- a/test/integration/unified-test-format/unified_test_format.test.ts +++ b/test/integration/unified-test-format/unified_test_format.test.ts @@ -19,7 +19,7 @@ describe('Unified Test Runner', () => { { client: { id: 'failPointClient', useMultipleMongoses: false } } ]) .test( - TestBuilder.it('should propagation the error to the withTransaction API') + TestBuilder.it('should propagate the error to the withTransaction API') .operation({ name: 'failPoint', object: 'testRunner', From 243f1a393482a83bc0113cf2a4d346f96a11807c Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Mon, 12 Aug 2024 15:54:12 -0400 Subject: [PATCH 3/3] chore: take out CSOT change --- test/tools/unified-spec-runner/schema.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/test/tools/unified-spec-runner/schema.ts b/test/tools/unified-spec-runner/schema.ts index ce722b2e70..81b8172463 100644 --- a/test/tools/unified-spec-runner/schema.ts +++ b/test/tools/unified-spec-runner/schema.ts @@ -386,7 +386,6 @@ export interface StoreEventsAsEntity { } export interface ExpectedError { isError?: true; - isTimeoutError?: boolean; isClientError?: boolean; errorContains?: string; errorCode?: number;