Firestore: Fix "missing index" error message (#6712)

Author: milaGGL

.changeset/little-rats-relax.md

@@ -0,0 +1,6 @@
+---
+'@firebase/firestore': patch
+'firebase': patch
+---
+
+Fix "missing index" error message to include the link to create the composite index.

packages/firestore/src/platform/browser/webchannel_connection.ts

@@ -97,8 +97,11 @@ export class WebChannelConnection extends RestConnection {
                 xhr.getResponseText()
               );
               if (status > 0) {
-                const responseError = (xhr.getResponseJson() as WebChannelError)
-                  .error;
+                let response = xhr.getResponseJson();
+                if (Array.isArray(response)) {
+                  response = response[0];
+                }
+                const responseError = (response as WebChannelError)?.error;
                 if (
                   !!responseError &&
                   !!responseError.status &&

packages/firestore/src/platform/browser_lite/fetch_connection.ts

@@ -69,9 +69,14 @@ export class FetchConnection extends RestConnection {
     }
 
     if (!response.ok) {
+      let errorResponse = await response.json();
+      if (Array.isArray(errorResponse)) {
+        errorResponse = errorResponse[0];
+      }
+      const errorMessage = errorResponse?.error?.message;
       throw new FirestoreError(
         mapCodeFromHttpStatus(response.status),
-        'Request failed with error: ' + response.statusText
+        `Request failed with error: ${errorMessage ?? response.statusText}`
       );
     }
 

packages/firestore/src/util/async_observer.ts

@@ -45,7 +45,7 @@ export class AsyncObserver<T> implements Observer<T> {
     if (this.observer.error) {
       this.scheduleEvent(this.observer.error, error);
     } else {
-      logError('Uncaught Error in snapshot listener:', error);
+      logError('Uncaught Error in snapshot listener:', error.toString());
     }
   }
 

packages/firestore/test/integration/api/aggregation.test.ts

@@ -36,6 +36,7 @@ import {
   withTestCollection,
   withTestDb
 } from '../util/helpers';
+import { USE_EMULATOR } from '../util/settings';
 
 apiDescribe('Count quries', (persistence: boolean) => {
   it('can run count query getCountFromServer', () => {
@@ -118,4 +119,24 @@ apiDescribe('Count quries', (persistence: boolean) => {
       );
     });
   });
+
+  // Only verify the error message for missing indexes when running against
+  // production, since the Firestore Emulator does not require index creation
+  // and will, therefore, never fail in this situation.
+  // eslint-disable-next-line no-restricted-properties
+  (USE_EMULATOR ? it.skip : it)(
+    'getCountFromServer error message is good if missing index',
+    () => {
+      return withEmptyTestCollection(persistence, async coll => {
+        const query_ = query(
+          coll,
+          where('key1', '==', 42),
+          where('key2', '<', 42)
+        );
+        await expect(getCountFromServer(query_)).to.be.eventually.rejectedWith(
+          /index.*https:\/\/console\.firebase\.google\.com/
+        );
+      });
+    }
+  );
 });

packages/firestore/test/integration/api/query.test.ts

@@ -54,9 +54,11 @@ import {
   apiDescribe,
   toChangesArray,
   toDataArray,
+  withEmptyTestCollection,
   withTestCollection,
   withTestDb
 } from '../util/helpers';
+import { USE_EMULATOR } from '../util/settings';
 
 apiDescribe('Queries', (persistence: boolean) => {
   addEqualityMatcher();
@@ -649,6 +651,38 @@ apiDescribe('Queries', (persistence: boolean) => {
     });
   });
 
+  // eslint-disable-next-line no-restricted-properties
+  (USE_EMULATOR ? it.skip : it)(
+    'can catch error message for missing index with error handler',
+    () => {
+      return withEmptyTestCollection(persistence, async coll => {
+        const query_ = query(
+          coll,
+          where('sort', '<=', '2'),
+          where('filter', '==', true)
+        );
+        const deferred = new Deferred<void>();
+
+        const unsubscribe = onSnapshot(
+          query_,
+          () => {
+            deferred.reject();
+          },
+          err => {
+            expect(err.code).to.equal('failed-precondition');
+            expect(err.message).to.exist;
+            expect(err.message).to.match(
+              /index.*https:\/\/console\.firebase\.google\.com/
+            );
+            deferred.resolve();
+          }
+        );
+        await deferred.promise;
+        unsubscribe();
+      });
+    }
+  );
+
   it('can explicitly sort by document ID', () => {
     const testDocs = {
       a: { key: 'a' },

packages/firestore/test/lite/integration.test.ts

@@ -83,7 +83,8 @@ import { runTransaction } from '../../src/lite-api/transaction';
 import { writeBatch } from '../../src/lite-api/write_batch';
 import {
   DEFAULT_PROJECT_ID,
-  DEFAULT_SETTINGS
+  DEFAULT_SETTINGS,
+  USE_EMULATOR
 } from '../integration/util/settings';
 
 import {
@@ -2380,4 +2381,24 @@ describe('Count quries', () => {
       expect(snapshot.data().count).to.equal(3);
     });
   });
+
+  // Only verify the error message for missing indexes when running against
+  // production, since the Firestore Emulator does not require index creation
+  // and will, therefore, never fail in this situation.
+  // eslint-disable-next-line no-restricted-properties
+  (USE_EMULATOR ? it.skip : it)(
+    'getCount error message is good if missing index',
+    () => {
+      return withTestCollection(async coll => {
+        const query_ = query(
+          coll,
+          where('key1', '==', 42),
+          where('key2', '<', 42)
+        );
+        await expect(getCount(query_)).to.be.eventually.rejectedWith(
+          /index.*https:\/\/console\.firebase\.google\.com/
+        );
+      });
+    }
+  );
 });