fix(NODE-6242): close becomes true after calling close when documents still remain

src/cursor/abstract_cursor.ts

@@ -209,6 +209,17 @@ export abstract class AbstractCursor<
     }
   }
 
+  /**
+   * The cursor has no id until it receives a response from the initial cursor creating command.
+   *
+   * It is non-zero for as long as the database has an open cursor.
+   *
+   * The initiating command may receive a zero id if the entire result is in the `firstBatch`.
+   *
+   * The id value may still be non-zero after calling `cursor.close()`
+   * since the value is only updated when the server returns a new id value,
+   * and a cursor may be closed before iteration is complete.
+   */
   get id(): Long | undefined {
     return this.cursorId ?? undefined;
   }
@@ -249,10 +260,17 @@ export abstract class AbstractCursor<
     this.cursorSession = clientSession;
   }
 
+  /**
+   * The cursor is closed and all remaining locally buffered documents have been iterated.
+   */
   get closed(): boolean {
-    return this.isClosed;
+    return this.isClosed && (this.documents?.length ?? 0) === 0;
   }
 
+  /**
+   * A `killCursors` command was attempted on this cursor.
+   * This is performed if the cursor id is non zero.
+   */
   get killed(): boolean {
     return this.isKilled;
   }
@@ -294,7 +312,7 @@ export abstract class AbstractCursor<
           return;
         }
 
-        if (this.closed && (this.documents?.length ?? 0) === 0) {
+        if (this.closed) {
           return;
         }
 

test/integration/change-streams/change_stream.test.ts

@@ -1060,7 +1060,8 @@ describe('Change Streams', function () {
             await changeStreamIterator.next();
             await changeStreamIterator.return();
             expect(changeStream.closed).to.be.true;
-            expect(changeStream.cursor).property('closed', true);
+            expect(changeStream.cursor).property('isClosed', true);
+            expect(changeStream.cursor).nested.property('session.hasEnded', true);
           }
         );
 

test/integration/node-specific/abstract_cursor.test.ts

@@ -193,7 +193,9 @@ describe('class AbstractCursor', function () {
         const error = await cursor.toArray().catch(e => e);
 
         expect(error).be.instanceOf(MongoAPIError);
-        expect(cursor.closed).to.be.true;
+        expect(cursor.id.isZero()).to.be.true;
+        // The first batch exhausted the cursor, the only thing to clean up is the session
+        expect(cursor.session.hasEnded).to.be.true;
       });
     });
 
@@ -225,7 +227,9 @@ describe('class AbstractCursor', function () {
           }
         } catch (error) {
           expect(error).to.be.instanceOf(MongoAPIError);
-          expect(cursor.closed).to.be.true;
+          expect(cursor.id.isZero()).to.be.true;
+          // The first batch exhausted the cursor, the only thing to clean up is the session
+          expect(cursor.session.hasEnded).to.be.true;
         }
       });
     });
@@ -259,7 +263,9 @@ describe('class AbstractCursor', function () {
 
         const error = await cursor.forEach(iterator).catch(e => e);
         expect(error).to.be.instanceOf(MongoAPIError);
-        expect(cursor.closed).to.be.true;
+        expect(cursor.id.isZero()).to.be.true;
+        // The first batch exhausted the cursor, the only thing to clean up is the session
+        expect(cursor.session.hasEnded).to.be.true;
       });
     });
   });
@@ -299,4 +305,66 @@ describe('class AbstractCursor', function () {
       expect(error).to.be.instanceof(MongoServerError);
     });
   });
+
+  describe('cursor end state', function () {
+    let client: MongoClient;
+    let cursor: FindCursor;
+
+    beforeEach(async function () {
+      client = this.configuration.newClient();
+      const test = client.db().collection('test');
+      await test.deleteMany({});
+      await test.insertMany([{ a: 1 }, { a: 2 }, { a: 3 }, { a: 4 }]);
+    });
+
+    afterEach(async function () {
+      await cursor.close();
+      await client.close();
+    });
+
+    describe('when the last batch has been received', () => {
+      it('has a zero id and is not closed and is never killed', async function () {
+        cursor = client.db().collection('test').find({});
+        expect(cursor).to.have.property('closed', false);
+        await cursor.tryNext();
+        expect(cursor.id.isZero()).to.be.true;
+        expect(cursor).to.have.property('closed', false);
+        expect(cursor).to.have.property('killed', false);
+      });
+    });
+
+    describe('when the last document has been iterated', () => {
+      it('has a zero id and is closed and is never killed', async function () {
+        cursor = client.db().collection('test').find({});
+        await cursor.next();
+        await cursor.next();
+        await cursor.next();
+        await cursor.next();
+        expect(await cursor.next()).to.be.null;
+        expect(cursor.id.isZero()).to.be.true;
+        expect(cursor).to.have.property('closed', true);
+        expect(cursor).to.have.property('killed', false);
+      });
+    });
+
+    describe('when some documents have been iterated and the cursor is closed', () => {
+      it('has a nonzero id and is closed and is killed', async function () {
+        cursor = client.db().collection('test').find({}, { batchSize: 2 });
+        await cursor.next();
+        await cursor.close();
+        expect(cursor).to.have.property('closed', false);
+        expect(cursor).to.have.property('killed', true);
+        expect(cursor.id.isZero()).to.be.false;
+
+        // After closing, next still returns just fine.
+        // I do not think this is part of our expected/intended use
+        // but I leave these assertions here so when/if a change is made
+        // it is done intentionally
+        expect(await cursor.next()).to.have.property('a', 2);
+        expect(await cursor.next()).to.be.null;
+        // now closed is true since we finished all the local documents
+        expect(cursor).to.have.property('closed', true);
+      });
+    });
+  });
 });

test/integration/node-specific/cursor_async_iterator.test.js

@@ -95,7 +95,7 @@ describe('Cursor Async Iterator Tests', function () {
       }
 
       expect(count).to.equal(1);
-      expect(cursor.closed).to.be.true;
+      expect(cursor.killed).to.be.true;
     });
 
     it('cleans up cursor when breaking out of for await of loops', async function () {
@@ -106,7 +106,8 @@ describe('Cursor Async Iterator Tests', function () {
         break;
       }
 
-      expect(cursor.closed).to.be.true;
+      // The expectation is that we have "cleaned" up the cursor on the server side
+      expect(cursor.killed).to.be.true;
     });
 
     it('returns when attempting to reuse the cursor after a break', async function () {
@@ -118,7 +119,7 @@ describe('Cursor Async Iterator Tests', function () {
         break;
       }
 
-      expect(cursor.closed).to.be.true;
+      expect(cursor.killed).to.be.true;
 
       for await (const doc of cursor) {
         expect.fail('Async generator returns immediately if cursor is closed', doc);