From c1e995ea63d9e968905a6f2b87d3585eb38296df Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Thu, 8 Aug 2024 10:18:34 -0400 Subject: [PATCH] test(NODE-6318): utf runner withTransaction callback propagates errors from operations --- .../unified_test_format.test.ts | 74 ++++++++++++ test/tools/unified-spec-runner/operations.ts | 112 ++++++++---------- test/tools/unified-spec-runner/schema.ts | 1 + 3 files changed, 127 insertions(+), 60 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..16d7e60909 --- /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'] }) + .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..1b619e6ff7 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'; @@ -44,11 +40,9 @@ type RunOperationFn = ( ) => Promise; export const operations = new Map(); -export class MalformedOperationError extends AssertionError {} - operations.set('createEntities', async ({ entities, operation, testConfig }) => { if (!operation.arguments?.entities) { - throw new AssertionError('encountered createEntities operation without entities argument'); + expect.fail('encountered createEntities operation without entities argument'); } await EntitiesMap.createEntities(testConfig, null, operation.arguments.entities!, entities); }); @@ -61,7 +55,7 @@ operations.set('abortTransaction', async ({ entities, operation }) => { operations.set('aggregate', async ({ entities, operation }) => { const dbOrCollection = entities.get(operation.object) as Db | Collection; if (!(dbOrCollection instanceof Db || dbOrCollection instanceof Collection)) { - throw new AssertionError(`Operation object '${operation.object}' must be a db or collection`); + expect.fail(`Operation object '${operation.object}' must be a db or collection`); } const { pipeline, ...opts } = operation.arguments!; const cursor = dbOrCollection.aggregate(pipeline, opts); @@ -153,27 +147,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 = { @@ -230,7 +224,7 @@ operations.set('close', async ({ entities, operation }) => { } catch {} /* eslint-enable no-empty */ - throw new AssertionError(`No closable entity with key ${operation.object}`); + expect.fail(`No closable entity with key ${operation.object}`); }); operations.set('commitTransaction', async ({ entities, operation }) => { @@ -240,8 +234,8 @@ operations.set('commitTransaction', async ({ entities, operation }) => { operations.set('createChangeStream', async ({ entities, operation }) => { const watchable = entities.get(operation.object); - if (watchable == null || !('watch' in watchable)) { - throw new AssertionError(`Entity ${operation.object} must be watchable`); + if (watchable == null || typeof watchable !== 'object' || !('watch' in watchable)) { + expect.fail(`Entity ${operation.object} must be watchable`); } const { pipeline, ...args } = operation.arguments!; @@ -292,7 +286,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; @@ -359,7 +353,7 @@ operations.set('insertOne', async ({ entities, operation }) => { const collection = entities.getEntity('collection', operation.object); const { document, ...opts } = operation.arguments!; if (!document) { - throw new MalformedOperationError('No document defined in the arguments for insertOne'); + expect.fail('No document defined in the arguments for insertOne'); } // Looping exposes the fact that we can generate _ids for inserted // documents and we don't want the original operation to get modified @@ -544,10 +538,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; }); @@ -586,13 +580,11 @@ operations.set('waitForEvent', async ({ entities, operation }) => { await Promise.race([ eventPromise, sleep(10000).then(() => - Promise.reject( - new AssertionError( - `Timed out waiting for ${eventName}; captured [${mongoClient - .getCapturedEvents('all') - .map(e => e.constructor.name) - .join(', ')}]` - ) + expect.fail( + `Timed out waiting for ${eventName}; captured [${mongoClient + .getCapturedEvents('all') + .map(e => e.constructor.name) + .join(', ')}]` ) ) ]); @@ -682,7 +674,7 @@ operations.set('waitForPrimaryChange', async ({ entities, operation }) => { await Promise.race([ newPrimaryPromise, sleep(timeoutMS ?? 10000).then(() => - Promise.reject(new AssertionError(`Timed out waiting for primary change on ${client}`)) + expect.fail(`Timed out waiting for primary change on ${client}`) ) ]); }); @@ -702,9 +694,7 @@ operations.set('waitForThread', async ({ entities, operation }) => { const thread = entities.getEntity('thread', threadId, true); await Promise.race([ thread.finish(), - sleep(10000).then(() => - Promise.reject(new AssertionError(`Timed out waiting for thread: ${threadId}`)) - ) + sleep(10000).then(() => expect.fail(`Timed out waiting for thread: ${threadId}`)) ]); }); @@ -720,7 +710,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); }); @@ -757,11 +747,10 @@ operations.set('estimatedDocumentCount', async ({ entities, operation }) => { operations.set('runCommand', async ({ entities, operation }: OperationFunctionParams) => { const db = entities.getEntity('db', operation.object); - if (operation.arguments?.command == null) - throw new AssertionError('runCommand requires a command'); + if (operation.arguments?.command == null) expect.fail('runCommand requires a command'); const { command } = operation.arguments; - if (operation.arguments.timeoutMS != null) throw new AssertionError('timeoutMS not supported'); + if (operation.arguments.timeoutMS != null) expect.fail('timeoutMS not supported'); const options = { readPreference: operation.arguments.readPreference, @@ -931,14 +920,14 @@ operations.set('modifyCollection', async ({ entities, operation }) => { return db.command(command, {}); }); -export async function executeOperationAndCheck( +export async function executeTestOperation( operation: OperationDescription, entities: EntitiesMap, client: MongoClient, testConfig: TestConfiguration -): Promise { +): Promise<{ result: unknown; success: true } | { result: Error; success: false }> { const opFunc = operations.get(operation.name); - expect(opFunc, `Unknown operation: ${operation.name}`).to.exist; + if (opFunc == null) expect.fail(`Unknown operation: ${operation.name}`); if (operation.arguments && operation.arguments.session) { // The session could need to be either pulled from the entity map or in the case where @@ -949,33 +938,36 @@ export async function executeOperationAndCheck( } } - let result; - try { - result = await opFunc!({ entities, operation, client, testConfig }); - } catch (error) { - if (operation.expectError) { - expectErrorCheck(error, operation.expectError, entities); - return; - } else if (!operation.ignoreResultAndError || error instanceof MalformedOperationError) { - throw error; - } + const result = await opFunc({ entities, operation, client, testConfig }); + return { result, success: true }; + } catch (result) { + return { result, success: false }; } +} - // We check the positive outcome here so the try-catch above doesn't catch our chai assertions - if (operation.ignoreResultAndError) { - return; - } +export async function executeOperationAndCheck( + operation: OperationDescription, + entities: EntitiesMap, + client: MongoClient, + testConfig: TestConfiguration, + rethrow = false +): Promise { + const outcome = await executeTestOperation(operation, entities, client, testConfig); + + if (operation.saveResultAsEntity) entities.set(operation.saveResultAsEntity, outcome.result); + + if (operation.ignoreResultAndError) return; if (operation.expectError) { - expect.fail(`Operation ${operation.name} succeeded but was not supposed to`); + if (outcome.success === true) expect.fail(`${operation.name} unexpectedly succeeded`); + expectErrorCheck(outcome.result, operation.expectError, entities); + if (rethrow) throw outcome.result; + return; } if (operation.expectResult) { - resultCheck(result, operation.expectResult as any, entities); - } - - if (operation.saveResultAsEntity) { - entities.set(operation.saveResultAsEntity, result); + if (outcome.success === false) expect.fail(`${operation.name} unexpectedly failed`); + return resultCheck(outcome.result, operation.expectResult as any, entities); } } 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;