Simplify logging (#2806)

Author: schmidt-sebastian

packages/firestore/src/api/database.ts

@@ -68,10 +68,7 @@ import {
   validateStringEnum,
   valueDescription
 } from '../util/input_validation';
-// eslint-disable-next-line import/no-duplicates
-import * as log from '../util/log';
-// eslint-disable-next-line import/no-duplicates
-import { LogLevel } from '../util/log';
+import { logError, setLogLevel, LogLevel, getLogLevel } from '../util/log';
 import { AutoId } from '../util/misc';
 import * as objUtils from '../util/obj';
 import { Deferred, Rejecter, Resolver } from '../util/promise';
@@ -198,13 +195,13 @@ class FirestoreSettings {
     // Nobody should set timestampsInSnapshots anymore, but the error depends on
     // whether they set it to true or false...
     if (settings.timestampsInSnapshots === true) {
-      log.error(`
+      logError(`
   The timestampsInSnapshots setting now defaults to true and you no
   longer need to explicitly set it. In a future release, the setting
   will be removed entirely and so it is recommended that you remove it
   from your firestore.settings() call now.`);
     } else if (settings.timestampsInSnapshots === false) {
-      log.error(`
+      logError(`
   The timestampsInSnapshots setting will soon be removed. YOU MUST UPDATE
   YOUR CODE.
 
@@ -394,7 +391,7 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService {
 
     if (settings) {
       if (settings.experimentalTabSynchronization !== undefined) {
-        log.error(
+        logError(
           "The 'experimentalTabSynchronization' setting has been renamed to " +
             "'synchronizeTabs'. In a future release, the setting will be removed " +
             'and it is recommended that you update your ' +
@@ -654,15 +651,14 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService {
   }
 
   static get logLevel(): firestore.LogLevel {
-    switch (log.getLogLevel()) {
+    switch (getLogLevel()) {
       case LogLevel.DEBUG:
         return 'debug';
-      case LogLevel.ERROR:
-        return 'error';
       case LogLevel.SILENT:
         return 'silent';
       default:
-        return fail('Unknown log level: ' + log.getLogLevel());
+        // The default log level is error
+        return 'error';
     }
   }
 
@@ -671,13 +667,13 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService {
     validateArgType('Firestore.setLogLevel', 'non-empty string', 1, level);
     switch (level) {
       case 'debug':
-        log.setLogLevel(log.LogLevel.DEBUG);
+        setLogLevel(LogLevel.DEBUG);
         break;
       case 'error':
-        log.setLogLevel(log.LogLevel.ERROR);
+        setLogLevel(LogLevel.ERROR);
         break;
       case 'silent':
-        log.setLogLevel(log.LogLevel.SILENT);
+        setLogLevel(LogLevel.SILENT);
         break;
       default:
         throw new FirestoreError(

packages/firestore/src/api/user_data_writer.ts

@@ -18,7 +18,6 @@
 import * as firestore from '@firebase/firestore-types';
 
 import * as api from '../protos/firestore_proto_api';
-import * as log from '../util/log';
 
 import { DocumentReference, Firestore } from './database';
 import { Blob } from './blob';
@@ -41,6 +40,7 @@ import { forEach } from '../util/obj';
 import { TypeOrder } from '../model/field_value';
 import { ResourcePath } from '../model/path';
 import { isValidResourceName } from '../remote/serializer';
+import { logError } from '../util/log';
 
 export type ServerTimestampBehavior = 'estimate' | 'previous' | 'none';
 
@@ -139,7 +139,7 @@ export class UserDataWriter<T = firestore.DocumentData> {
 
     if (!databaseId.isEqual(this.firestore._databaseId)) {
       // TODO(b/64130202): Somehow support foreign references.
-      log.error(
+      logError(
         `Document ${key} contains a document ` +
           `reference within a different database (` +
           `${databaseId.projectId}/${databaseId.database}) which is not ` +

packages/firestore/src/core/firestore_client.ts

@@ -1,6 +1,6 @@
 /**
  * @license
- * Copyright 2017 Google Inc.
+ * Copyright 2017 Google LLC
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -33,7 +33,7 @@ import { Datastore } from '../remote/datastore';
 import { RemoteStore } from '../remote/remote_store';
 import { AsyncQueue } from '../util/async_queue';
 import { Code, FirestoreError } from '../util/error';
-import { debug } from '../util/log';
+import { logDebug } from '../util/log';
 import { Deferred } from '../util/promise';
 import {
   EventManager,
@@ -324,7 +324,7 @@ export class FirestoreClient {
    * implementation is available in this.persistence.
    */
   private initializeRest(user: User): Promise<void> {
-    debug(LOG_TAG, 'Initializing. user=', user.uid);
+    logDebug(LOG_TAG, 'Initializing. user=', user.uid);
     return this.platform
       .loadConnection(this.databaseInfo)
       .then(async connection => {
@@ -406,7 +406,7 @@ export class FirestoreClient {
   private handleCredentialChange(user: User): Promise<void> {
     this.asyncQueue.verifyOperationInProgress();
 
-    debug(LOG_TAG, 'Credential Changed. Current user: ' + user.uid);
+    logDebug(LOG_TAG, 'Credential Changed. Current user: ' + user.uid);
     return this.syncEngine.handleCredentialChange(user);
   }
 

packages/firestore/src/core/sync_engine.ts

@@ -1,6 +1,6 @@
 /**
  * @license
- * Copyright 2017 Google Inc.
+ * Copyright 2017 Google LLC
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -34,7 +34,7 @@ import { RemoteStore } from '../remote/remote_store';
 import { RemoteSyncer } from '../remote/remote_syncer';
 import { assert, fail } from '../util/assert';
 import { Code, FirestoreError } from '../util/error';
-import * as log from '../util/log';
+import { logDebug } from '../util/log';
 import { primitiveComparator } from '../util/misc';
 import { ObjectMap } from '../util/obj_map';
 import { Deferred } from '../util/promise';
@@ -535,7 +535,7 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer {
       // had, we would have cached the affected documents), and so we will just
       // see any resulting document changes via normal remote document updates
       // as applicable.
-      log.debug(LOG_TAG, 'Cannot apply mutation batch with id: ' + batchId);
+      logDebug(LOG_TAG, 'Cannot apply mutation batch with id: ' + batchId);
       return;
     }
 
@@ -611,7 +611,7 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer {
    */
   async registerPendingWritesCallback(callback: Deferred<void>): Promise<void> {
     if (!this.remoteStore.canUseNetwork()) {
-      log.debug(
+      logDebug(
         LOG_TAG,
         'The network is disabled. The task returned by ' +
           "'awaitPendingWrites()' will not complete until the network is enabled."
@@ -751,7 +751,7 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer {
         this.limboDocumentRefs.addReference(limboChange.key, targetId);
         this.trackLimboChange(limboChange);
       } else if (limboChange instanceof RemovedLimboDocument) {
-        log.debug(LOG_TAG, 'Document no longer in limbo: ' + limboChange.key);
+        logDebug(LOG_TAG, 'Document no longer in limbo: ' + limboChange.key);
         this.limboDocumentRefs.removeReference(limboChange.key, targetId);
         const isReferenced = this.limboDocumentRefs.containsKey(
           limboChange.key
@@ -769,7 +769,7 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer {
   private trackLimboChange(limboChange: AddedLimboDocument): void {
     const key = limboChange.key;
     if (!this.limboTargetsByKey.get(key)) {
-      log.debug(LOG_TAG, 'New document in limbo: ' + key);
+      logDebug(LOG_TAG, 'New document in limbo: ' + key);
       const limboTargetId = this.limboTargetIdGenerator.next();
       const query = Query.atPath(key.path);
       this.limboResolutionsByTarget[limboTargetId] = new LimboResolution(key);
@@ -1047,7 +1047,7 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer {
     if (this.isPrimary) {
       // If we receive a target state notification via WebStorage, we are
       // either already secondary or another tab has taken the primary lease.
-      log.debug(LOG_TAG, 'Ignoring unexpected query state notification.');
+      logDebug(LOG_TAG, 'Ignoring unexpected query state notification.');
       return;
     }
 

packages/firestore/src/local/index_free_query_engine.ts

@@ -1,6 +1,6 @@
 /**
  * @license
- * Copyright 2019 Google Inc.
+ * Copyright 2019 Google LLC
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -19,7 +19,7 @@ import { QueryEngine } from './query_engine';
 import { LocalDocumentsView } from './local_documents_view';
 import { PersistenceTransaction } from './persistence';
 import { PersistencePromise } from './persistence_promise';
-import { Query, LimitType } from '../core/query';
+import { LimitType, Query } from '../core/query';
 import { SnapshotVersion } from '../core/snapshot_version';
 import {
   DocumentKeySet,
@@ -28,7 +28,7 @@ import {
 } from '../model/collections';
 import { Document } from '../model/document';
 import { assert } from '../util/assert';
-import { debug, getLogLevel, LogLevel } from '../util/log';
+import { getLogLevel, LogLevel, logDebug } from '../util/log';
 import { SortedSet } from '../util/sorted_set';
 
 // TOOD(b/140938512): Drop SimpleQueryEngine and rename IndexFreeQueryEngine.
@@ -99,7 +99,7 @@ export class IndexFreeQueryEngine implements QueryEngine {
         }
 
         if (getLogLevel() <= LogLevel.DEBUG) {
-          debug(
+          logDebug(
             'IndexFreeQueryEngine',
             'Re-using previous result from %s to execute query: %s',
             lastLimboFreeSnapshotVersion.toString(),
@@ -194,7 +194,7 @@ export class IndexFreeQueryEngine implements QueryEngine {
     query: Query
   ): PersistencePromise<DocumentMap> {
     if (getLogLevel() <= LogLevel.DEBUG) {
-      debug(
+      logDebug(
         'IndexFreeQueryEngine',
         'Using full collection scan to execute query: %s',
         query.toString()

packages/firestore/src/local/indexeddb_persistence.ts

@@ -26,7 +26,7 @@ import { JsonProtoSerializer } from '../remote/serializer';
 import { assert, fail } from '../util/assert';
 import { AsyncQueue, TimerId } from '../util/async_queue';
 import { Code, FirestoreError } from '../util/error';
-import * as log from '../util/log';
+import { logDebug, logError } from '../util/log';
 import { CancelablePromise } from '../util/promise';
 
 import { decode, encode, EncodedResourcePath } from './encoded_resource_path';
@@ -427,7 +427,7 @@ export class IndexedDbPersistence implements Persistence {
           throw e;
         }
 
-        log.debug(
+        logDebug(
           LOG_TAG,
           'Releasing owner lease after error during lease refresh',
           e
@@ -634,7 +634,7 @@ export class IndexedDbPersistence implements Persistence {
       })
       .next(canActAsPrimary => {
         if (this.isPrimary !== canActAsPrimary) {
-          log.debug(
+          logDebug(
             LOG_TAG,
             `Client ${
               canActAsPrimary ? 'is' : 'is not'
@@ -760,7 +760,7 @@ export class IndexedDbPersistence implements Persistence {
       transaction: PersistenceTransaction
     ) => PersistencePromise<T>
   ): Promise<T> {
-    log.debug(LOG_TAG, 'Starting transaction:', action);
+    logDebug(LOG_TAG, 'Starting transaction:', action);
 
     const simpleDbMode = mode === 'readonly' ? 'readonly' : 'readwrite';
 
@@ -790,7 +790,7 @@ export class IndexedDbPersistence implements Persistence {
             })
             .next(holdsPrimaryLease => {
               if (!holdsPrimaryLease) {
-                log.error(
+                logError(
                   `Failed to obtain primary lease for action '${action}'.`
                 );
                 this.isPrimary = false;
@@ -897,7 +897,7 @@ export class IndexedDbPersistence implements Persistence {
     const store = primaryClientStore(txn);
     return store.get(DbPrimaryClient.key).next(primaryClient => {
       if (this.isLocalClient(primaryClient)) {
-        log.debug(LOG_TAG, 'Releasing primary lease.');
+        logDebug(LOG_TAG, 'Releasing primary lease.');
         return store.delete(DbPrimaryClient.key);
       } else {
         return PersistencePromise.resolve();
@@ -913,7 +913,7 @@ export class IndexedDbPersistence implements Persistence {
     if (updateTimeMs < minAcceptable) {
       return false;
     } else if (updateTimeMs > maxAcceptable) {
-      log.error(
+      logError(
         `Detected an update time that is in the future: ${updateTimeMs} > ${maxAcceptable}`
       );
       return false;
@@ -1007,7 +1007,7 @@ export class IndexedDbPersistence implements Persistence {
       const isZombied =
         this.webStorage.getItem(this.zombiedClientLocalStorageKey(clientId)) !==
         null;
-      log.debug(
+      logDebug(
         LOG_TAG,
         `Client '${clientId}' ${
           isZombied ? 'is' : 'is not'
@@ -1016,7 +1016,7 @@ export class IndexedDbPersistence implements Persistence {
       return isZombied;
     } catch (e) {
       // Gracefully handle if LocalStorage isn't working.
-      log.error(LOG_TAG, 'Failed to get zombied client id.', e);
+      logError(LOG_TAG, 'Failed to get zombied client id.', e);
       return false;
     }
   }
@@ -1033,7 +1033,7 @@ export class IndexedDbPersistence implements Persistence {
       );
     } catch (e) {
       // Gracefully handle if LocalStorage isn't available / working.
-      log.error('Failed to set zombie client id.', e);
+      logError('Failed to set zombie client id.', e);
     }
   }
 

packages/firestore/src/local/local_store.ts

@@ -1,6 +1,6 @@
 /**
  * @license
- * Copyright 2017 Google Inc.
+ * Copyright 2017 Google LLC
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -39,7 +39,7 @@ import {
 import { RemoteEvent, TargetChange } from '../remote/remote_event';
 import { assert } from '../util/assert';
 import { Code, FirestoreError } from '../util/error';
-import * as log from '../util/log';
+import { logDebug } from '../util/log';
 import { primitiveComparator } from '../util/misc';
 import * as objUtils from '../util/obj';
 import { ObjectMap } from '../util/obj_map';
@@ -610,7 +610,7 @@ export class LocalStore {
                 documentBuffer.addEntry(doc, remoteVersion);
                 changedDocs = changedDocs.insert(key, doc);
               } else {
-                log.debug(
+                logDebug(
                   LOG_TAG,
                   'Ignoring outdated watch update for ',
                   key,
@@ -1119,7 +1119,7 @@ export async function ignoreIfPrimaryLeaseLoss(
     err.code === Code.FAILED_PRECONDITION &&
     err.message === PRIMARY_LEASE_LOST_ERROR_MSG
   ) {
-    log.debug(LOG_TAG, 'Unexpectedly lost primary lease');
+    logDebug(LOG_TAG, 'Unexpectedly lost primary lease');
   } else {
     throw err;
   }