test(NODE-6318): utf runner withTransaction callback propagates errors from operations

Author: nbbeeken

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();
+});

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<Document | boolean | number | null | void | string>;
 export const operations = new Map<string, RunOperationFn>();
 
-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<void> {
+): 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<void> {
+  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);
   }
 }

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;