Comment fixes

Author: schmidt-sebastian

packages/firestore/src/remote/persistent_stream.ts

@@ -224,7 +224,9 @@ export abstract class PersistentStream<
    * When stop returns, isStarted and isOpen will both return false.
    */
   stop(): void {
-    this.close(PersistentStreamState.Stopped);
+    if (this.isStarted()) {
+      this.close(PersistentStreamState.Stopped);
+    }
   }
 
   /**
@@ -256,7 +258,7 @@ export abstract class PersistentStream<
         // We ignore Promise rejections for cancelled idle checks.
         assert(
           err.code === Code.CANCELLED,
-          'Received unexpected error in idle timeout closure.'
+          `Received unexpected error in idle timeout closure. Expected CANCELLED, but was: ${err}`
         );
       });
   }
@@ -293,7 +295,7 @@ export abstract class PersistentStream<
    * </ul>
    *
    * A new stream can be opened by calling `start` unless `finalState` is set to
-   * `PersistentStreamState.Stoppen`.
+   * `PersistentStreamState.Stopped`.
    *
    * @param finalState the intended state of the stream after closing.
    * @param error the error the connection was closed with.
@@ -324,17 +326,15 @@ export abstract class PersistentStream<
     // inhibit backoff or otherwise manipulate the state in its non-started state.
     this.state = finalState;
 
-    // Clean up the underlying stream because we are no longer interested in
-    // events
+    // Clean up the underlying stream because we are no longer interested in events.
     if (this.stream !== null) {
       this.stream.close();
       this.stream = null;
     }
 
     const listener = this.listener!;
 
-    // Clear the listener to avoid bleeding of events from the underlying
-    // streams
+    // Clear the listener to avoid bleeding of events from the underlying streams.
     this.listener = null;
 
     // If the caller explicitly requested a stream stop, don't notify them of a closing stream (it

packages/firestore/src/remote/remote_store.ts

@@ -708,8 +708,9 @@ export class RemoteStore {
       'onWriteStreamClose() should only be called when the network is enabled'
     );
 
-    // Ignore close if there are no pending writes.
-    if (this.pendingWrites.length > 0) {
+    // If the write stream closed due to an error, invoke the error callbacks if there are pending
+    // writes.
+    if (error && this.pendingWrites.length > 0) {
       assert(
         !!error,
         'We have pending writes, but the write stream closed without an error'

packages/firestore/src/util/async_queue.ts

@@ -24,9 +24,9 @@ export class AsyncQueue {
   // The last promise in the queue.
   private tail: Promise<AnyJs | void> = Promise.resolve();
 
-  // A map of timeout handles to the respective deferred promises. Contains an
-  // entry for each operation that is still queued to be run in the future
-  // (i.e. it has a delay that has not yet elapsed).
+  // A map of timeout handles to their respective deferred promises. Contains an
+  // entry for each operation that is still queued to run in the future (i.e. it
+  // has a delay that has not yet elapsed).
   private delayedOperations: Map<number, Deferred<any>> = new Map();
 
   // visible for testing

packages/firestore/test/integration/util/test_queue.ts

@@ -22,6 +22,10 @@ const IDLE_DISPATCH_DELAY_MS = 60.0 * 1000;
 /** The maximum delay we use in a test run. */
 const TEST_DISPATCH_DELAY_MS = 1.0 * 1000;
 
+/**
+ * Dispatch queue used in the integration tests that caps delayed executions at
+ * 1.0 seconds.
+ */
 export class TestQueue extends AsyncQueue {
   idleTimeoutDeferred: Deferred<void> | null = null;