You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
While trying to debug an issue in the Messages app, I noticed that the code is littered with empty catch statements that silence exceptions (added in commits such as this). This is bad practice and makes it very difficult to debug.
I propose to change this practice and instead log such exceptions.
I'm opening this discussion instead of jumping to a PR because it would affect code across multiple apps in many places, and because there are a couple of things to consider:
1. Existing logging practices across the apps (specifically lack thereof).
I had a look at a couple of apps and didn't see any logging at all (I searched for references to Android's Log class and didn't find any in live code). Is there a specific reason behind that?
(Aside: The standard practice for logging is to declare a TAG variable in each class, which is tedious, but in Kotlin there is a nicer way.)
2. What should the logging code look like?
The simplest way would be: catch (e: Exception) { Log.w(TAG, "Suppressing exception", e) }
However, it might be worth using a custom helper method. For example, retrieving the Android logs is tricky for non-developers that don't have adb, so it could be useful to be able to redirect the logs to a custom file that the user can open (either using a preference or by changing the code). I don't think it's possible to intercept calls to Log.w() etc., and reading the Android logs doesn't seem easy, so the only way to save the app logs to a custom file is to use custom versions of all the logging methods (or use some library which does the same, but that seems overkill).
I can write a simple implementation that allows switching between logging to the Android log and logging to a file if people are interested.
3. Which exceptions should be suppressed without logging?
Identifying which exceptions are potentially interesting and which exceptions are never of interest is hard because it requires knowledge of the code. So I propose that to start with, we log all the exceptions that are currently suppressed, and disable logging on a case-by-case basis (for example if a particular line is spamming the log).
4. Logging of exceptions that are displayed to the user.
There are many exceptions that are currently displayed as a toast (using fun Context.showErrorToast(exception: Exception, ...)).
I propose that such exception are additionally logged, because the toast doesn't show the stack trace, disappears quickly, can't be copied, etc.
I also propose that the parameter type of showErrorToast() is changed from Exception to Throwable, to allow logging e.g. OutOfMemoryError (which inherits from Error, not from Exception).
Both of these changes are trivial.
reacted with thumbs up emoji reacted with thumbs down emoji reacted with laugh emoji reacted with hooray emoji reacted with confused emoji reacted with heart emoji reacted with rocket emoji reacted with eyes emoji
-
While trying to debug an issue in the Messages app, I noticed that the code is littered with empty
catch
statements that silence exceptions (added in commits such as this). This is bad practice and makes it very difficult to debug.I propose to change this practice and instead log such exceptions.
I'm opening this discussion instead of jumping to a PR because it would affect code across multiple apps in many places, and because there are a couple of things to consider:
1. Existing logging practices across the apps (specifically lack thereof).
I had a look at a couple of apps and didn't see any logging at all (I searched for references to Android's Log class and didn't find any in live code). Is there a specific reason behind that?
(Aside: The standard practice for logging is to declare a TAG variable in each class, which is tedious, but in Kotlin there is a nicer way.)
2. What should the logging code look like?
The simplest way would be:
catch (e: Exception) { Log.w(TAG, "Suppressing exception", e) }
However, it might be worth using a custom helper method. For example, retrieving the Android logs is tricky for non-developers that don't have adb, so it could be useful to be able to redirect the logs to a custom file that the user can open (either using a preference or by changing the code). I don't think it's possible to intercept calls to
Log.w()
etc., and reading the Android logs doesn't seem easy, so the only way to save the app logs to a custom file is to use custom versions of all the logging methods (or use some library which does the same, but that seems overkill).I can write a simple implementation that allows switching between logging to the Android log and logging to a file if people are interested.
3. Which exceptions should be suppressed without logging?
Identifying which exceptions are potentially interesting and which exceptions are never of interest is hard because it requires knowledge of the code. So I propose that to start with, we log all the exceptions that are currently suppressed, and disable logging on a case-by-case basis (for example if a particular line is spamming the log).
4. Logging of exceptions that are displayed to the user.
There are many exceptions that are currently displayed as a toast (using
fun Context.showErrorToast(exception: Exception, ...)
).I propose that such exception are additionally logged, because the toast doesn't show the stack trace, disappears quickly, can't be copied, etc.
I also propose that the parameter type of showErrorToast() is changed from Exception to Throwable, to allow logging e.g. OutOfMemoryError (which inherits from Error, not from Exception).
Both of these changes are trivial.
Beta Was this translation helpful? Give feedback.
All reactions