From 5116c3e74963f968747e4cc7ee5595a3a743be2f Mon Sep 17 00:00:00 2001 From: Joshua Horton Date: Tue, 20 May 2025 10:57:46 +0700 Subject: [PATCH 1/3] feat(android): log active keyboard, model IDs with reported errors Cherry-pick-of: #13983 To facilitate code investigation and issue repro attempts, we should log the engine's current keyboard, language code, and model when reporting errors. Fixes: KEYMAN-ANDROID-71V (As this adds Sentry logging, I needed a reliable error with inspectable logs for development.) Test-bot: skip --- .../java/com/keyman/engine/KMManager.java | 2 + .../java/com/keyman/engine/util/KMLog.java | 82 +++++++++++++++++++ 2 files changed, 84 insertions(+) diff --git a/android/KMEA/app/src/main/java/com/keyman/engine/KMManager.java b/android/KMEA/app/src/main/java/com/keyman/engine/KMManager.java index 690af82a59..3e6ee5e106 100644 --- a/android/KMEA/app/src/main/java/com/keyman/engine/KMManager.java +++ b/android/KMEA/app/src/main/java/com/keyman/engine/KMManager.java @@ -486,6 +486,8 @@ public final class KMManager { public static void initialize(final Context context, KeyboardType keyboardType) { appContext = context.getApplicationContext(); + // To facilitate tagging events with current keyboard, language, model. + KMLog.setEngineContext(appContext); if (!didCopyAssets || isTestMode()) { // Copy and install assets diff --git a/android/KMEA/app/src/main/java/com/keyman/engine/util/KMLog.java b/android/KMEA/app/src/main/java/com/keyman/engine/util/KMLog.java index cb13cc967e..b1116a3ccb 100644 --- a/android/KMEA/app/src/main/java/com/keyman/engine/util/KMLog.java +++ b/android/KMEA/app/src/main/java/com/keyman/engine/util/KMLog.java @@ -4,35 +4,90 @@ package com.keyman.engine.util; +import static com.keyman.engine.KMManager.KMKey_LexicalModelID; + +import android.annotation.SuppressLint; +import android.content.Context; import android.util.Log; import android.widget.Toast; import com.keyman.engine.BaseActivity; import com.keyman.engine.BuildConfig; import com.keyman.engine.KMManager; +import com.keyman.engine.data.Keyboard; +import com.keyman.engine.data.LexicalModel; import com.keyman.engine.util.DependencyUtil; import com.keyman.engine.util.DependencyUtil.LibraryType; +import java.util.Map; + import io.sentry.Breadcrumb; import io.sentry.Sentry; import io.sentry.SentryLevel; public final class KMLog { + // Needed to facilitate looking up the current keyboard & model. + @SuppressLint("StaticFieldLeak") + private static Context engineContext; + private static final String TAG = "KMLog"; + private static final String KEYBOARD_TAG = "keyboardId"; + private static final String MODEL_TAG = "modelId"; + private static final String LANGCODE_TAG = "languageCode"; + + private static boolean isLogging = false; + + public static void setEngineContext(Context context) { + engineContext = context; + } + + private static void tagDebugInfo() { + String kbdId = ""; + String lngCode = ""; + String modelId = ""; + // Do not risk raising a new error while tagging info for another error. + try { + if (engineContext != null) { + Keyboard kbd = KMManager.getCurrentKeyboardInfo(null); + if (kbd != null) { + kbdId = kbd.getKeyboardID(); + lngCode = kbd.getLanguageCode(); + Map modelMap = KMManager.getAssociatedLexicalModel(kbd.getLanguageID()); + if (modelMap != null) { + modelId = modelMap.get(KMKey_LexicalModelID); + if (modelId == null) { + modelId = ""; + } + } + } + } + } catch (Exception ignored) { + } + Sentry.setExtra(KEYBOARD_TAG, kbdId); + Sentry.setExtra(LANGCODE_TAG, lngCode); + Sentry.setExtra(MODEL_TAG, modelId); + } + /** * Utility to log info and send to Sentry * @param tag String of the caller * @param msg String of the info message */ public static void LogInfo(String tag, String msg) { + if(isLogging) { + return; + } + isLogging = true; if (msg != null && !msg.isEmpty()) { Log.i(tag, msg); if (DependencyUtil.libraryExists(LibraryType.SENTRY) && Sentry.isEnabled()) { + tagDebugInfo(); Sentry.captureMessage(msg, SentryLevel.INFO); } } + isLogging = false; } /** @@ -46,9 +101,15 @@ public final class KMLog { return; } + if(isLogging) { + return; + } + isLogging = true; + Log.i(tag, msg); if (!DependencyUtil.libraryExists(LibraryType.SENTRY) || !Sentry.isEnabled()) { + isLogging = false; return; } @@ -74,7 +135,9 @@ public final class KMLog { crumb.setData("stacktrace", trace); } } + tagDebugInfo(); Sentry.addBreadcrumb(crumb); + isLogging = false; } /** @@ -83,6 +146,10 @@ public final class KMLog { * @param msg String of the error message */ public static void LogError(String tag, String msg) { + if(isLogging) { + return; + } + isLogging = true; if (msg != null && !msg.isEmpty()) { Log.e(tag, msg); @@ -91,9 +158,11 @@ public final class KMLog { } if (DependencyUtil.libraryExists(LibraryType.SENTRY) && Sentry.isEnabled()) { + tagDebugInfo(); Sentry.captureMessage(msg, SentryLevel.ERROR); } } + isLogging = false; } /** @@ -103,6 +172,10 @@ public final class KMLog { * @param e Throwable exception */ public static void LogException(String tag, String msg, Throwable e) { + if(isLogging) { + return; + } + isLogging = true; String errorMsg = ""; if (msg != null && !msg.isEmpty()) { errorMsg = msg + "\n" + e; @@ -116,9 +189,11 @@ public final class KMLog { } if (DependencyUtil.libraryExists(LibraryType.SENTRY) && Sentry.isEnabled()) { + tagDebugInfo(); Sentry.addBreadcrumb(errorMsg); Sentry.captureException(e); } + isLogging = false; } /** @@ -131,7 +206,12 @@ public final class KMLog { */ public static void LogExceptionWithData(String tag, String msg, String objName, Object obj, Throwable e) { + if(isLogging) { + return; + } + isLogging = true; if (obj != null && DependencyUtil.libraryExists(LibraryType.SENTRY) && Sentry.isEnabled()) { + tagDebugInfo(); String objStr = null; try { objStr = obj.toString(); @@ -141,6 +221,8 @@ public final class KMLog { } // Report the original exception LogException(tag, msg, e); + Sentry.removeExtra(objName); } + isLogging = false; } } From c193c3fe17e7fc129894966f4cdcd69e3e348085 Mon Sep 17 00:00:00 2001 From: Joshua Horton Date: Tue, 20 May 2025 11:58:47 +0700 Subject: [PATCH 2/3] feat(android): log installed keyboard count --- .../java/com/keyman/engine/KMManager.java | 2 - .../java/com/keyman/engine/util/KMLog.java | 37 +++++++------------ 2 files changed, 14 insertions(+), 25 deletions(-) diff --git a/android/KMEA/app/src/main/java/com/keyman/engine/KMManager.java b/android/KMEA/app/src/main/java/com/keyman/engine/KMManager.java index 3e6ee5e106..690af82a59 100644 --- a/android/KMEA/app/src/main/java/com/keyman/engine/KMManager.java +++ b/android/KMEA/app/src/main/java/com/keyman/engine/KMManager.java @@ -486,8 +486,6 @@ public final class KMManager { public static void initialize(final Context context, KeyboardType keyboardType) { appContext = context.getApplicationContext(); - // To facilitate tagging events with current keyboard, language, model. - KMLog.setEngineContext(appContext); if (!didCopyAssets || isTestMode()) { // Copy and install assets diff --git a/android/KMEA/app/src/main/java/com/keyman/engine/util/KMLog.java b/android/KMEA/app/src/main/java/com/keyman/engine/util/KMLog.java index b1116a3ccb..9c8f4a8f57 100644 --- a/android/KMEA/app/src/main/java/com/keyman/engine/util/KMLog.java +++ b/android/KMEA/app/src/main/java/com/keyman/engine/util/KMLog.java @@ -6,8 +6,6 @@ package com.keyman.engine.util; import static com.keyman.engine.KMManager.KMKey_LexicalModelID; -import android.annotation.SuppressLint; -import android.content.Context; import android.util.Log; import android.widget.Toast; @@ -15,8 +13,6 @@ import com.keyman.engine.BaseActivity; import com.keyman.engine.BuildConfig; import com.keyman.engine.KMManager; import com.keyman.engine.data.Keyboard; -import com.keyman.engine.data.LexicalModel; -import com.keyman.engine.util.DependencyUtil; import com.keyman.engine.util.DependencyUtil.LibraryType; import java.util.Map; @@ -26,45 +22,40 @@ import io.sentry.Sentry; import io.sentry.SentryLevel; public final class KMLog { - // Needed to facilitate looking up the current keyboard & model. - @SuppressLint("StaticFieldLeak") - private static Context engineContext; - private static final String TAG = "KMLog"; private static final String KEYBOARD_TAG = "keyboardId"; + private static final String KEYBOARD_COUNT_TAG = "installedKeyboardCount"; private static final String MODEL_TAG = "modelId"; private static final String LANGCODE_TAG = "languageCode"; private static boolean isLogging = false; - public static void setEngineContext(Context context) { - engineContext = context; - } - private static void tagDebugInfo() { String kbdId = ""; String lngCode = ""; String modelId = ""; + int kbdCount = 0; // Do not risk raising a new error while tagging info for another error. try { - if (engineContext != null) { - Keyboard kbd = KMManager.getCurrentKeyboardInfo(null); - if (kbd != null) { - kbdId = kbd.getKeyboardID(); - lngCode = kbd.getLanguageCode(); - Map modelMap = KMManager.getAssociatedLexicalModel(kbd.getLanguageID()); - if (modelMap != null) { - modelId = modelMap.get(KMKey_LexicalModelID); - if (modelId == null) { - modelId = ""; - } + // Both take a context parameter... but don't actually need or use it! + Keyboard kbd = KMManager.getCurrentKeyboardInfo(null); + kbdCount = KMManager.getKeyboardsList(null).size(); + if (kbd != null) { + kbdId = kbd.getKeyboardID(); + lngCode = kbd.getLanguageCode(); + Map modelMap = KMManager.getAssociatedLexicalModel(kbd.getLanguageID()); + if (modelMap != null) { + modelId = modelMap.get(KMKey_LexicalModelID); + if (modelId == null) { + modelId = ""; } } } } catch (Exception ignored) { } Sentry.setExtra(KEYBOARD_TAG, kbdId); + Sentry.setExtra(KEYBOARD_COUNT_TAG, "" + kbdCount); Sentry.setExtra(LANGCODE_TAG, lngCode); Sentry.setExtra(MODEL_TAG, modelId); } From 261283d3565678a94ea0f2dde5d2ec6c8e8ba24b Mon Sep 17 00:00:00 2001 From: Joshua Horton Date: Tue, 20 May 2025 13:30:38 +0700 Subject: [PATCH 3/3] change(android): address PR review concerns --- .../src/main/java/com/keyman/engine/util/KMLog.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/android/KMEA/app/src/main/java/com/keyman/engine/util/KMLog.java b/android/KMEA/app/src/main/java/com/keyman/engine/util/KMLog.java index 9c8f4a8f57..1ce55bf0ce 100644 --- a/android/KMEA/app/src/main/java/com/keyman/engine/util/KMLog.java +++ b/android/KMEA/app/src/main/java/com/keyman/engine/util/KMLog.java @@ -29,6 +29,10 @@ public final class KMLog { private static final String MODEL_TAG = "modelId"; private static final String LANGCODE_TAG = "languageCode"; + // Some of the methods used to generate debug logging information can, themselves, + // trigger errors that can also trigger the same logging. We must not get + // caught in an infinite loop / stack-overflow; this field helps us avoid states + // that would otherwise cause error-looping, etc. private static boolean isLogging = false; private static void tagDebugInfo() { @@ -52,7 +56,9 @@ public final class KMLog { } } } - } catch (Exception ignored) { + } catch (Exception ex) { + String msg = ex.getMessage() == null ? "" : ex.getMessage(); + Sentry.setExtra("debugLoggingError", msg); } Sentry.setExtra(KEYBOARD_TAG, kbdId); Sentry.setExtra(KEYBOARD_COUNT_TAG, "" + kbdCount); @@ -212,6 +218,8 @@ public final class KMLog { } // Report the original exception LogException(tag, msg, e); + // And remove the exception-specific tagged data, lest it also be + // tracked on subsequent errors not associated with the current call. Sentry.removeExtra(objName); } isLogging = false;