Fix log reporting for release builds (#1916) (#1952)

* Fix log reporting for release builds

* Fix logs for release builds

* wip

* Clean up the branch to exclude unrelated changes

* With java docs

* Uncomment quiz checker

* Check for external storage permissions before sending logs

* With more java docs

* Fix crash while zipping log files

* Do not log token and cookies

* Add instruction to restart app

Author: maskaravivek

app/build.gradle

@@ -13,16 +13,24 @@ dependencies {
     implementation 'fr.avianey.com.viewpagerindicator:library:2.4.1.1@aar'
     implementation 'in.yuvi:http.fluent:1.3'
     implementation 'com.github.chrisbanes:PhotoView:2.0.0'
+
     implementation 'ch.acra:acra:4.9.2'
+
     implementation 'org.mediawiki:api:1.3'
     implementation 'commons-codec:commons-codec:1.10'
     implementation 'com.github.pedrovgs:renderers:3.3.3'
-    implementation 'com.google.code.gson:gson:2.8.1'
-    implementation 'com.jakewharton.timber:timber:4.5.1'
+    implementation 'com.google.code.gson:gson:2.8.5'
+    implementation 'com.jakewharton.timber:timber:4.4.0'
     implementation 'info.debatty:java-string-similarity:0.24'
     implementation 'com.borjabravo:readmoretextview:2.1.0'
 
-    implementation 'com.android.support.constraint:constraint-layout:1.1.0'
+    implementation 'com.android.support.constraint:constraint-layout:1.1.3'
+
+    implementation 'org.slf4j:slf4j-api:1.7.25'
+    api ("com.github.tony19:logback-android-classic:1.1.1-6") {
+        exclude group: 'com.google.android', module: 'android'
+    }
+
     implementation('com.mapbox.mapboxsdk:mapbox-android-sdk:5.5.0@aar') {
         transitive = true
     }

app/prod/release/app-commons-v2.8.3-acra-prod-release.apk

@@ -0,0 +1 @@
+[{"outputType":{"type":"APK"},"apkInfo":{"type":"MAIN","splits":[],"versionCode":90,"versionName":"2.8.3","enabled":true,"outputFile":"app-commons-v2.8.3-acra-prod-release.apk","fullName":"prodRelease","baseName":"prod-release"},"path":"app-commons-v2.8.3-acra-prod-release.apk","properties":{}}]

app/prod/release/output.json

@@ -1,4 +1,70 @@
 -dontobfuscate
 -keep class org.apache.http.** { *; }
 -dontwarn org.apache.http.**
--keep class android.support.v7.widget.ShareActionProvider { *; }
+-keep class android.support.v7.widget.ShareActionProvider { *; }
+
+# --- Butter Knife ---
+# Finder.castParam() is stripped when not needed and ProGuard notes it
+# unnecessarily. When castParam() is needed, it's not stripped. e.g.:
+#
+#  @OnItemSelected(value = R.id.history_entry_list)
+#  void foo(ListView bar) {
+#      L.d("baz");
+#  }
+
+-dontnote butterknife.internal.**
+# --- /Butter Knife ---
+
+# --- Retrofit2 ---
+# Platform calls Class.forName on types which do not exist on Android to determine platform.
+-dontnote retrofit2.Platform
+# Platform used when running on Java 8 VMs. Will not be used at runtime.
+-dontwarn retrofit2.Platform$Java8
+# Retain generic type information for use by reflection by converters and adapters.
+-keepattributes Signature
+# Retain declared checked exceptions for use by a Proxy instance.
+-keepattributes Exceptions
+# --- /Retrofit ---
+
+# --- OkHttp + Okio ---
+-dontwarn okhttp3.**
+-dontwarn okio.**
+# --- /OkHttp + Okio ---
+
+# --- Gson ---
+# https://github.com/google/gson/blob/master/examples/android-proguard-example/proguard.cfg
+
+# Gson uses generic type information stored in a class file when working with fields. Proguard
+# removes such information by default, so configure it to keep all of it.
+-keepattributes Signature
+
+# For using GSON @Expose annotation
+-keepattributes *Annotation*
+
+# Gson specific classes
+-dontwarn sun.misc.**
+#-keep class com.google.gson.stream.** { *; }
+
+# Application classes that will be serialized/deserialized over Gson
+-keep class com.google.gson.examples.android.model.** { *; }
+
+# Prevent proguard from stripping interface information from TypeAdapterFactory,
+# JsonSerializer, JsonDeserializer instances (so they can be used in @JsonAdapter)
+-keep class * implements com.google.gson.TypeAdapterFactory
+-keep class * implements com.google.gson.JsonSerializer
+-keep class * implements com.google.gson.JsonDeserializer
+# --- /Gson ---
+
+
+# --- /logback ---
+
+-keep class ch.qos.** { *; }
+-keep class org.slf4j.** { *; }
+-keepattributes *Annotation*
+
+-dontwarn ch.qos.logback.core.net.*
+
+# --- /acra ---
+-keep class org.acra.** { *; }
+-keepattributes SourceFile,LineNumberTable
+-keepattributes *Annotation*

app/proguard-rules.txt

@@ -122,6 +122,8 @@
             android:name=".achievements.AchievementsActivity"
             android:label="@string/Achievements" />
 
+        <activity android:name="com.github.pedrovgs.lynx.LynxActivity"/>
+
         <service android:name=".upload.UploadService" />
         <service
             android:name=".auth.WikiAccountAuthenticatorService"
@@ -157,6 +159,11 @@
                 android:resource="@xml/modifications_sync_adapter" />
         </service>
 
+        <service
+            android:name="org.acra.sender.SenderService"
+            android:exported="false"
+            android:process=":acra" />
+
         <provider
             android:name="android.support.v4.content.FileProvider"
             android:authorities="${applicationId}.provider"

app/src/main/AndroidManifest.xml

@@ -1,10 +1,16 @@
 package fr.free.nrw.commons;
 
 import android.annotation.SuppressLint;
+import android.app.Application;
+import android.app.NotificationChannel;
+import android.app.NotificationManager;
 import android.content.Context;
 import android.content.SharedPreferences;
 import android.database.sqlite.SQLiteDatabase;
-import android.support.multidex.MultiDexApplication;
+import android.os.Build;
+import android.os.Process;
+import android.support.annotation.NonNull;
+import android.util.Log;
 
 import com.facebook.drawee.backends.pipeline.Fresco;
 import com.facebook.imagepipeline.core.ImagePipelineConfig;
@@ -25,17 +31,20 @@
 
 import fr.free.nrw.commons.auth.SessionManager;
 import fr.free.nrw.commons.category.CategoryDao;
+import fr.free.nrw.commons.concurrency.BackgroundPoolExceptionHandler;
+import fr.free.nrw.commons.concurrency.ThreadPoolService;
 import fr.free.nrw.commons.contributions.ContributionDao;
 import fr.free.nrw.commons.data.DBOpenHelper;
 import fr.free.nrw.commons.di.ApplicationlessInjection;
+import fr.free.nrw.commons.logging.FileLoggingTree;
+import fr.free.nrw.commons.logging.LogUtils;
 import fr.free.nrw.commons.modifications.ModifierSequenceDao;
 import fr.free.nrw.commons.upload.FileUtils;
 import fr.free.nrw.commons.utils.ContributionUtils;
 import io.reactivex.android.schedulers.AndroidSchedulers;
 import io.reactivex.schedulers.Schedulers;
 import timber.log.Timber;
 
-// TODO: Use ProGuard to rip out reporting when publishing
 @ReportsCrashes(
         mailTo = "[email protected]",
         mode = ReportingInteractionMode.DIALOG,
@@ -44,24 +53,28 @@
         resDialogCommentPrompt = R.string.crash_dialog_comment_prompt,
         resDialogOkToast = R.string.crash_dialog_ok_toast
 )
-public class CommonsApplication extends MultiDexApplication {
-
+public class CommonsApplication extends Application {
     @Inject SessionManager sessionManager;
     @Inject DBOpenHelper dbOpenHelper;
 
     @Inject @Named("default_preferences") SharedPreferences defaultPrefs;
     @Inject @Named("application_preferences") SharedPreferences applicationPrefs;
     @Inject @Named("prefs") SharedPreferences otherPrefs;
+    @Inject
+    @Named("isBeta")
+    boolean isBeta;
 
     public static final String DEFAULT_EDIT_SUMMARY = "Uploaded using [[COM:MOA|Commons Mobile App]]";
 
     public static final String FEEDBACK_EMAIL = "[email protected]";
 
     public static final String FEEDBACK_EMAIL_SUBJECT = "Commons Android App (%s) Feedback";
 
-    public static final String LOGS_PRIVATE_EMAIL = "[email protected]";
+    public static final String NOTIFICATION_CHANNEL_ID_ALL = "CommonsNotificationAll";
 
-    public static final String LOGS_PRIVATE_EMAIL_SUBJECT = "Commons Android App (%s) Logs";
+    /**
+     * Constants End
+     */
 
     private RefWatcher refWatcher;
 
@@ -82,29 +95,86 @@ public void onCreate() {
                 .getInstance(this)
                 .getCommonsApplicationComponent()
                 .inject(this);
+
+        initTimber();
+
 //        Set DownsampleEnabled to True to downsample the image in case it's heavy
         ImagePipelineConfig config = ImagePipelineConfig.newBuilder(this)
                 .setDownsampleEnabled(true)
                 .build();
-        Fresco.initialize(this,config);
-        if (setupLeakCanary() == RefWatcher.DISABLED) {
-            return;
+        try {
+            Fresco.initialize(this, config);
+        } catch (Exception e) {
+            Timber.e(e);
+            // TODO: Remove when we're able to initialize Fresco in test builds.
         }
+
         // Empty temp directory in case some temp files are created and never removed.
         ContributionUtils.emptyTemporaryDirectory();
 
-        Timber.plant(new Timber.DebugTree());
-
-        if (!BuildConfig.DEBUG) {
-            ACRA.init(this);
-        } else {
+        initAcra();
+        if (BuildConfig.DEBUG) {
             Stetho.initializeWithDefaults(this);
         }
 
+        createNotificationChannel(this);
+
+
+        if (setupLeakCanary() == RefWatcher.DISABLED) {
+            return;
+        }
         // Fire progress callbacks for every 3% of uploaded content
         System.setProperty("in.yuvi.http.fluent.PROGRESS_TRIGGER_THRESHOLD", "3.0");
     }
 
+    /**
+     * Plants debug and file logging tree.
+     * Timber lets you plant your own logging trees.
+     *
+     */
+    private void initTimber() {
+        String logFileName = isBeta ? "CommonsBetaAppLogs" : "CommonsAppLogs";
+        String logDirectory = LogUtils.getLogDirectory(isBeta);
+        FileLoggingTree tree = new FileLoggingTree(
+                Log.DEBUG,
+                logFileName,
+                logDirectory,
+                1000,
+                getFileLoggingThreadPool());
+
+        Timber.plant(tree);
+        Timber.plant(new Timber.DebugTree());
+    }
+
+    /**
+     * Remove ACRA's UncaughtExceptionHandler
+     * We do this because ACRA's handler spawns a new process possibly screwing up with a few things
+     */
+    private void initAcra() {
+        Thread.UncaughtExceptionHandler exceptionHandler = Thread.getDefaultUncaughtExceptionHandler();
+        ACRA.init(this);
+        Thread.setDefaultUncaughtExceptionHandler(exceptionHandler);
+    }
+
+    private ThreadPoolService getFileLoggingThreadPool() {
+        return new ThreadPoolService.Builder("file-logging-thread")
+                .setPriority(Process.THREAD_PRIORITY_LOWEST)
+                .setPoolSize(1)
+                .setExceptionHandler(new BackgroundPoolExceptionHandler())
+                .build();
+    }
+
+    public static void createNotificationChannel(@NonNull Context context) {
+        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
+            NotificationManager manager = (NotificationManager) context.getSystemService(Context.NOTIFICATION_SERVICE);
+            NotificationChannel channel = manager.getNotificationChannel(NOTIFICATION_CHANNEL_ID_ALL);
+            if (channel == null) {
+                channel = new NotificationChannel(NOTIFICATION_CHANNEL_ID_ALL,
+                        context.getString(R.string.notifications_channel_name_all), NotificationManager.IMPORTANCE_DEFAULT);
+                manager.createNotificationChannel(channel);
+            }
+        }
+    }
 
     /**
      * Helps in setting up LeakCanary library

app/src/main/java/fr/free/nrw/commons/CommonsApplication.java

@@ -148,39 +148,6 @@ public static boolean isDarkTheme(Context context) {
         return PreferenceManager.getDefaultSharedPreferences(context).getBoolean("theme", false);
     }
 
-    /**
-     * Will be used to fetch the logs generated by the app ever since the beginning of times....
-     * i.e. since the time the app started.
-     *
-     * @return String containing all the logs since the time the app started
-     */
-    public static String getAppLogs() {
-        final String processId = Integer.toString(android.os.Process.myPid());
-
-        StringBuilder stringBuilder = new StringBuilder();
-
-        try {
-            String[] command = new String[]{"logcat","-d","-v","threadtime"};
-
-            Process process = Runtime.getRuntime().exec(command);
-
-            BufferedReader bufferedReader = new BufferedReader(
-                    new InputStreamReader(process.getInputStream())
-            );
-
-            String line;
-            while ((line = bufferedReader.readLine()) != null) {
-                if (line.contains(processId)) {
-                    stringBuilder.append(line);
-                }
-            }
-        } catch (IOException ioe) {
-            Timber.e("getAppLogs failed", ioe);
-        }
-
-        return stringBuilder.toString();
-    }
-
     public static void rateApp(Context context) {
         final String appPackageName = BuildConfig.class.getPackage().getName();
         try {

app/src/main/java/fr/free/nrw/commons/Utils.java

@@ -0,0 +1,23 @@
+package fr.free.nrw.commons.concurrency;
+
+import android.support.annotation.NonNull;
+
+import fr.free.nrw.commons.BuildConfig;
+
+public class BackgroundPoolExceptionHandler implements ExceptionHandler {
+    /**
+     * If an exception occurs on a background thread, this handler will crash for debug builds
+     * but fail silently for release builds.
+     * @param t
+     */
+    @Override
+    public void onException(@NonNull final Throwable t) {
+        //Crash for debug build
+        if (BuildConfig.DEBUG) {
+            Thread thread = new Thread(() -> {
+                throw new RuntimeException(t);
+            });
+            thread.start();
+        }
+    }
+}

app/src/main/java/fr/free/nrw/commons/concurrency/BackgroundPoolExceptionHandler.java

@@ -0,0 +1,39 @@
+package fr.free.nrw.commons.concurrency;
+
+import java.util.concurrent.CancellationException;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Future;
+import java.util.concurrent.ScheduledThreadPoolExecutor;
+import java.util.concurrent.ThreadFactory;
+
+class ExceptionAwareThreadPoolExecutor extends ScheduledThreadPoolExecutor {
+
+    private final ExceptionHandler exceptionHandler;
+
+    public ExceptionAwareThreadPoolExecutor(int corePoolSize, ThreadFactory threadFactory,
+                                            ExceptionHandler exceptionHandler) {
+        super(corePoolSize, threadFactory);
+        this.exceptionHandler = exceptionHandler;
+    }
+
+    @Override
+    protected void afterExecute(Runnable r, Throwable t) {
+        super.afterExecute(r, t);
+        if (t == null && r instanceof Future<?>) {
+            try {
+                Future<?> future = (Future<?>) r;
+                if (future.isDone()) future.get();
+            } catch (CancellationException | InterruptedException e) {
+                //ignore
+            } catch (ExecutionException e) {
+                t = e.getCause() != null ? e.getCause() : e;
+            } catch (Exception e) {
+                t = e;
+            }
+        }
+
+        if (t != null) {
+            exceptionHandler.onException(t);
+        }
+    }
+}