Review

Author: schmidt-sebastian

packages/firestore/src/local/local_store.ts

@@ -879,11 +879,10 @@ export class LocalStore {
     keepPersistedTargetData: boolean
   ): Promise<void> {
     const targetData = this.targetDataByTarget.get(targetId);
-
-    if (targetData === null) {
-      logDebug(`Ignoring release of nonexistent target: ${targetId}`);
-      return Promise.resolve();
-    }
+    debugAssert(
+      targetData !== null,
+      `Tried to release nonexistent target: ${targetId}`
+    );
 
     const mode = keepPersistedTargetData ? 'readwrite' : 'readwrite-primary';
     return this.persistence

packages/firestore/test/unit/specs/recovery_spec.test.ts

@@ -102,6 +102,7 @@ describeSpec(
             // The primary client 0 receives a notification that the query can
             // be released, but it can only process the change after we recover
             // the database.
+            .expectActiveTargets({ query})
             .recoverDatabase()
             .runTimer(TimerId.AsyncQueueRetry)
             .expectActiveTargets()

packages/firestore/test/unit/specs/spec_builder.ts

@@ -446,7 +446,7 @@ export class SpecBuilder {
 
   /** Overrides the currently expected set of active targets. */
   expectActiveTargets(
-    ...targets: Array<{ query: Query; resumeToken: string }>
+    ...targets: Array<{ query: Query; resumeToken?: string }>
   ): this {
     this.assertStep('Active target expectation requires previous step');
     const currentStep = this.currentStep!;