feat: Unify logging behavior across environments (#4416)

ionic-team · Apr 29, 2021 · bae0f3d · bae0f3d
1 parent c16ccc0
commit bae0f3d
Show file tree

Hide file tree

Showing 30 changed files with 265 additions and 62 deletions.
diff --git a/android/capacitor/src/main/assets/native-bridge.js b/android/capacitor/src/main/assets/native-bridge.js
@@ -369,7 +369,7 @@ const nativeBridge = (function (exports) {
                             methodName: methodName,
                             options: options || {},
                         };
-                        if (cap.DEBUG && pluginName !== 'Console') {
+                        if (cap.isLoggingEnabled && pluginName !== 'Console') {
                             cap.logToNative(callData);
                         }
                         // post the call data to native
@@ -390,7 +390,7 @@ const nativeBridge = (function (exports) {
              */
             cap.fromNative = result => {
                 var _a, _b;
-                if (cap.DEBUG && result.pluginId !== 'Console') {
+                if (cap.isLoggingEnabled && result.pluginId !== 'Console') {
                     cap.logFromNative(result);
                 }
                 // get the stored call, if it exists

diff --git a/android/capacitor/src/main/java/com/getcapacitor/Bridge.java b/android/capacitor/src/main/java/com/getcapacitor/Bridge.java
@@ -755,7 +755,7 @@ public <I, O> ActivityResultLauncher<I> registerForActivityResult(
      */
     private JSInjector getJSInjector() {
         try {
-            String globalJS = JSExport.getGlobalJS(context, isDevMode());
+            String globalJS = JSExport.getGlobalJS(context, config.isLoggingEnabled(), isDevMode());
             String bridgeJS = JSExport.getBridgeJS(context);
             String pluginJS = JSExport.getPluginJS(plugins.values());
             String cordovaJS = JSExport.getCordovaJS(context);

diff --git a/android/capacitor/src/main/java/com/getcapacitor/CapConfig.java b/android/capacitor/src/main/java/com/getcapacitor/CapConfig.java
@@ -11,6 +11,7 @@
 import java.io.IOException;
 import java.util.HashMap;
 import java.util.Iterator;
+import java.util.Locale;
 import java.util.Map;
 import org.json.JSONException;
 import org.json.JSONObject;
@@ -20,6 +21,10 @@
  */
 public class CapConfig {
 
+    private static final String LOG_BEHAVIOR_NONE = "none";
+    private static final String LOG_BEHAVIOR_DEBUG = "debug";
+    private static final String LOG_BEHAVIOR_PRODUCTION = "production";
+
     // Server Config
     private boolean html5mode = true;
     private String serverUrl;
@@ -34,7 +39,7 @@ public class CapConfig {
     private boolean allowMixedContent = false;
     private boolean captureInput = false;
     private boolean webContentsDebuggingEnabled = false;
-    private boolean hideLogs = false;
+    private boolean loggingEnabled = true;
 
     // Embedded
     private String startPath;
@@ -110,7 +115,7 @@ private CapConfig(Builder builder) {
         this.allowMixedContent = builder.allowMixedContent;
         this.captureInput = builder.captureInput;
         this.webContentsDebuggingEnabled = builder.webContentsDebuggingEnabled;
-        this.hideLogs = builder.hideLogs;
+        this.loggingEnabled = builder.loggingEnabled;
 
         // Embedded
         this.startPath = builder.startPath;
@@ -137,6 +142,8 @@ private void loadConfig(AssetManager assetManager) {
      * Deserializes the config from JSON into a Capacitor Configuration object.
      */
     private void deserializeConfig(@Nullable Context context) {
+        boolean isDebug = context != null && (context.getApplicationInfo().flags & ApplicationInfo.FLAG_DEBUGGABLE) != 0;
+
         // Server
         html5mode = JSONUtils.getBoolean(configJSON, "server.html5mode", html5mode);
         serverUrl = JSONUtils.getString(configJSON, "server.url", null);
@@ -158,9 +165,27 @@ private void deserializeConfig(@Nullable Context context) {
                 JSONUtils.getBoolean(configJSON, "allowMixedContent", allowMixedContent)
             );
         captureInput = JSONUtils.getBoolean(configJSON, "android.captureInput", captureInput);
-        hideLogs = JSONUtils.getBoolean(configJSON, "android.hideLogs", JSONUtils.getBoolean(configJSON, "hideLogs", hideLogs));
-        webContentsDebuggingEnabled = context != null && (context.getApplicationInfo().flags & ApplicationInfo.FLAG_DEBUGGABLE) != 0;
-        webContentsDebuggingEnabled = JSONUtils.getBoolean(configJSON, "android.webContentsDebuggingEnabled", webContentsDebuggingEnabled);
+        webContentsDebuggingEnabled = JSONUtils.getBoolean(configJSON, "android.webContentsDebuggingEnabled", isDebug);
+
+        String logBehavior = JSONUtils.getString(
+            configJSON,
+            "android.loggingBehavior",
+            JSONUtils.getString(configJSON, "loggingBehavior", null)
+        );
+        if (logBehavior == null) {
+            boolean hideLogs = JSONUtils.getBoolean(configJSON, "android.hideLogs", JSONUtils.getBoolean(configJSON, "hideLogs", false));
+            logBehavior = hideLogs ? LOG_BEHAVIOR_NONE : LOG_BEHAVIOR_DEBUG;
+        }
+        switch (logBehavior.toLowerCase(Locale.ROOT)) {
+            case LOG_BEHAVIOR_PRODUCTION:
+                loggingEnabled = true;
+                break;
+            case LOG_BEHAVIOR_NONE:
+                loggingEnabled = false;
+                break;
+            default: // LOG_BEHAVIOR_DEBUG
+                loggingEnabled = isDebug;
+        }
 
         // Plugins
         pluginsConfiguration = deserializePluginsConfig(JSONUtils.getObject(configJSON, "plugins"));
@@ -214,8 +239,8 @@ public boolean isWebContentsDebuggingEnabled() {
         return webContentsDebuggingEnabled;
     }
 
-    public boolean isLogsHidden() {
-        return hideLogs;
+    public boolean isLoggingEnabled() {
+        return loggingEnabled;
     }
 
     public PluginConfig getPluginConfiguration(String pluginId) {
@@ -372,7 +397,7 @@ public static class Builder {
         private boolean allowMixedContent = false;
         private boolean captureInput = false;
         private Boolean webContentsDebuggingEnabled = null;
-        private boolean hideLogs = false;
+        private boolean loggingEnabled = true;
 
         // Embedded
         private String startPath = null;
@@ -467,8 +492,8 @@ public Builder setWebContentsDebuggingEnabled(boolean webContentsDebuggingEnable
             return this;
         }
 
-        public Builder setLogsHidden(boolean hideLogs) {
-            this.hideLogs = hideLogs;
+        public Builder setLoggingEnabled(boolean enabled) {
+            this.loggingEnabled = enabled;
             return this;
         }
     }

diff --git a/android/capacitor/src/main/java/com/getcapacitor/JSExport.java b/android/capacitor/src/main/java/com/getcapacitor/JSExport.java
@@ -17,8 +17,8 @@ public class JSExport {
     private static String CATCHALL_OPTIONS_PARAM = "_options";
     private static String CALLBACK_PARAM = "_callback";
 
-    public static String getGlobalJS(Context context, boolean isDebug) {
-        return "window.Capacitor = { DEBUG: " + isDebug + ", Plugins: {} };";
+    public static String getGlobalJS(Context context, boolean loggingEnabled, boolean isDebug) {
+        return "window.Capacitor = { DEBUG: " + isDebug + ", isLoggingEnabled: " + loggingEnabled + ", Plugins: {} };";
     }
 
     public static String getCordovaJS(Context context) {

diff --git a/android/capacitor/src/main/java/com/getcapacitor/Logger.java b/android/capacitor/src/main/java/com/getcapacitor/Logger.java
@@ -98,6 +98,6 @@ public static void error(String tag, String message, Throwable e) {
     }
 
     protected static boolean shouldLog() {
-        return config == null || !config.isLogsHidden();
+        return config == null || config.isLoggingEnabled();
     }
 }
diff --git a/android/capacitor/src/test/java/com/getcapacitor/ConfigBuildingTest.java b/android/capacitor/src/test/java/com/getcapacitor/ConfigBuildingTest.java
@@ -46,7 +46,7 @@ public void setup() {
                     .setAllowNavigation(new String[] { "http://www.google.com" })
                     .setAndroidScheme("test")
                     .setCaptureInput(true)
-                    .setLogsHidden(true)
+                    .setLoggingEnabled(true)
                     .setHTML5mode(false)
                     .setOverriddenUserAgentString("test-user-agent")
                     .setAppendedUserAgentString("test-append")
@@ -66,7 +66,7 @@ public void getCoreConfigValues() {
         assertArrayEquals(new String[] { "http://www.google.com" }, config.getAllowNavigation());
         assertEquals("test", config.getAndroidScheme());
         assertTrue(config.isInputCaptured());
-        assertTrue(config.isLogsHidden());
+        assertTrue(config.isLoggingEnabled());
         assertFalse(config.isHTML5Mode());
         assertEquals("test-user-agent", config.getOverriddenUserAgentString());
         assertEquals("test-append", config.getAppendedUserAgentString());

diff --git a/android/capacitor/src/test/java/com/getcapacitor/ConfigReadingTest.java b/android/capacitor/src/test/java/com/getcapacitor/ConfigReadingTest.java
@@ -47,7 +47,7 @@ public void bad() {
             CapConfig config = CapConfig.loadDefault(context);
             assertEquals("not a real domain", config.getServerUrl());
             assertNull(config.getBackgroundColor());
-            assertFalse(config.isLogsHidden());
+            assertFalse(config.isLoggingEnabled());
         } catch (IOException e) {
             fail();
         }
@@ -62,7 +62,7 @@ public void flat() {
             assertEquals("level 1 override", config.getOverriddenUserAgentString());
             assertEquals("level 1 append", config.getAppendedUserAgentString());
             assertEquals("#ffffff", config.getBackgroundColor());
-            assertTrue(config.isLogsHidden());
+            assertFalse(config.isLoggingEnabled());
             assertEquals(1, config.getPluginConfiguration("SplashScreen").getInt("launchShowDuration", 0));
         } catch (IOException e) {
             fail();
@@ -78,7 +78,7 @@ public void hierarchy() {
             assertEquals("level 2 override", config.getOverriddenUserAgentString());
             assertEquals("level 2 append", config.getAppendedUserAgentString());
             assertEquals("#000000", config.getBackgroundColor());
-            assertFalse(config.isLogsHidden());
+            assertFalse(config.isLoggingEnabled());
         } catch (IOException e) {
             fail();
         }

diff --git a/cli/src/config.ts b/cli/src/config.ts
@@ -62,6 +62,8 @@ export async function loadConfig(): Promise<Config> {
     },
   };
 
+  checkExternalConfig(conf);
+
   debug('config: %O', config);
 
   return config;
@@ -435,3 +437,16 @@ const config: CapacitorConfig = ${formatJSObject(extConfig)};
 
 export default config;\n`;
 }
+
+function checkExternalConfig(config: ExtConfigPairs): void {
+  if (
+    typeof config.extConfig.hideLogs !== 'undefined' ||
+    typeof config.extConfig.android?.hideLogs !== 'undefined' ||
+    typeof config.extConfig.ios?.hideLogs !== 'undefined'
+  ) {
+    logger.warn(
+      `The ${c.strong('hideLogs')} configuration option has been deprecated. ` +
+        `Please update to use ${c.strong('loggingBehavior')} instead.`,
+    );
+  }
+}
diff --git a/cli/src/declarations.ts b/cli/src/declarations.ts
@@ -45,10 +45,28 @@ export interface CapacitorConfig {
    * Hide or show the native logs for iOS and Android.
    *
    * @since 2.1.0
+   * @deprecated 3.0.0
    * @default false
    */
   hideLogs?: boolean;
 
+  /**
+   * The build configuration (as defined by the native app) under which Capacitor
+   * will send statements to the log system. This applies to log statements in
+   * native code as well as statements redirected from JavaScript (`console.debug`,
+   * `console.error`, etc.). Enabling logging will let statements render in the
+   * Xcode and Android Studio windows but can leak information on device if enabled
+   * in released builds.
+   *
+   * 'none' = logs are never produced
+   * 'debug' = logs are produced in debug builds but not production builds
+   * 'production' = logs are always produced
+   *
+   * @since 3.0.0
+   * @default debug
+   */
+  loggingBehavior?: 'none' | 'debug' | 'production';
+
   /**
    * User agent of Capacitor Web View.
    *
@@ -152,10 +170,21 @@ export interface CapacitorConfig {
      * Overrides global `hideLogs` option.
      *
      * @since 2.1.0
+     * @deprecated 3.0.0
      * @default false
      */
     hideLogs?: boolean;
 
+    /**
+     * The build configuration under which Capacitor will generate logs on Android.
+     *
+     * Overrides global `loggingBehavior` option.
+     *
+     * @since 3.0.0
+     * @default debug
+     */
+    loggingBehavior?: 'none' | 'debug' | 'production';
+
     /**
      * Allowlist of plugins to include during `npx cap sync` for Android.
      *
@@ -269,10 +298,21 @@ export interface CapacitorConfig {
      * Overrides global `hideLogs` option.
      *
      * @since 1.1.0
+     * @deprecated 3.0.0
      * @default false
      */
     hideLogs?: boolean;
 
+    /**
+     * The build configuration under which Capacitor will generate logs on iOS.
+     *
+     * Overrides global `loggingBehavior` option.
+     *
+     * @since 3.0.0
+     * @default debug
+     */
+    loggingBehavior?: 'none' | 'debug' | 'production';
+
     /**
      * Allowlist of plugins to include during `npx cap sync` for iOS.
      *

diff --git a/core/native-bridge.ts b/core/native-bridge.ts
@@ -437,7 +437,7 @@ const initBridge = (w: any): void => {
             options: options || {},
           };
 
-          if (cap.DEBUG && pluginName !== 'Console') {
+          if (cap.isLoggingEnabled && pluginName !== 'Console') {
             cap.logToNative(callData);
           }
 
@@ -459,7 +459,7 @@ const initBridge = (w: any): void => {
      * Process a response from the native layer.
      */
     cap.fromNative = result => {
-      if (cap.DEBUG && result.pluginId !== 'Console') {
+      if (cap.isLoggingEnabled && result.pluginId !== 'Console') {
         cap.logFromNative(result);
       }
 

diff --git a/core/src/definitions.ts b/core/src/definitions.ts
@@ -52,6 +52,7 @@ export interface CapacitorGlobal {
   ) => void;
 
   DEBUG?: boolean;
+  isLoggingEnabled?: boolean;
 
   // Deprecated in v3, will be removed from v4
 

diff --git a/core/src/runtime.ts b/core/src/runtime.ts
@@ -223,6 +223,7 @@ export const createCapacitor = (win: WindowCapacitor): CapacitorInstance => {
   cap.registerPlugin = registerPlugin;
   cap.Exception = CapacitorException;
   cap.DEBUG = !!cap.DEBUG;
+  cap.isLoggingEnabled = !!cap.isLoggingEnabled;
 
   // Deprecated props
   cap.platform = cap.getPlatform();

diff --git a/core/src/tests/runtime.spec.ts b/core/src/tests/runtime.spec.ts
@@ -45,6 +45,19 @@ describe('runtime', () => {
     expect(cap.DEBUG).toBe(true);
   });
 
+  it('isLoggingEnabled false default', () => {
+    cap = createCapacitor(win);
+    expect(cap.isLoggingEnabled).toBe(false);
+  });
+
+  it('isLoggingEnabled set from window.Capacitor.isLoggingEnabled', () => {
+    (win as any).Capacitor = {
+      isLoggingEnabled: true,
+    };
+    cap = createCapacitor(win);
+    expect(cap.isLoggingEnabled).toBe(true);
+  });
+
   it('cannot reset server url after initializing capacitor', () => {
     win.WEBVIEW_SERVER_URL = 'whatever://home';
     initBridge(win);

diff --git a/ios/Capacitor/Capacitor.xcodeproj/project.pbxproj b/ios/Capacitor/Capacitor.xcodeproj/project.pbxproj
@@ -29,6 +29,7 @@
 		623D691E254C7462002D01D1 /* CAPInstanceConfiguration.m in Sources */ = {isa = PBXBuildFile; fileRef = 623D691C254C7462002D01D1 /* CAPInstanceConfiguration.m */; };
 		625AF1ED258963C700869675 /* WebViewAssetHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = 625AF1EC258963C700869675 /* WebViewAssetHandler.swift */; };
 		6263686025F6EC0100576C1C /* PluginCallAccessorTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 6263685F25F6EC0100576C1C /* PluginCallAccessorTests.m */; };
+		626D2D992613B61E0046CE81 /* hidinglogs.json in CopyFiles */ = {isa = PBXBuildFile; fileRef = 626D2D902613B4BB0046CE81 /* hidinglogs.json */; };
 		62959B162524DA7800A3D7F1 /* CAPPluginCall.h in Headers */ = {isa = PBXBuildFile; fileRef = 62959AE22524DA7700A3D7F1 /* CAPPluginCall.h */; settings = {ATTRIBUTES = (Public, ); }; };
 		62959B172524DA7800A3D7F1 /* JSExport.swift in Sources */ = {isa = PBXBuildFile; fileRef = 62959AE32524DA7700A3D7F1 /* JSExport.swift */; };
 		62959B192524DA7800A3D7F1 /* CAPBridgedPlugin.h in Headers */ = {isa = PBXBuildFile; fileRef = 62959AE52524DA7700A3D7F1 /* CAPBridgedPlugin.h */; settings = {ATTRIBUTES = (Public, ); }; };
@@ -110,6 +111,7 @@
 			dstPath = configurations;
 			dstSubfolderSpec = 1;
 			files = (
+				626D2D992613B61E0046CE81 /* hidinglogs.json in CopyFiles */,
 				62A91C3F2553710E00861508 /* nonjson.json in CopyFiles */,
 				62E0736125535E8700BAAADB /* server.json in CopyFiles */,
 				62E0736225535E8700BAAADB /* bad.json in CopyFiles */,
@@ -157,6 +159,7 @@
 		623D691C254C7462002D01D1 /* CAPInstanceConfiguration.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = CAPInstanceConfiguration.m; sourceTree = "<group>"; };
 		625AF1EC258963C700869675 /* WebViewAssetHandler.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = WebViewAssetHandler.swift; sourceTree = "<group>"; };
 		6263685F25F6EC0100576C1C /* PluginCallAccessorTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = PluginCallAccessorTests.m; sourceTree = "<group>"; };
+		626D2D902613B4BB0046CE81 /* hidinglogs.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = hidinglogs.json; sourceTree = "<group>"; };
 		62959AE22524DA7700A3D7F1 /* CAPPluginCall.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = CAPPluginCall.h; sourceTree = "<group>"; };
 		62959AE32524DA7700A3D7F1 /* JSExport.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = JSExport.swift; sourceTree = "<group>"; };
 		62959AE52524DA7700A3D7F1 /* CAPBridgedPlugin.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = CAPBridgedPlugin.h; sourceTree = "<group>"; };
@@ -382,6 +385,7 @@
 				62E0735225535E6500BAAADB /* server.json */,
 				62E0735325535E6500BAAADB /* bad.json */,
 				62E0735425535E6500BAAADB /* flat.json */,
+				626D2D902613B4BB0046CE81 /* hidinglogs.json */,
 				62E0735525535E6500BAAADB /* hierarchy.json */,
 				62A91C392553710300861508 /* nonjson.json */,
 			);

diff --git a/ios/Capacitor/Capacitor/CAPBridgeViewController.swift b/ios/Capacitor/Capacitor/CAPBridgeViewController.swift
@@ -29,8 +29,8 @@ import Cordova
     override public final func loadView() {
         // load the configuration and set the logging flag
         let configDescriptor = instanceDescriptor()
-        let configuration = InstanceConfiguration(with: configDescriptor)
-        CAPLog.enableLogging = configuration.enableLogging
+        let configuration = InstanceConfiguration(with: configDescriptor, isDebug: CapacitorBridge.isDevEnvironment)
+        CAPLog.enableLogging = configuration.loggingEnabled
         logWarnings(for: configDescriptor)
 
         if configDescriptor.instanceType == .fixed {