diff --git a/src/cmap/connection_pool.ts b/src/cmap/connection_pool.ts index 723b4c8f67f..65bbe5c7eab 100644 --- a/src/cmap/connection_pool.ts +++ b/src/cmap/connection_pool.ts @@ -423,11 +423,10 @@ export class ConnectionPool extends TypedEventEmitter { this[kPoolState] = PoolState.paused; this.clearMinPoolSizeTimer(); - this.processWaitQueue(); - if (!alreadyPaused) { this.emit(ConnectionPool.CONNECTION_POOL_CLEARED, new ConnectionPoolClearedEvent(this)); } + this.processWaitQueue(); } /** Close the pool */ diff --git a/src/cmap/errors.ts b/src/cmap/errors.ts index c8355ec562c..b2892715ad2 100644 --- a/src/cmap/errors.ts +++ b/src/cmap/errors.ts @@ -24,7 +24,6 @@ export class PoolClosedError extends MongoDriverError { * @category Error */ export class PoolClearedError extends MongoNetworkError { - // TODO(NODE-3144): needs to extend RetryableError or be marked retryable in some other way per spec /** The address of the connection pool */ address: string; diff --git a/test/integration/retryable-reads/retryable_reads.spec.prose.test.ts b/test/integration/retryable-reads/retryable_reads.spec.prose.test.ts new file mode 100644 index 00000000000..af6f9cbed4e --- /dev/null +++ b/test/integration/retryable-reads/retryable_reads.spec.prose.test.ts @@ -0,0 +1,140 @@ +/* eslint-disable @typescript-eslint/no-non-null-assertion */ +import { expect } from 'chai'; + +import { Collection, MongoClient } from '../../../src'; + +describe('Retryable Reads Spec Prose', () => { + let client: MongoClient, failPointName; + + afterEach(async () => { + try { + if (failPointName) { + await client.db('admin').command({ configureFailPoint: failPointName, mode: 'off' }); + } + } finally { + failPointName = undefined; + await client?.close(); + } + }); + + describe('PoolClearedError Retryability Test', () => { + // This test will be used to ensure drivers properly retry after encountering PoolClearedErrors. + // It MUST be implemented by any driver that implements the CMAP specification. + // This test requires MongoDB 4.2.9+ for blockConnection support in the failpoint. + + let cmapEvents: Array<{ name: string; event: Record }>; + let commandStartedEvents: Array>; + let testCollection: Collection; + beforeEach(async function () { + // 1. Create a client with maxPoolSize=1 and retryReads=true. + client = this.configuration.newClient( + this.configuration.url({ + useMultipleMongoses: false // If testing against a sharded deployment, be sure to connect to only a single mongos. + }), + { maxPoolSize: 1, retryReads: true, monitorCommands: true } + ); + + console.log(client.options); + + testCollection = client.db('retryable-reads-prose').collection('pool-clear-retry'); + await testCollection.drop().catch(() => null); + await testCollection.insertMany([{ test: 1 }, { test: 2 }]); + + // 2. Enable the following failpoint: + // NOTE: "9. Disable the failpoint" is done in afterEach + failPointName = 'failCommand'; + const failPoint = await client.db('admin').command({ + configureFailPoint: failPointName, + mode: { times: 1 }, + data: { + failCommands: ['find'], + errorCode: 91, + blockConnection: true, + blockTimeMS: 1000 + } + }); + + expect(failPoint).to.have.property('ok', 1); + + cmapEvents = []; + commandStartedEvents = []; + for (const observedEvent of [ + 'connectionCheckOutStarted', + 'connectionCheckedOut', + 'connectionCheckOutFailed', + 'connectionPoolCleared' + ]) { + client.on(observedEvent, ev => { + cmapEvents.push({ name: observedEvent, event: ev }); + }); + } + + client.on('commandStarted', ev => { + commandStartedEvents.push(ev); + }); + }); + + it('should emit events in the expected sequence', { + metadata: { requires: { mongodb: '>=4.2.9', topology: '!load-balanced' } }, + test: async function () { + // 3. Start two threads and attempt to perform a findOne simultaneously on both. + const results = await Promise.all([ + testCollection.findOne({ test: 1 }), + testCollection.findOne({ test: 2 }) + ]); + + client.removeAllListeners(); + // 4. Verify that both findOne attempts succeed. + expect(results[0]).to.have.property('test', 1); + expect(results[1]).to.have.property('test', 2); + + // NOTE: For the subsequent checks, we rely on the exact sequence of ALL events + // for ease of readability; however, only the relative order matters for + // the purposes of this test, so if this ever becomes an issue, the test + // can be refactored to assert on relative index values instead + + // 5. Via CMAP monitoring, assert that the first check out succeeds. + expect(cmapEvents.shift()).to.have.property( + 'name', + 'connectionCheckOutStarted', + 'expected 1) checkout 1 to start' + ); + expect(cmapEvents.shift()).to.have.property( + 'name', + 'connectionCheckOutStarted', + 'expected 2) checkout 2 to start' + ); + expect(cmapEvents.shift()).to.have.property( + 'name', + 'connectionCheckedOut', + 'expected 3) first checkout to succeed' + ); + + // 6. Via CMAP monitoring, assert that a PoolClearedEvent is then emitted. + expect(cmapEvents.shift()).to.have.property( + 'name', + 'connectionPoolCleared', + 'expected 4) pool to clear' + ); + + // 7. Via CMAP monitoring, assert that the second check out then fails due to a connection error. + const nextEvent = cmapEvents.shift(); + expect(nextEvent).to.have.property( + 'name', + 'connectionCheckOutFailed', + 'expected 5) checkout 2 to fail' + ); + expect(nextEvent!.event).to.have.property('reason', 'connectionError'); + + // 8. Via Command Monitoring, assert that exactly three find CommandStartedEvents were observed in total. + const observedFindCommandStartedEvents = commandStartedEvents.filter( + ({ commandName }) => commandName === 'find' + ); + expect(observedFindCommandStartedEvents).to.have.lengthOf( + 3, + 'expected 3 find command started events' + ); + } + }); + }); +}); diff --git a/test/integration/retryable-writes/retryable_writes.spec.prose.test.ts b/test/integration/retryable-writes/retryable_writes.spec.prose.test.ts index 2171885f85c..e0fa383fb11 100644 --- a/test/integration/retryable-writes/retryable_writes.spec.prose.test.ts +++ b/test/integration/retryable-writes/retryable_writes.spec.prose.test.ts @@ -1,64 +1,191 @@ +/* eslint-disable @typescript-eslint/no-non-null-assertion */ import { expect } from 'chai'; -import { MongoError, MongoServerError, TopologyType } from '../../../src'; +import { Collection, MongoClient, MongoError, MongoServerError } from '../../../src'; describe('Retryable Writes Spec Prose', () => { - /** - * 1 Test that retryable writes raise an exception when using the MMAPv1 storage engine. - * For this test, execute a write operation, such as insertOne, which should generate an exception and the error code is 20. - * Assert that the error message is the replacement error message: - * - * ``` - * This MongoDB deployment does not support retryable writes. Please add - * retryWrites=false to your connection string. - * ``` - * Note: Drivers that rely on serverStatus to determine the storage engine in use MAY skip this test for sharded clusters, since mongos does not report this information in its serverStatus response. - */ - let client; - - beforeEach(async function () { - if ( - this.configuration.buildInfo.versionArray[0] < 4 || - this.configuration.topologyType !== TopologyType.ReplicaSetWithPrimary - ) { - this.currentTest.skipReason = - 'configureFailPoint only works on server versions greater than 4'; - this.skip(); + let client: MongoClient, failPointName; + + afterEach(async () => { + try { + if (failPointName) { + await client.db('admin').command({ configureFailPoint: failPointName, mode: 'off' }); + } + } finally { + failPointName = undefined; + await client?.close(); } - client = this.configuration.newClient(); - await client.connect(); }); - afterEach(async () => { - await client?.close(); + describe('1. Test that retryable writes raise an exception when using the MMAPv1 storage engine.', () => { + /** + * For this test, execute a write operation, such as insertOne, which should generate an exception and the error code is 20. + * Assert that the error message is the replacement error message: + * + * ``` + * This MongoDB deployment does not support retryable writes. Please add + * retryWrites=false to your connection string. + * ``` + * Note: Drivers that rely on serverStatus to determine the storage engine in use MAY skip this test for sharded clusters, since mongos does not report this information in its serverStatus response. + */ + beforeEach(async function () { + client = this.configuration.newClient(); + await client.connect(); + + failPointName = 'failCommand'; + const failPoint = await client.db('admin').command({ + configureFailPoint: failPointName, + mode: { times: 1 }, + data: { + failCommands: ['insert'], + errorCode: 20, // MMAP Error code, + closeConnection: false + } + }); + + expect(failPoint).to.have.property('ok', 1); + }); + + for (const testTopology of ['replicaset', 'sharded']) { + const minFailPointVersion = testTopology === 'replicaset' ? '>=4.0.0' : '>=4.1.5'; + it(`should error with the correct error message when topology is ${testTopology}`, { + metadata: { requires: { mongodb: minFailPointVersion, topology: [testTopology as any] } }, + test: async function () { + const error = await client + .db('test') + .collection('test') + .insertOne({ a: 1 }) + .catch(error => error); + + expect(error).to.exist; + expect(error).that.is.instanceOf(MongoServerError); + expect(error).to.have.property('originalError').that.instanceOf(MongoError); + expect(error.originalError).to.have.property('code', 20); + expect(error).to.have.property( + 'message', + 'This MongoDB deployment does not support retryable writes. Please add retryWrites=false to your connection string.' + ); + } + }); + } }); - it('retryable writes raise an exception when using the MMAPv1 storage engine', async () => { - const failPoint = await client.db('admin').command({ - configureFailPoint: 'failCommand', - mode: { times: 1 }, - data: { - failCommands: ['insert'], - errorCode: 20, // MMAP Error code, - closeConnection: false + describe('2. Test that drivers properly retry after encountering PoolClearedErrors.', () => { + // This test MUST be implemented by any driver that implements the CMAP specification. + // This test requires MongoDB 4.2.9+ for blockConnection support in the failpoint. + + let cmapEvents: Array<{ name: string; event: Record }>; + let commandStartedEvents: Array>; + let testCollection: Collection; + beforeEach(async function () { + // i. Create a client with maxPoolSize=1 and retryWrites=true. + client = this.configuration.newClient( + this.configuration.url({ + useMultipleMongoses: false // If testing against a sharded deployment, be sure to connect to only a single mongos. + }), + { maxPoolSize: 1, retryWrites: true, monitorCommands: true } + ); + + testCollection = client.db('retryable-writes-prose').collection('pool-clear-retry'); + await testCollection.drop().catch(() => null); + + // ii. Enable the following failpoint: + // NOTE: "ix. Disable the failpoint" is done in afterEach + failPointName = 'failCommand'; + const failPoint = await client.db('admin').command({ + configureFailPoint: failPointName, + mode: { times: 1 }, + data: { + failCommands: ['insert'], + errorCode: 91, + blockConnection: true, + blockTimeMS: 1000, + errorLabels: ['RetryableWriteError'] + } + }); + + expect(failPoint).to.have.property('ok', 1); + + cmapEvents = []; + commandStartedEvents = []; + for (const observedEvent of [ + 'connectionCheckOutStarted', + 'connectionCheckedOut', + 'connectionCheckOutFailed', + 'connectionPoolCleared' + ]) { + client.on(observedEvent, ev => { + cmapEvents.push({ name: observedEvent, event: ev }); + }); } + client.on('commandStarted', ev => { + commandStartedEvents.push(ev); + }); }); - expect(failPoint).to.have.property('ok', 1); - - const error = await client - .db('test') - .collection('test') - .insertOne({ a: 1 }) - .catch(error => error); - - expect(error).to.exist; - expect(error).that.is.instanceOf(MongoServerError); - expect(error).to.have.property('originalError').that.instanceOf(MongoError); - expect(error.originalError).to.have.property('code', 20); - expect(error).to.have.property( - 'message', - 'This MongoDB deployment does not support retryable writes. Please add retryWrites=false to your connection string.' - ); + it('should emit events in the expected sequence', { + metadata: { requires: { mongodb: '>=4.2.9', topology: ['replicaset', 'sharded'] } }, + test: async function () { + // iii. Start two threads and attempt to perform an insertOne simultaneously on both. + await Promise.all([ + testCollection.insertOne({ test: 1 }), + testCollection.insertOne({ test: 2 }) + ]); + + client.removeAllListeners(); + // iv. Verify that both insertOne attempts succeed. + const result = await testCollection.find().toArray(); + expect(result).to.have.lengthOf(2); + const mappedAndSortedResult = result.map(item => item.test).sort(); + expect(mappedAndSortedResult).to.deep.equal([1, 2]); + + // NOTE: For the subsequent checks, we rely on the exact sequence of ALL events + // for ease of readability; however, only the relative order matters for + // the purposes of this test, so if this ever becomes an issue, the test + // can be refactored to assert on relative index values instead + + // v. Via CMAP monitoring, assert that the first check out succeeds. + expect(cmapEvents.shift()).to.have.property( + 'name', + 'connectionCheckOutStarted', + 'expected 1) checkout 1 to start' + ); + expect(cmapEvents.shift()).to.have.property( + 'name', + 'connectionCheckOutStarted', + 'expected 2) checkout 2 to start' + ); + expect(cmapEvents.shift()).to.have.property( + 'name', + 'connectionCheckedOut', + 'expected 3) first checkout to succeed' + ); + + // vi. Via CMAP monitoring, assert that a PoolClearedEvent is then emitted. + expect(cmapEvents.shift()).to.have.property( + 'name', + 'connectionPoolCleared', + 'expected 4) pool to clear' + ); + + // vii. Via CMAP monitoring, assert that the second check out then fails due to a connection error. + const nextEvent = cmapEvents.shift(); + expect(nextEvent).to.have.property( + 'name', + 'connectionCheckOutFailed', + 'expected 5) checkout 2 to fail' + ); + expect(nextEvent!.event).to.have.property('reason', 'connectionError'); + + // viii. Via Command Monitoring, assert that exactly three insert CommandStartedEvents were observed in total. + const observedInsertCommandStartedEvents = commandStartedEvents.filter( + ({ commandName }) => commandName === 'insert' + ); + expect(observedInsertCommandStartedEvents).to.have.lengthOf( + 3, + 'expected 3 insert command started events' + ); + } + }); }); });