Warn when stream is closing due to error, not debug. (#3064)

* Warn when stream is closing due to error, not debug. * Add setLogLevel * Improvements. * Add all levels. * Add missing string values for log level
firebase · Jun 1, 2020 · 31f7afc · 31f7afc
1 parent 9b32270
commit 31f7afc
Show file tree

Hide file tree

Showing 5 changed files with 36 additions and 25 deletions.
diff --git a/packages/firestore-types/index.d.ts b/packages/firestore-types/index.d.ts
@@ -37,7 +37,13 @@ export interface PersistenceSettings {
   experimentalTabSynchronization?: boolean;
 }
 
-export type LogLevel = 'debug' | 'error' | 'silent';
+export type LogLevel =
+  | 'debug'
+  | 'error'
+  | 'silent'
+  | 'warn'
+  | 'info'
+  | 'verbose';
 
 export function setLogLevel(logLevel: LogLevel): void;
 

diff --git a/packages/firestore/src/api/database.ts b/packages/firestore/src/api/database.ts
@@ -620,8 +620,16 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService {
     switch (getLogLevel()) {
       case LogLevel.DEBUG:
         return 'debug';
+      case LogLevel.ERROR:
+        return 'error';
       case LogLevel.SILENT:
         return 'silent';
+      case LogLevel.WARN:
+        return 'warn';
+      case LogLevel.INFO:
+        return 'info';
+      case LogLevel.VERBOSE:
+        return 'verbose';
       default:
         // The default log level is error
         return 'error';
@@ -630,23 +638,13 @@ export class Firestore implements firestore.FirebaseFirestore, FirebaseService {
 
   static setLogLevel(level: firestore.LogLevel): void {
     validateExactNumberOfArgs('Firestore.setLogLevel', arguments, 1);
-    validateArgType('Firestore.setLogLevel', 'non-empty string', 1, level);
-    switch (level) {
-      case 'debug':
-        setLogLevel(LogLevel.DEBUG);
-        break;
-      case 'error':
-        setLogLevel(LogLevel.ERROR);
-        break;
-      case 'silent':
-        setLogLevel(LogLevel.SILENT);
-        break;
-      default:
-        throw new FirestoreError(
-          Code.INVALID_ARGUMENT,
-          'Invalid log level: ' + level
-        );
-    }
+    validateStringEnum(
+      'setLogLevel',
+      ['debug', 'error', 'silent', 'warn', 'info', 'verbose'],
+      1,
+      level
+    );
+    setLogLevel(level);
   }
 
   // Note: this is not a property because the minifier can't work correctly with

diff --git a/packages/firestore/src/platform_browser/webchannel_connection.ts b/packages/firestore/src/platform_browser/webchannel_connection.ts
@@ -45,7 +45,7 @@ import {
 import { StreamBridge } from '../remote/stream_bridge';
 import { debugAssert, fail, hardAssert } from '../util/assert';
 import { Code, FirestoreError } from '../util/error';
-import { logDebug } from '../util/log';
+import { logDebug, logWarn } from '../util/log';
 import { Indexable } from '../util/misc';
 import { Rejecter, Resolver } from '../util/promise';
 import { StringMap } from '../util/types';
@@ -352,7 +352,7 @@ export class WebChannelConnection implements Connection {
     unguardedEventListen<Error>(WebChannel.EventType.ERROR, err => {
       if (!closed) {
         closed = true;
-        logDebug(LOG_TAG, 'WebChannel transport errored:', err);
+        logWarn(LOG_TAG, 'WebChannel transport errored:', err);
         streamBridge.callOnClose(
           new FirestoreError(
             Code.UNAVAILABLE,

diff --git a/packages/firestore/src/platform_node/grpc_connection.ts b/packages/firestore/src/platform_node/grpc_connection.ts
@@ -35,7 +35,7 @@ import { mapCodeFromRpcCode } from '../remote/rpc_error';
 import { StreamBridge } from '../remote/stream_bridge';
 import { hardAssert } from '../util/assert';
 import { FirestoreError } from '../util/error';
-import { logError, logDebug } from '../util/log';
+import { logError, logDebug, logWarn } from '../util/log';
 import { NodeCallback, nodePromise } from '../util/node_api';
 import { Deferred } from '../util/promise';
 
@@ -230,7 +230,7 @@ export class GrpcConnection implements Connection {
     });
 
     grpcStream.on('error', (grpcError: ServiceError) => {
-      logDebug(
+      logWarn(
         LOG_TAG,
         'GRPC stream error. Code:',
         grpcError.code,

diff --git a/packages/firestore/src/util/log.ts b/packages/firestore/src/util/log.ts
@@ -15,7 +15,7 @@
  * limitations under the License.
  */
 
-import { Logger, LogLevel } from '@firebase/logger';
+import { Logger, LogLevel, LogLevelString } from '@firebase/logger';
 import { SDK_VERSION } from '../core/version';
 import { PlatformSupport } from '../platform/platform';
 
@@ -28,8 +28,8 @@ export function getLogLevel(): LogLevel {
   return logClient.logLevel;
 }
 
-export function setLogLevel(newLevel: LogLevel): void {
-  logClient.logLevel = newLevel;
+export function setLogLevel(newLevel: LogLevelString | LogLevel): void {
+  logClient.setLogLevel(newLevel);
 }
 
 export function logDebug(msg: string, ...obj: unknown[]): void {
@@ -46,6 +46,13 @@ export function logError(msg: string, ...obj: unknown[]): void {
   }
 }
 
+export function logWarn(msg: string, ...obj: unknown[]): void {
+  if (logClient.logLevel <= LogLevel.WARN) {
+    const args = obj.map(argToString);
+    logClient.warn(`Firestore (${SDK_VERSION}): ${msg}`, ...args);
+  }
+}
+
 /**
  * Converts an additional log parameter to a string representation.
  */