fix: close default BulkWriter upon terminate. (#2276)

* fix: close default BulkWriter upon terminate.

* fix test

* fix test

Author: tom-andersen

dev/src/bulk-writer.ts

@@ -399,13 +399,13 @@ export class BulkWriter {
   private _lastOp: Promise<void> = Promise.resolve();
 
   /**
-   * Whether this BulkWriter instance has started to close. Afterwards, no
-   * new operations can be enqueued, except for retry operations scheduled by
-   * the error handler.
+   * When this BulkWriter instance has started to close, a flush promise is
+   * saved. Afterwards, no new operations can be enqueued, except for retry
+   * operations scheduled by the error handler.
    * @private
    * @internal
    */
-  private _closing = false;
+  private _closePromise: Promise<void> | undefined;
 
   /**
    * Rate limiter used to throttle requests as per the 500/50/5 rule.
@@ -921,11 +921,11 @@ export class BulkWriter {
    * ```
    */
   close(): Promise<void> {
-    this._verifyNotClosed();
-    this.firestore._decrementBulkWritersCount();
-    const flushPromise = this.flush();
-    this._closing = true;
-    return flushPromise;
+    if (!this._closePromise) {
+      this._closePromise = this.flush();
+      this.firestore._decrementBulkWritersCount();
+    }
+    return this._closePromise;
   }
 
   /**
@@ -934,7 +934,7 @@ export class BulkWriter {
    * @internal
    */
   _verifyNotClosed(): void {
-    if (this._closing) {
+    if (this._closePromise) {
       throw new Error('BulkWriter has already been closed.');
     }
   }

dev/src/index.ts

@@ -1487,7 +1487,11 @@ export class Firestore implements firestore.Firestore {
    *
    * @return A Promise that resolves when the client is terminated.
    */
-  terminate(): Promise<void> {
+  async terminate(): Promise<void> {
+    if (this._bulkWriter) {
+      await this._bulkWriter.close();
+      this._bulkWriter = undefined;
+    }
     if (this.registeredListenersCount > 0 || this.bulkWritersCount > 0) {
       return Promise.reject(
         'All onSnapshot() listeners must be unsubscribed, and all BulkWriter ' +

dev/system-test/firestore.ts

@@ -6967,7 +6967,11 @@ describe('BulkWriter class', () => {
     writer = firestore.bulkWriter();
   });
 
-  afterEach(() => verifyInstance(firestore));
+  afterEach(async () => {
+    await writer.close();
+    await verifyInstance(firestore);
+    await firestore.terminate();
+  });
 
   it('has create() method', async () => {
     const ref = randomCol.doc('doc1');
@@ -7131,6 +7135,7 @@ describe('BulkWriter class', () => {
       });
       await firestore.recursiveDelete(randomCol, bulkWriter);
       expect(callbackCount).to.equal(6);
+      await bulkWriter.close();
     });
   });
 

dev/test/bulk-writer.ts

@@ -488,7 +488,9 @@ describe('BulkWriter', () => {
     expect(() => bulkWriter.create(doc, {})).to.throw(expected);
     expect(() => bulkWriter.update(doc, {})).to.throw(expected);
     expect(() => bulkWriter.flush()).to.throw(expected);
-    expect(() => bulkWriter.close()).to.throw(expected);
+
+    // Calling close() multiple times is allowed.
+    await bulkWriter.close();
   });
 
   it('send writes to the same documents in the different batches', async () => {