feat(NODE-4598)!: close cursor on early loop break (#3502)

Author: durran

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
+```

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

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