Correctly disable file logging if initialization fails

If logging initialization failed, the file logger would not be removed
correctly, and it would log to a GB_LOGFILES_DIR_IS_UNDEFINED directory.

Remove the file logger from the root appender, regardless of it being
null or not.

The issue can be easily reproduced before the fix by throwing an
exception in FileUtils#getExternalFilesDir. Before the fix, it would
still log to the aforementioned directory.

Aditionally, update the Settings screen to reflect that writing log
files is not available in such cases.
This commit is contained in:
José Rebelo 2022-08-06 00:31:43 +01:00
parent 7892b8be6a
commit efdc4c434a
3 changed files with 21 additions and 7 deletions

View File

@ -76,13 +76,17 @@ public abstract class Logging {
protected abstract String createLogDirectory() throws IOException;
protected void init() throws IOException {
rememberFileLogger();
String dir = createLogDirectory();
if (dir == null) {
throw new IllegalArgumentException("log directory must not be null");
}
// used by assets/logback.xml since the location cannot be statically determined
System.setProperty(PROP_LOGFILES_DIR, dir);
rememberFileLogger();
}
public boolean isInitialized() {
return System.getProperty(PROP_LOGFILES_DIR) != null;
}
private Logger getLogger() {
@ -100,13 +104,18 @@ public abstract class Logging {
private void stopFileLogger() {
if (fileLogger != null && fileLogger.isStarted()) {
fileLogger.stop();
removeFileLogger(fileLogger);
}
// We still need to remove the logger from the root appender, otherwise slf4j will log to
// a GB_LOGFILES_DIR_IS_UNDEFINED directory (especially if something failed before we got
// the fileLogger
removeFileLogger();
}
private void rememberFileLogger() {
ch.qos.logback.classic.Logger root = (ch.qos.logback.classic.Logger) LoggerFactory.getLogger(Logger.ROOT_LOGGER_NAME);
fileLogger = (FileAppender<ILoggingEvent>) root.getAppender("FILE");
if (fileLogger == null) {
ch.qos.logback.classic.Logger root = (ch.qos.logback.classic.Logger) LoggerFactory.getLogger(Logger.ROOT_LOGGER_NAME);
fileLogger = (FileAppender<ILoggingEvent>) root.getAppender("FILE");
}
}
private void addFileLogger(Appender<ILoggingEvent> fileLogger) {
@ -120,10 +129,10 @@ public abstract class Logging {
}
}
private void removeFileLogger(Appender<ILoggingEvent> fileLogger) {
private void removeFileLogger() {
try {
ch.qos.logback.classic.Logger root = (ch.qos.logback.classic.Logger) LoggerFactory.getLogger(Logger.ROOT_LOGGER_NAME);
if (root.isAttached(fileLogger)) {
if (fileLogger != null && root.isAttached(fileLogger)) {
root.detachAppender(fileLogger);
}
} catch (Throwable ex) {

View File

@ -184,7 +184,11 @@ public class SettingsActivity extends AbstractSettingsActivity {
});
// If we didn't manage to initialize file logging, disable the preference
if (!GBApplication.getLogging().isInitialized()) {
pref.setEnabled(false);
pref.setSummary(R.string.pref_write_logfiles_not_available);
}
pref = findPreference("language");
pref.setOnPreferenceChangeListener(new Preference.OnPreferenceChangeListener() {

View File

@ -480,6 +480,7 @@
<string name="watch9_pairing_tap_hint">When your watch vibrates, shake the device or press its button.</string>
<string name="title_activity_sleepmonitor">Sleep monitor</string>
<string name="pref_write_logfiles">Write log files</string>
<string name="pref_write_logfiles_not_available">File logging initialization failed, writing log files is not available</string>
<string name="initializing">Initializing</string>
<string name="busy_task_fetch_activity_data">Fetching activity data</string>
<string name="sleep_activity_date_range">From %1$s to %2$s</string>