Protect against user error around badly configured APPLICATION_LOG

[jarofgreen]

If a user specifies APPLICATION_LOG but does not configure where this log is they get:

06-01 21:27:43.668    9702-9702/org.brokenopenapp.acratest E/ACRA﹕ Error while reading application log file
    java.io.FileNotFoundException: /data/data/org.brokenopenapp.acratest/files: open failed: EISDIR (Is a directory)
            at libcore.io.IoBridge.open(IoBridge.java:409)
            at java.io.FileInputStream.<init>(FileInputStream.java:78)
            at android.app.ContextImpl.openFileInput(ContextImpl.java:788)
            at android.content.ContextWrapper.openFileInput(ContextWrapper.java:179)
            at org.acra.collector.LogFileCollector.collectLogFile(LogFileCollector.java:61)
            at org.acra.collector.CrashReportDataFactory.createCrashData(CrashReportDataFactory.java:335)
            at org.acra.ErrorReporter.report(ErrorReporter.java:746)
            at org.acra.ErrorReporter.access$1400(ErrorReporter.java:81)
            at org.acra.ErrorReporter$ReportBuilder.send(ErrorReporter.java:1142)
            at org.acra.ErrorReporter.handleException(ErrorReporter.java:651)
            at org.brokenopenapp.acratest.MainActivity.crashRuntimeException(MainActivity.java:38)
            at org.brokenopenapp.acratest.MainActivity.onClickCrash1(MainActivity.java:21)
            at java.lang.reflect.Method.invokeNative(Native Method)
            at java.lang.reflect.Method.invoke(Method.java:515)
            at android.view.View$1.onClick(View.java:3818)
            at android.view.View.performClick(View.java:4438)
            at android.view.View$PerformClick.run(View.java:18422)
            at android.os.Handler.handleCallback(Handler.java:733)
            at android.os.Handler.dispatchMessage(Handler.java:95)
            at android.os.Looper.loop(Looper.java:136)
            at android.app.ActivityThread.main(ActivityThread.java:5001)
            at java.lang.reflect.Method.invokeNative(Native Method)
            at java.lang.reflect.Method.invoke(Method.java:515)
            at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:785)
            at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:601)
            at dalvik.system.NativeStart.main(Native Method)
     Caused by: libcore.io.ErrnoException: open failed: EISDIR (Is a directory)
            at libcore.io.IoBridge.open(IoBridge.java:398)
            at java.io.FileInputStream.<init>(FileInputStream.java:78)
            at android.app.ContextImpl.openFileInput(ContextImpl.java:788)
            at android.content.ContextWrapper.openFileInput(ContextWrapper.java:179)
            at org.acra.collector.LogFileCollector.collectLogFile(LogFileCollector.java:61)
            at org.acra.collector.CrashReportDataFactory.createCrashData(CrashReportDataFactory.java:335)
            at org.acra.ErrorReporter.report(ErrorReporter.java:746)
            at org.acra.ErrorReporter.access$1400(ErrorReporter.java:81)
            at org.acra.ErrorReporter$ReportBuilder.send(ErrorReporter.java:1142)
            at org.acra.ErrorReporter.handleException(ErrorReporter.java:651)
            at org.brokenopenapp.acratest.MainActivity.crashRuntimeException(MainActivity.java:38)
            at org.brokenopenapp.acratest.MainActivity.onClickCrash1(MainActivity.java:21)
            at java.lang.reflect.Method.invokeNative(Native Method)
            at java.lang.reflect.Method.invoke(Method.java:515)
            at android.view.View$1.onClick(View.java:3818)
            at android.view.View.performClick(View.java:4438)
            at android.view.View$PerformClick.run(View.java:18422)
            at android.os.Handler.handleCallback(Handler.java:733)
            at android.os.Handler.dispatchMessage(Handler.java:95)
            at android.os.Looper.loop(Looper.java:136)
            at android.app.ActivityThread.main(ActivityThread.java:5001)
            at java.lang.reflect.Method.invokeNative(Native Method)
            at java.lang.reflect.Method.invoke(Method.java:515)
            at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:785)
            at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:601)
            at dalvik.system.NativeStart.main(Native Method)

This is caused by

            // Application specific log file
            if (crashReportFields.contains(APPLICATION_LOG)) {
                try {
                    final String logFile = LogFileCollector.collectLogFile(context,
                                                                           ACRA.getConfig().applicationLogFile(),
                                                                           ACRA.getConfig().applicationLogFileLines());
                    crashReportData.put(APPLICATION_LOG, logFile);
                } catch (IOException e) {
                    ACRA.log.e(LOG_TAG, "Error while reading application log file " + ACRA.getConfig().applicationLogFile(), e);
                }
            }

If the field is set, then it will try to include it even if the other values are not configured. Some kind of check like:

            // Application specific log file
            if (crashReportFields.contains(APPLICATION_LOG) && ACRA.getConfig().applicationLogFile().length() > 0) {

Would prevent this.

Actually, I would probably add a isApplicationLogFileSet() method that returned a boolean to ACRAConfiguration so that there is a clearer API (not relying on an empty string being returned) and it could be tested.

This is technically user error, but it would catch people who just stick every field going in customReportContent without understanding what they do. Cough, Cough.

[william-ferguson-au]

Probably better to log a clearer error if no config has been provided. Eg

if (crashReportFields.contains(APPLICATION_LOG)) {
   if (ACRA.getConfig().applicationLogFile().length() == 0) {
      ACRA.log.e(LOG_TAG, "Error while reading application log file - log file not specified.  You need to include a value for .. etc ..");
   } else {
     .. // normal handling
   }
}

[jarofgreen]

Good point, that would be more useful :-)

[william-ferguson-au]

Resolved by capturing any badly configured application log filename.