chore(mac): add breadcrumbs for Sentry #12939

sgschantz · 2025-01-17T05:05:43Z

Add additional context (breadcrumbs and new custom tags) to help understand the cause of Sentry issues.

Also change the mechanism to force a crash being reported to Sentry and adds a command to cause a captureMessage call to Sentry.

To enable the Sentry tests, set the flag in UserDefaults by exectuting the following command in terminal:
defaults write keyman.inputmethod.Keyman KMForceSentryError 1

Then with the SIL Euro Latin keyboard type one of the following two characters in the Stickies app:
{ - to generate a crash event
[ - to generate a message capture event

Fixes #11623

@keymanapp-test-bot skip

can now force crashes or send messages by typing a single character when sentry testing is enabled in the UserDefaults

keymanapp-test-bot · 2025-01-17T05:05:46Z

User Test Results

Test specification and instructions

User tests are not required

Test Artifacts

macOS
- Keyman for macOS

resolves link error when running unit tests

Fixes: #11623

…/github.com/keymanapp/keyman into chore/mac/11623-add-breadcrumbs

darcywong00 · 2025-01-24T03:06:30Z

mac/Keyman4MacIM/Keyman4MacIM/KMConfiguration/KMConfigurationWindowController.m

    [self.activeKeyboards addObject:partialPath];
    [self saveActiveKeyboards];
  }
  else if (checkBox.state == NSOffState) {
-    os_log_debug([KMLogs uiLog], "Disabling active keyboard: %{public}@", kmxFilePath);
+    os_log_debug([KMLogs uiLog], "disabling active keyboard: %{public}@", kmxFileName);


Would it be easier if addUserBreadCrumb happened inside os_log_debug()?

os_log_debug is a macOS API, so I can't combine there. I'd like to combine Sentry and macOS logging in a single call, but I am also instructed not to wrap the macOS API call because it tracks the location. So if I leave it in place, I would only be wrapping Sentry (which I am essentially doing to simplify it) and that still leaves two function calls from the location I want to log.

darcywong00 · 2025-01-24T03:14:49Z

mac/Keyman4MacIM/Keyman4MacIM/KMInputMethodAppDelegate.m

+/**
+ * Determine whether to force sentry events.
+ * When true, then typing certain characters will cause Sentry events or crashes to occur.
+ *
+ * To enable, set the flag in UserDefaults
+ * `defaults write keyman.inputmethod.Keyman KMForceSentryError 1`
+ * Also, use the SIL Euro Latin keyboard and type in the Stickies app
+ */
+- (BOOL)canForceSentryEvents {
+  NSString* const testKeyboardName = @"sil_euro_latin.kmx";
+  NSString* const testClientApplicationId = @"com.apple.Stickies";
+  BOOL canForce = NO;


Can you include this change in the PR description? (Makes the developer testing more visible).

Ideally, this could have been separated from the Sentry breadcrumbs PR so it shows in HISTORY.md.

darcywong00 · 2025-01-24T03:16:14Z

mac/Keyman4MacIM/Keyman4MacIM/Logging/KMSentryHelper.h

@@ -0,0 +1,29 @@
+/*
+ * Keyman is copyright (C) SIL International. MIT License.


Nit on new source files to use "SIL Global" instead of "SIL International"

darcywong00 · 2025-01-24T03:19:12Z

mac/Keyman4MacIM/Keyman4MacIM/Logging/KMSentryHelper.m

+ (void)addBreadCrumb:(NSString *)category message:(NSString *)messageText {
+  SentryBreadcrumb *crumb = [[SentryBreadcrumb alloc] init];
+  crumb.level = kSentryLevelInfo;
+  crumb.category = category;
+  crumb.message = messageText;
+  [SentrySDK addBreadcrumb:crumb];
+}
+
+ (void)addDebugBreadCrumb:(NSString *)category message:(NSString *)messageText {
+  SentryBreadcrumb *crumb = [[SentryBreadcrumb alloc] init];
+  crumb.level = kSentryLevelDebug;
+  crumb.category = category;
+  crumb.message = messageText;
+  [SentrySDK addBreadcrumb:crumb];
+}
+
+ (void)addUserBreadCrumb:(NSString *)category message:(NSString *)messageText {
+  SentryBreadcrumb *crumb = [[SentryBreadcrumb alloc] init];
+  crumb.type = @"user";
+  crumb.level = kSentryLevelInfo;
+  crumb.category = category;
+  crumb.message = messageText;
+  [SentrySDK addBreadcrumb:crumb];
+}


Would it be cleaner to refactor these into a single function (with a level and an optional type parameters)?

I'll take a look at simplifying this. I'm applying an interesting cross section of arguments, and I'd rather have multiple different functions with fewer parameters because that would easier to read in the client code. I should still be able to eliminate duplication in the wrapper at least.

I added comments and removed one method not being used, but I think the three functions available are sufficient and would be cleaner from the caller's perspective than creating a variadic function. Also, the duplication is pretty minimal and probably not worth complicating the code to reduce it.

darcywong00 · 2025-01-27T02:56:42Z

mac/Keyman4MacIM/Keyman4MacIM/KMConfiguration/KMConfigurationWindowController.m

+    [self.deleteAlertView setMessageText:[NSString localizedStringWithFormat:deleteKeyboardMessage, keyboardName]];
+  } else {
+    keyboardName = [info objectForKey:kKMKeyboardNameKey];
+    [self.deleteAlertView setMessageText:[NSString localizedStringWithFormat:deleteKeyboardMessage, keyboardName]];


If this is identical to l.421, can they be pulled out of the if/else?

yes, thanks for spotting that!

darcywong00 · 2025-01-27T02:58:56Z

mac/Keyman4MacIM/Keyman4MacIM/KMDownloadKeyboard/KMDownloadKBWindowController.m

@@ -9,6 +9,7 @@
 #import "KMDownloadKBWindowController.h"
 #import "KMInputMethodAppDelegate.h"
 #import "KMLogs.h"
+#import "KMSentryHelper.h"


Is this used?

nope, I'll remove

darcywong00 · 2025-01-27T03:10:07Z

mac/Keyman4MacIM/Keyman4MacIM/Logging/KMSentryHelper.m

+NSString * const kApiComplianceTagName = @"apiCompliance";
+NSString * const kArchitectureTagName = @"architecture";
+NSString * const kOskVisibleTagName = @"oskVisible";
+NSString * const kClientAppIdTagName = @"clientAppId";
+NSString * const kKeyboardTagName = @"keyboard";
+NSString * const kHasAccessibilityTagName = @"accessibilityEnabled";
+NSString * const kActiveKeyboardCountTagName = @"activeKeyboardCount";
+NSString * const kLifecycleStateTagName = @"lifecycleState";


Do these get sent to Sentry?

yes, all are sent to Sentry with the setTagValue API

darcywong00

lgtm

keyman-server · 2025-01-28T18:02:52Z

Changes in this pull request will be available for download in Keyman version 18.0.179-alpha

sgschantz added 2 commits January 13, 2025 14:08

chore(mac): reworked sentry testing

8e833c8

can now force crashes or send messages by typing a single character when sentry testing is enabled in the UserDefaults

chore(mac): add KMSentryHelper class, and new tags and breadcrumbs

76d0d69

sgschantz added the mac/ label Jan 17, 2025

sgschantz added this to the A18S20 milestone Jan 17, 2025

sgschantz self-assigned this Jan 17, 2025

github-actions bot added the chore label Jan 17, 2025

sgschantz added 2 commits January 20, 2025 14:17

chore(mac): added architecture tag, more breadcrumbs

162dc40

chore(mac): add KMSentryHelper to test project

2b39027

resolves link error when running unit tests

sgschantz mentioned this pull request Jan 21, 2025

bug(mac): crash during compliance check #11674

Open

sgschantz and others added 4 commits January 22, 2025 11:28

chore(mac): added tag for lifecycleState

d6ce0af

Fixes: #11623

Merge branch 'master' into chore/mac/11623-add-breadcrumbs

4a04937

chore(mac): Merge branch 'chore/mac/11623-add-breadcrumbs' of https:/…

b0776a4

…/github.com/keymanapp/keyman into chore/mac/11623-add-breadcrumbs

chore(mac): clean up comments

0ec2664

sgschantz marked this pull request as ready for review January 22, 2025 05:53

sgschantz requested a review from SabineSIL as a code owner January 22, 2025 05:53

chore(mac): fixed else statement for architecture tag

c9c47e8

darcywong00 reviewed Jan 24, 2025

View reviewed changes

chore(mac): added comments and renamed function

077229e

darcywong00 reviewed Jan 27, 2025

View reviewed changes

chore(mac): remove extraneous code

e5820f5

darcywong00 approved these changes Jan 28, 2025

View reviewed changes

sgschantz merged commit 8972040 into master Jan 28, 2025
5 checks passed

sgschantz deleted the chore/mac/11623-add-breadcrumbs branch January 28, 2025 05:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(mac): add breadcrumbs for Sentry #12939

chore(mac): add breadcrumbs for Sentry #12939

sgschantz commented Jan 17, 2025 •

edited

Loading

keymanapp-test-bot bot commented Jan 17, 2025 •

edited

Loading

darcywong00 Jan 24, 2025

sgschantz Jan 24, 2025

darcywong00 Jan 24, 2025

sgschantz Jan 24, 2025

darcywong00 Jan 24, 2025

darcywong00 Jan 24, 2025

sgschantz Jan 24, 2025

sgschantz Jan 27, 2025

darcywong00 Jan 27, 2025

sgschantz Jan 27, 2025

darcywong00 Jan 27, 2025

sgschantz Jan 27, 2025

darcywong00 Jan 27, 2025

sgschantz Jan 27, 2025

darcywong00 left a comment

keyman-server commented Jan 28, 2025

		@@ -0,0 +1,29 @@
		/*
		* Keyman is copyright (C) SIL International. MIT License.

chore(mac): add breadcrumbs for Sentry #12939

chore(mac): add breadcrumbs for Sentry #12939

Conversation

sgschantz commented Jan 17, 2025 • edited Loading

keymanapp-test-bot bot commented Jan 17, 2025 • edited Loading

User Test Results

Test Artifacts

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

darcywong00 left a comment

Choose a reason for hiding this comment

keyman-server commented Jan 28, 2025

sgschantz commented Jan 17, 2025 •

edited

Loading

keymanapp-test-bot bot commented Jan 17, 2025 •

edited

Loading