diff --git a/etc/notes/CHANGES_5.0.0.md b/etc/notes/CHANGES_5.0.0.md index 20dae142c57..7729a4b87a8 100644 --- a/etc/notes/CHANGES_5.0.0.md +++ b/etc/notes/CHANGES_5.0.0.md @@ -47,3 +47,17 @@ The new minimum supported Node.js version is now 14.20.1. The MongoClient option `promiseLibrary` along with the `Promise.set` export that allows specifying a custom promise library has been removed. This allows the driver to adopt async/await syntax which has [performance benefits](https://v8.dev/blog/fast-async) over manual promise construction. + +### Cursor closes on exit of for await of loops + +Cursors will now automatically close when exiting a for await of loop on the cursor itself. + +```js +const cursor = collection.find({}); +for await (const doc of cursor) { + console.log(doc); + break; +} + +cursor.closed // true +``` diff --git a/src/cursor/abstract_cursor.ts b/src/cursor/abstract_cursor.ts index de3a0cb9775..39dfc06bf9c 100644 --- a/src/cursor/abstract_cursor.ts +++ b/src/cursor/abstract_cursor.ts @@ -301,29 +301,37 @@ export abstract class AbstractCursor< return; } - while (true) { - const document = await this.next(); + try { + while (true) { + const document = await this.next(); - // Intentional strict null check, because users can map cursors to falsey values. - // We allow mapping to all values except for null. - // eslint-disable-next-line no-restricted-syntax - if (document === null) { - if (!this.closed) { - const message = - 'Cursor returned a `null` document, but the cursor is not exhausted. Mapping documents to `null` is not supported in the cursor transform.'; + // Intentional strict null check, because users can map cursors to falsey values. + // We allow mapping to all values except for null. + // eslint-disable-next-line no-restricted-syntax + if (document === null) { + if (!this.closed) { + const message = + 'Cursor returned a `null` document, but the cursor is not exhausted. Mapping documents to `null` is not supported in the cursor transform.'; - await cleanupCursorAsync(this, { needsToEmitClosed: true }).catch(() => null); + await cleanupCursorAsync(this, { needsToEmitClosed: true }).catch(() => null); - throw new MongoAPIError(message); + throw new MongoAPIError(message); + } + break; } - break; - } - yield document; + yield document; - if (this[kId] === Long.ZERO) { - // Cursor exhausted - break; + if (this[kId] === Long.ZERO) { + // Cursor exhausted + break; + } + } + } finally { + // Only close the cursor if it has not already been closed. This finally clause handles + // the case when a user would break out of a for await of loop early. + if (!this.closed) { + await this.close().catch(() => null); } } } diff --git a/test/integration/node-specific/cursor_async_iterator.test.js b/test/integration/node-specific/cursor_async_iterator.test.js index 4635c414541..f0c5254914a 100644 --- a/test/integration/node-specific/cursor_async_iterator.test.js +++ b/test/integration/node-specific/cursor_async_iterator.test.js @@ -1,6 +1,7 @@ 'use strict'; const { expect } = require('chai'); +const sinon = require('sinon'); describe('Cursor Async Iterator Tests', function () { context('default promise library', function () { @@ -36,6 +37,7 @@ describe('Cursor Async Iterator Tests', function () { } expect(counter).to.equal(1000); + expect(cursor.closed).to.be.true; }); it('should be able to use a for-await loop on an aggregation cursor', async function () { @@ -48,6 +50,7 @@ describe('Cursor Async Iterator Tests', function () { } expect(counter).to.equal(1000); + expect(cursor.closed).to.be.true; }); it('should be able to use a for-await loop on a command cursor', { @@ -64,6 +67,8 @@ describe('Cursor Async Iterator Tests', function () { } expect(counter).to.equal(indexes.length); + expect(cursor1.closed).to.be.true; + expect(cursor2.closed).to.be.true; } }); @@ -78,6 +83,36 @@ describe('Cursor Async Iterator Tests', function () { } expect(count).to.equal(1); + expect(cursor.closed).to.be.true; + }); + + it('cleans up cursor when breaking out of for await of loops', async function () { + const cursor = collection.find(); + + for await (const doc of cursor) { + expect(doc).to.exist; + break; + } + + expect(cursor.closed).to.be.true; + }); + + it('returns when attempting to reuse the cursor after a break', async function () { + const cursor = collection.find(); + const spy = sinon.spy(cursor); + + for await (const doc of cursor) { + expect(doc).to.exist; + break; + } + + expect(cursor.closed).to.be.true; + + for await (const doc of cursor) { + expect.fail('Async generator returns immediately if cursor is closed', doc); + } + // cursor.close() should only be called once. + expect(spy.close.calledOnce).to.be.true; }); }); });