From 91ce40bd9bc46087b1908b075c12cc21d300d3be Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sebastian=20K=C3=BCrten?= <sebastian@mobanisto.de>
Date: Wed, 2 Feb 2022 19:22:59 +0100
Subject: [PATCH] Improve logback config for Android; always log message with
 exception

---
 mailbox-android/src/debug/assets/logback.xml        |  2 +-
 .../mailbox/core/tor/AndroidNetworkManager.java     |  2 +-
 mailbox-android/src/release/assets/logback.xml      |  2 +-
 .../mailbox/core/tor/JavaCliNetworkManager.java     |  2 +-
 .../mailbox/core/contacts/ContactsManager.kt        |  4 ++--
 .../briarproject/mailbox/core/db/JdbcDatabase.kt    |  2 +-
 .../org/briarproject/mailbox/core/db/JdbcUtils.kt   |  6 +++---
 .../mailbox/core/lifecycle/LifecycleManagerImpl.kt  |  2 +-
 .../mailbox/core/setup/QrCodeEncoder.kt             |  4 ++--
 .../briarproject/mailbox/core/tor/TorPlugin.java    | 13 +++++++------
 .../org/briarproject/mailbox/core/util/IoUtils.java |  7 ++++---
 .../org/briarproject/mailbox/core/util/LogUtils.kt  |  8 ++++----
 .../mailbox/core/util/NetworkUtils.java             |  3 ++-
 13 files changed, 30 insertions(+), 27 deletions(-)

diff --git a/mailbox-android/src/debug/assets/logback.xml b/mailbox-android/src/debug/assets/logback.xml
index 58a3a18c..57184508 100644
--- a/mailbox-android/src/debug/assets/logback.xml
+++ b/mailbox-android/src/debug/assets/logback.xml
@@ -4,7 +4,7 @@
             <pattern>%logger{12}</pattern>
         </tagEncoder>
         <encoder>
-            <pattern>%msg</pattern>
+            <pattern>[%thread] %msg%n</pattern>
         </encoder>
     </appender>
     <root level="ALL">
diff --git a/mailbox-android/src/main/java/org/briarproject/mailbox/core/tor/AndroidNetworkManager.java b/mailbox-android/src/main/java/org/briarproject/mailbox/core/tor/AndroidNetworkManager.java
index 290a62ab..e457fc9e 100644
--- a/mailbox-android/src/main/java/org/briarproject/mailbox/core/tor/AndroidNetworkManager.java
+++ b/mailbox-android/src/main/java/org/briarproject/mailbox/core/tor/AndroidNetworkManager.java
@@ -183,7 +183,7 @@ public class AndroidNetworkManager implements NetworkManager, Service {
 			}
 			return hasIpv6Unicast;
 		} catch (SocketException e) {
-			logException(LOG, e);
+			logException(LOG, e, "Error while checking for IPv6 only status");
 			return false;
 		}
 	}
diff --git a/mailbox-android/src/release/assets/logback.xml b/mailbox-android/src/release/assets/logback.xml
index 895de00d..603aa5dc 100644
--- a/mailbox-android/src/release/assets/logback.xml
+++ b/mailbox-android/src/release/assets/logback.xml
@@ -4,7 +4,7 @@
             <pattern>%logger{12}</pattern>
         </tagEncoder>
         <encoder>
-            <pattern>%msg</pattern>
+            <pattern>[%thread] %msg%n</pattern>
         </encoder>
     </appender>
     <root level="DEBUG">
diff --git a/mailbox-cli/src/main/java/org/briarproject/mailbox/core/tor/JavaCliNetworkManager.java b/mailbox-cli/src/main/java/org/briarproject/mailbox/core/tor/JavaCliNetworkManager.java
index eed4dcdd..bb32ed6c 100644
--- a/mailbox-cli/src/main/java/org/briarproject/mailbox/core/tor/JavaCliNetworkManager.java
+++ b/mailbox-cli/src/main/java/org/briarproject/mailbox/core/tor/JavaCliNetworkManager.java
@@ -57,7 +57,7 @@ class JavaCliNetworkManager implements NetworkManager {
 				}
 			}
 		} catch (SocketException e) {
-			logException(LOG, e);
+			logException(LOG, e, "Error while getting network status");
 		}
 		return new NetworkStatus(connected, false, !hasIpv4 && hasIpv6Unicast);
 	}
diff --git a/mailbox-core/src/main/java/org/briarproject/mailbox/core/contacts/ContactsManager.kt b/mailbox-core/src/main/java/org/briarproject/mailbox/core/contacts/ContactsManager.kt
index 619d66de..2435f967 100644
--- a/mailbox-core/src/main/java/org/briarproject/mailbox/core/contacts/ContactsManager.kt
+++ b/mailbox-core/src/main/java/org/briarproject/mailbox/core/contacts/ContactsManager.kt
@@ -78,10 +78,10 @@ class ContactsManager @Inject constructor(
         val c: Contact = try {
             call.receive()
         } catch (e: JacksonException) {
-            logException(LOG, e)
+            logException(LOG, e) { "Error while receiving contact" }
             throw BadRequestException("Unable to deserialise Contact: ${e.message}", e)
         } catch (e: UnsupportedMediaTypeException) {
-            logException(LOG, e)
+            logException(LOG, e) { "Error while receiving contact" }
             throw BadRequestException("Unable to deserialise Contact: ${e.message}", e)
         }
 
diff --git a/mailbox-core/src/main/java/org/briarproject/mailbox/core/db/JdbcDatabase.kt b/mailbox-core/src/main/java/org/briarproject/mailbox/core/db/JdbcDatabase.kt
index 208d5f2f..bb01002a 100644
--- a/mailbox-core/src/main/java/org/briarproject/mailbox/core/db/JdbcDatabase.kt
+++ b/mailbox-core/src/main/java/org/briarproject/mailbox/core/db/JdbcDatabase.kt
@@ -265,7 +265,7 @@ abstract class JdbcDatabase(private val dbTypes: DatabaseTypes, private val cloc
             }
         } catch (e: SQLException) {
             // Try to close the connection
-            logException(LOG, e)
+            logException(LOG, e) { "Error while aborting transaction" }
             tryToClose(connection, LOG)
             // Whatever happens, allow the database to close
             connectionsLock.lock()
diff --git a/mailbox-core/src/main/java/org/briarproject/mailbox/core/db/JdbcUtils.kt b/mailbox-core/src/main/java/org/briarproject/mailbox/core/db/JdbcUtils.kt
index ac9e1b7d..d776c5cd 100644
--- a/mailbox-core/src/main/java/org/briarproject/mailbox/core/db/JdbcUtils.kt
+++ b/mailbox-core/src/main/java/org/briarproject/mailbox/core/db/JdbcUtils.kt
@@ -32,7 +32,7 @@ object JdbcUtils {
         try {
             rs?.close()
         } catch (e: SQLException) {
-            logException(logger, e)
+            logException(logger, e) { "Error while closing result set" }
         }
     }
 
@@ -40,7 +40,7 @@ object JdbcUtils {
         try {
             s?.close()
         } catch (e: SQLException) {
-            logException(logger, e)
+            logException(logger, e) { "Error while closing statement" }
         }
     }
 
@@ -48,7 +48,7 @@ object JdbcUtils {
         try {
             c?.close()
         } catch (e: SQLException) {
-            logException(logger, e)
+            logException(logger, e) { "Error while closing connection" }
         }
     }
 
diff --git a/mailbox-core/src/main/java/org/briarproject/mailbox/core/lifecycle/LifecycleManagerImpl.kt b/mailbox-core/src/main/java/org/briarproject/mailbox/core/lifecycle/LifecycleManagerImpl.kt
index e3c5e18a..02a42b93 100644
--- a/mailbox-core/src/main/java/org/briarproject/mailbox/core/lifecycle/LifecycleManagerImpl.kt
+++ b/mailbox-core/src/main/java/org/briarproject/mailbox/core/lifecycle/LifecycleManagerImpl.kt
@@ -133,7 +133,7 @@ internal class LifecycleManagerImpl @Inject constructor(
             startupLatch.countDown()
             SUCCESS
         } catch (e: ServiceException) {
-            logException(LOG, e)
+            logException(LOG, e) { "Error while starting services" }
             SERVICE_ERROR
         } finally {
             startStopWipeSemaphore.release()
diff --git a/mailbox-core/src/main/java/org/briarproject/mailbox/core/setup/QrCodeEncoder.kt b/mailbox-core/src/main/java/org/briarproject/mailbox/core/setup/QrCodeEncoder.kt
index b56acdd4..719e137a 100644
--- a/mailbox-core/src/main/java/org/briarproject/mailbox/core/setup/QrCodeEncoder.kt
+++ b/mailbox-core/src/main/java/org/briarproject/mailbox/core/setup/QrCodeEncoder.kt
@@ -66,7 +66,7 @@ class QrCodeEncoder @Inject constructor(
         val addressString = try {
             torPlugin.hiddenServiceAddress
         } catch (e: DbException) {
-            logException(LOG, e)
+            logException(LOG, e) { "Error while getting hidden service address" }
             return null
         }
         if (addressString == null) {
@@ -85,7 +85,7 @@ class QrCodeEncoder @Inject constructor(
                 setupManager.getSetupToken(txn)
             }
         } catch (e: DbException) {
-            logException(LOG, e)
+            logException(LOG, e) { "Error while getting setup token" }
             return null
         }
         if (tokenString == null) {
diff --git a/mailbox-core/src/main/java/org/briarproject/mailbox/core/tor/TorPlugin.java b/mailbox-core/src/main/java/org/briarproject/mailbox/core/tor/TorPlugin.java
index 73f881b4..489f57d7 100644
--- a/mailbox-core/src/main/java/org/briarproject/mailbox/core/tor/TorPlugin.java
+++ b/mailbox-core/src/main/java/org/briarproject/mailbox/core/tor/TorPlugin.java
@@ -363,7 +363,7 @@ public abstract class TorPlugin
 		try {
 			s = settingsManager.getSettings(SETTINGS_NAMESPACE);
 		} catch (DbException e) {
-			logException(LOG, e);
+			logException(LOG, e, "Error while retrieving settings");
 			s = new Settings();
 		}
 		String privateKey3 = s.get(HS_PRIVATE_KEY_V3);
@@ -384,7 +384,7 @@ public abstract class TorPlugin
 				response = controlConnection.addOnion(privKey, portLines);
 			}
 		} catch (IOException e) {
-			logException(LOG, e);
+			logException(LOG, e, "Error while add onion service");
 			return;
 		}
 		if (!response.containsKey(HS_ADDRESS)) {
@@ -408,7 +408,7 @@ public abstract class TorPlugin
 			try {
 				settingsManager.mergeSettings(s, SETTINGS_NAMESPACE);
 			} catch (DbException e) {
-				logException(LOG, e);
+				logException(LOG, e, "Error while merging settings");
 			}
 		}
 		state.setServicePublished();
@@ -456,7 +456,8 @@ public abstract class TorPlugin
 				controlConnection.shutdownTor("TERM");
 				controlSocket.close();
 			} catch (IOException e) {
-				logException(LOG, e);
+				logException(LOG, e,
+						"Error while sending tor shutdown instructions");
 			}
 		}
 	}
@@ -519,7 +520,7 @@ public abstract class TorPlugin
 			try {
 				if (state.isTorRunning()) enableNetwork(false);
 			} catch (IOException ex) {
-				logException(LOG, ex);
+				logException(LOG, ex, "Error while disabling network");
 			}
 		});
 	}
@@ -572,7 +573,7 @@ public abstract class TorPlugin
 				}
 				enableNetwork(enableNetwork);
 			} catch (IOException e) {
-				logException(LOG, e);
+				logException(LOG, e, "Error while updating connetion status");
 			}
 		});
 	}
diff --git a/mailbox-core/src/main/java/org/briarproject/mailbox/core/util/IoUtils.java b/mailbox-core/src/main/java/org/briarproject/mailbox/core/util/IoUtils.java
index 27b35f45..04a4b483 100644
--- a/mailbox-core/src/main/java/org/briarproject/mailbox/core/util/IoUtils.java
+++ b/mailbox-core/src/main/java/org/briarproject/mailbox/core/util/IoUtils.java
@@ -81,7 +81,8 @@ public class IoUtils {
 		try {
 			if (c != null) c.close();
 		} catch (IOException e) {
-			logException(logger, e);
+			logException(logger, e, () -> "Error while closing " +
+					c.getClass().getSimpleName());
 		}
 	}
 
@@ -89,7 +90,7 @@ public class IoUtils {
 		try {
 			if (s != null) s.close();
 		} catch (IOException e) {
-			logException(logger, e);
+			logException(logger, e, "Error while closing socket");
 		}
 	}
 
@@ -97,7 +98,7 @@ public class IoUtils {
 		try {
 			if (ss != null) ss.close();
 		} catch (IOException e) {
-			logException(logger, e);
+			logException(logger, e, "Error while closing server socket");
 		}
 	}
 
diff --git a/mailbox-core/src/main/java/org/briarproject/mailbox/core/util/LogUtils.kt b/mailbox-core/src/main/java/org/briarproject/mailbox/core/util/LogUtils.kt
index a0ae2f03..70170ead 100644
--- a/mailbox-core/src/main/java/org/briarproject/mailbox/core/util/LogUtils.kt
+++ b/mailbox-core/src/main/java/org/briarproject/mailbox/core/util/LogUtils.kt
@@ -76,13 +76,13 @@ object LogUtils {
     }
 
     @JvmStatic
-    fun logException(logger: Logger, t: Throwable, message: () -> String) {
-        if (logger.isWarnEnabled) logger.warn(message(), t)
+    fun logException(logger: Logger, t: Throwable, message: String) {
+        if (logger.isWarnEnabled) logger.warn(message, t)
     }
 
     @JvmStatic
-    fun logException(logger: Logger, t: Throwable) {
-        if (logger.isWarnEnabled) logger.warn(t.toString(), t)
+    fun logException(logger: Logger, t: Throwable, message: () -> String) {
+        if (logger.isWarnEnabled) logger.warn(message(), t)
     }
 
     fun logFileOrDir(logger: Logger, f: File) {
diff --git a/mailbox-core/src/main/java/org/briarproject/mailbox/core/util/NetworkUtils.java b/mailbox-core/src/main/java/org/briarproject/mailbox/core/util/NetworkUtils.java
index 1fdaf259..bac22af9 100644
--- a/mailbox-core/src/main/java/org/briarproject/mailbox/core/util/NetworkUtils.java
+++ b/mailbox-core/src/main/java/org/briarproject/mailbox/core/util/NetworkUtils.java
@@ -43,7 +43,8 @@ public class NetworkUtils {
 			//noinspection ConstantConditions
 			return ifaces == null ? emptyList() : list(ifaces);
 		} catch (SocketException e) {
-			logException(LOG, e);
+			logException(LOG, e,
+					"Error while retrieving list of network interfaces");
 			return emptyList();
 		}
 	}
-- 
GitLab