From c3e2295e4195320dfee5b6339a120660c001906a Mon Sep 17 00:00:00 2001 From: Ross Date: Mon, 23 May 2022 21:17:27 +1000 Subject: [PATCH 1/4] fix(windows): Add sentry logging in DoInstall This commit adds some sentry logging to the TTIPMaintenance.DoInstall to assist in tracking bug with installing a TIP for new language associated with a Keyboard. --- ...an.Configuration.System.TIPMaintenance.pas | 88 ++++++++++++++++-- windows/src/desktop/kmshell/kmshell.res | Bin 7036 -> 7036 bytes 2 files changed, 82 insertions(+), 6 deletions(-) diff --git a/windows/src/desktop/kmshell/install/Keyman.Configuration.System.TIPMaintenance.pas b/windows/src/desktop/kmshell/install/Keyman.Configuration.System.TIPMaintenance.pas index 2df2ba8a0d..6a33698040 100644 --- a/windows/src/desktop/kmshell/install/Keyman.Configuration.System.TIPMaintenance.pas +++ b/windows/src/desktop/kmshell/install/Keyman.Configuration.System.TIPMaintenance.pas @@ -3,8 +3,9 @@ unit Keyman.Configuration.System.TIPMaintenance; interface uses + Sentry.Client, System.Classes, - + utiltsf, KeymanAPI_TLB; type @@ -62,7 +63,43 @@ uses utilexecute, utilsystem; -{ TTIPMaintenance } + { For temporary debugging couldn't include utilkeyman.pas so extracted needed function } + +function GetShortKeyboardName(const FileName: string): string; +begin + if (LowerCase(ExtractFileExt(FileName)) = '.kmx') or + (LowerCase(ExtractFileExt(FileName)) = '.kxx') or + (LowerCase(ExtractFileExt(FileName)) = '.kmp') + then Result := ChangeFileExt(ExtractFileName(FileName), '') + else Result := FileName; +end; + +// https://stackoverflow.com/questions/9004870/enumerate-registry-subkeys-in-delphi +procedure GetSubKeyList(RootKey: HKEY; const Key: string; var KeyList: string); +var + Registry: TRegistry; + SubKeyNames: TStringList; + Name: string; +begin + Registry := TRegistry.Create(KEY_READ); + KeyList := ''; + Try + Registry.RootKey := RootKey; + Registry.OpenKeyReadOnly(Key); + SubKeyNames := TStringList.Create; + Try + Registry.GetKeyNames(SubKeyNames); + for Name in SubKeyNames do + KeyList := KeyList + ' ' + Name; + Finally + SubKeyNames.Free; + End; + Finally + Registry.Free; + End; +end; + + { TTIPMaintenance } class function TTIPMaintenance.InstallTipsForPackages(Packages: TStrings): Boolean; var @@ -179,8 +216,8 @@ var RegistrationRequired: WordBool; TemporaryKeyboardID: WideString; LangID: Integer; - childExitCode: Cardinal; - CanonicalTag, Command: string; + childExitCode, elevatedExitCode: Cardinal; + CanonicalTag, Command, RootPath, KeyList: string; begin Result := False; KL.MethodEnter(nil, 'TTIPMaintenance.DoInstall', [KeyboardID, BCP47Tag]); @@ -191,6 +228,8 @@ begin begin // The keyboard was not found KL.Log('lang not found: BCP47Tag = %s, CanonicalTag = %s', [BCP47Tag, CanonicalTag]); + // Additonal Sentry logging added to track TIP crash git hub issue #6619 + TKeymanSentryClient.Client.MessageEvent(Sentry.Client.SENTRY_LEVEL_INFO,Format('lang not found: BCP47Tag = %s, CanonicalTag = %s', [BCP47Tag, CanonicalTag]),TRUE); Exit(False); end; @@ -208,6 +247,10 @@ begin begin KL.Log('Failed to find installation langid'); // We were not able to find a TIP, perhaps all transient TIPs have been used + + // Additonal Sentry logging added to track TIP crash git hub issue #6619 + TKeymanSentryClient.Client.MessageEvent(Sentry.Client.SENTRY_LEVEL_INFO, Format('TIPM.FindInstallationLangID failed kbID[%s] BCP47Tag[%s] CanonicalTag[%s] lang.BCP47Tag[%s]. Result LangID[%d] TempKeyboardID[%s] RegistrationRequired[%d]', + [KeyboardID, BCP47Tag, CanonicalTag, lang.BCP47Code , LangId, TemporaryKeyboardID, RegistrationRequired] ), TRUE); Exit(False); end; @@ -218,8 +261,25 @@ begin Command := '-register-tip '+IntToHex(LangID,4)+' "'+KeyboardID+'" "'+lang.BCP47Code+'" '+GetUserDefaultLangParameterString; KL.Log('Calling elevated kmshell %s', [Command]); // This calls back into TTIPMaintenance.RegisterTip - if WaitForElevatedConfiguration(0, Command) <> 0 then - Exit(False); + elevatedExitCode := WaitForElevatedConfiguration(0, Command); + if elevatedExitCode <> 0 then + // Additonal Sentry logging added to track TIP crash git hub issue #6619 + TKeymanSentryClient.Client.MessageEvent(Sentry.Client.SENTRY_LEVEL_INFO, + Format('TIPM.ElevatedRegistration child process failed CommadLine[%s] kbID[%s] BCP47Tag[%s] CanonicalTag[%s] TempKeyboardID[%s] RegistrationRequired[%d] Exit Code[%d]', + [Command, KeyboardID, BCP47Tag, CanonicalTag, TemporaryKeyboardID, RegistrationRequired, elevatedExitCode] ), TRUE); + // Log registry subkeys in the language profile. + if IsTransientLanguageID(LangID) then + begin + RootPath := SRegKey_InstalledKeyboards_LM+'\'+GetShortKeyboardName(KeyboardID) + SRegSubKey_TransientLanguageProfiles; + end + else + begin + RootPath := SRegKey_InstalledKeyboards_LM+'\'+GetShortKeyboardName(KeyboardID) + SRegSubKey_LanguageProfiles; + end; + GetSubKeyList(HKEY_LOCAL_MACHINE,RootPath,KeyList); + TKeymanSentryClient.Client.MessageEvent(Sentry.Client.SENTRY_LEVEL_INFO, + Format('TIPM.ElevatedRegistration child process failed LP Registry Keys[%s]', [KeyList] ), TRUE); + Exit(False); // sentry log end; Command := '-install-tip '+IntToHex(LangID,4)+' "'+KeyboardID+'" "'+lang.BCP47Code+'" "'+TemporaryKeyboardID+'"'; @@ -229,6 +289,22 @@ begin (childExitCode <> 0) then begin kmcom.Refresh; + // Additional Sentry logging added to track TIP crash git hub issue #6619 + TKeymanSentryClient.Client.MessageEvent(Sentry.Client.SENTRY_LEVEL_INFO, + Format('TIPM.InstallTip child process failed CommadLine[%s] kbID[%s] BCP47Tag[%s] CanonicalTag[%s] RegistrationRequired[%d] Exit Code[%d]', + [Command, KeyboardID, BCP47Tag, CanonicalTag, RegistrationRequired, childExitCode] ), TRUE); + // Log the Registry subkeys in the language profile. + if IsTransientLanguageID(LangID) then + begin + RootPath := SRegKey_InstalledKeyboards_LM+'\'+GetShortKeyboardName(KeyboardID) + SRegSubKey_TransientLanguageProfiles; + end + else + begin + RootPath := SRegKey_InstalledKeyboards_LM+'\'+GetShortKeyboardName(KeyboardID) + SRegSubKey_LanguageProfiles; + end; + GetSubKeyList(HKEY_LOCAL_MACHINE,RootPath,KeyList); + TKeymanSentryClient.Client.MessageEvent(Sentry.Client.SENTRY_LEVEL_INFO, + Format('TIPM.InstallTip child process failed LP Registry Keys[%s]', [KeyList] ), TRUE); Exit(False); end; diff --git a/windows/src/desktop/kmshell/kmshell.res b/windows/src/desktop/kmshell/kmshell.res index e0b6daf82305264ae376ead4b59cc19c9801d17c..e5a5224915cf8677ef1df7e9ca5df277ce21c8ab 100644 GIT binary patch delta 15 Wcmexk_Qz~O35((5Px2cpSfl|yN(R~h delta 15 Wcmexk_Qz~O3Co|E{Bj#BSfl|xMFw{O From 79fe04a7236d91f7c6fb967b2305e7a03ce9bc1e Mon Sep 17 00:00:00 2001 From: rc-swag <58423624+rc-swag@users.noreply.github.com> Date: Tue, 24 May 2022 08:10:17 +1000 Subject: [PATCH 2/4] fix(windows): address review comments Co-authored-by: Marc Durdin --- .../Keyman.Configuration.System.TIPMaintenance.pas | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/windows/src/desktop/kmshell/install/Keyman.Configuration.System.TIPMaintenance.pas b/windows/src/desktop/kmshell/install/Keyman.Configuration.System.TIPMaintenance.pas index 6a33698040..441f3c2c9b 100644 --- a/windows/src/desktop/kmshell/install/Keyman.Configuration.System.TIPMaintenance.pas +++ b/windows/src/desktop/kmshell/install/Keyman.Configuration.System.TIPMaintenance.pas @@ -229,7 +229,10 @@ begin // The keyboard was not found KL.Log('lang not found: BCP47Tag = %s, CanonicalTag = %s', [BCP47Tag, CanonicalTag]); // Additonal Sentry logging added to track TIP crash git hub issue #6619 - TKeymanSentryClient.Client.MessageEvent(Sentry.Client.SENTRY_LEVEL_INFO,Format('lang not found: BCP47Tag = %s, CanonicalTag = %s', [BCP47Tag, CanonicalTag]),TRUE); + if TKeymanSentryClient.Client <> nil then + TKeymanSentryClient.Client.MessageEvent(Sentry.Client.SENTRY_LEVEL_INFO, + Format('TTIPMaintenance.DoInstall: lang not found: BCP47Tag = %s, CanonicalTag = %s', [BCP47Tag, CanonicalTag]), + TRUE); Exit(False); end; @@ -249,8 +252,11 @@ begin // We were not able to find a TIP, perhaps all transient TIPs have been used // Additonal Sentry logging added to track TIP crash git hub issue #6619 - TKeymanSentryClient.Client.MessageEvent(Sentry.Client.SENTRY_LEVEL_INFO, Format('TIPM.FindInstallationLangID failed kbID[%s] BCP47Tag[%s] CanonicalTag[%s] lang.BCP47Tag[%s]. Result LangID[%d] TempKeyboardID[%s] RegistrationRequired[%d]', - [KeyboardID, BCP47Tag, CanonicalTag, lang.BCP47Code , LangId, TemporaryKeyboardID, RegistrationRequired] ), TRUE); + if TKeymanSentryClient.Client <> nil then + TKeymanSentryClient.Client.MessageEvent(Sentry.Client.SENTRY_LEVEL_INFO, + Format('TTIPMaintenance.DoInstall: FindInstallationLangID failed kbID[%s] BCP47Tag[%s] CanonicalTag[%s] lang.BCP47Tag[%s]. Result LangID[%d] TempKeyboardID[%s] RegistrationRequired[%d]', + [KeyboardID, BCP47Tag, CanonicalTag, lang.BCP47Code, LangId, TemporaryKeyboardID, RegistrationRequired]), + TRUE); Exit(False); end; From e9a7b8a2d1a57f72b183dcb9ec3336608e19a6d5 Mon Sep 17 00:00:00 2001 From: Ross Date: Tue, 24 May 2022 13:38:56 +1000 Subject: [PATCH 3/4] fix(windows): Address Review comments --- ...an.Configuration.System.TIPMaintenance.pas | 68 +++++++++--------- windows/src/desktop/kmshell/kmshell.res | Bin 7036 -> 7036 bytes 2 files changed, 36 insertions(+), 32 deletions(-) diff --git a/windows/src/desktop/kmshell/install/Keyman.Configuration.System.TIPMaintenance.pas b/windows/src/desktop/kmshell/install/Keyman.Configuration.System.TIPMaintenance.pas index 441f3c2c9b..f68805bef0 100644 --- a/windows/src/desktop/kmshell/install/Keyman.Configuration.System.TIPMaintenance.pas +++ b/windows/src/desktop/kmshell/install/Keyman.Configuration.System.TIPMaintenance.pas @@ -85,7 +85,11 @@ begin KeyList := ''; Try Registry.RootKey := RootKey; - Registry.OpenKeyReadOnly(Key); + if not Registry.OpenKeyReadOnly(Key) then + begin + KeyList := ''; + Exit; + end; SubKeyNames := TStringList.Create; Try Registry.GetKeyNames(SubKeyNames); @@ -254,8 +258,8 @@ begin // Additonal Sentry logging added to track TIP crash git hub issue #6619 if TKeymanSentryClient.Client <> nil then TKeymanSentryClient.Client.MessageEvent(Sentry.Client.SENTRY_LEVEL_INFO, - Format('TTIPMaintenance.DoInstall: FindInstallationLangID failed kbID[%s] BCP47Tag[%s] CanonicalTag[%s] lang.BCP47Tag[%s]. Result LangID[%d] TempKeyboardID[%s] RegistrationRequired[%d]', - [KeyboardID, BCP47Tag, CanonicalTag, lang.BCP47Code, LangId, TemporaryKeyboardID, RegistrationRequired]), + Format('TTIPMaintenance.DoInstall: FindInstallationLangID failed kbID[%s] BCP47Tag[%s] CanonicalTag[%s] lang.BCP47Tag[%s] Result LangID[%x] TempKeyboardID[%s] RegistrationRequired[%s]', + [KeyboardID, BCP47Tag, CanonicalTag, lang.BCP47Code, LangId, TemporaryKeyboardID, BoolToStr(RegistrationRequired, True)]), TRUE); Exit(False); end; @@ -270,21 +274,21 @@ begin elevatedExitCode := WaitForElevatedConfiguration(0, Command); if elevatedExitCode <> 0 then // Additonal Sentry logging added to track TIP crash git hub issue #6619 - TKeymanSentryClient.Client.MessageEvent(Sentry.Client.SENTRY_LEVEL_INFO, - Format('TIPM.ElevatedRegistration child process failed CommadLine[%s] kbID[%s] BCP47Tag[%s] CanonicalTag[%s] TempKeyboardID[%s] RegistrationRequired[%d] Exit Code[%d]', - [Command, KeyboardID, BCP47Tag, CanonicalTag, TemporaryKeyboardID, RegistrationRequired, elevatedExitCode] ), TRUE); - // Log registry subkeys in the language profile. - if IsTransientLanguageID(LangID) then - begin - RootPath := SRegKey_InstalledKeyboards_LM+'\'+GetShortKeyboardName(KeyboardID) + SRegSubKey_TransientLanguageProfiles; - end - else - begin - RootPath := SRegKey_InstalledKeyboards_LM+'\'+GetShortKeyboardName(KeyboardID) + SRegSubKey_LanguageProfiles; - end; - GetSubKeyList(HKEY_LOCAL_MACHINE,RootPath,KeyList); - TKeymanSentryClient.Client.MessageEvent(Sentry.Client.SENTRY_LEVEL_INFO, - Format('TIPM.ElevatedRegistration child process failed LP Registry Keys[%s]', [KeyList] ), TRUE); + if TKeymanSentryClient.Client <> nil then + // Log command line, state of variables and registry subkeys in the language profile + if IsTransientLanguageID(LangID) then + begin + RootPath := SRegKey_InstalledKeyboards_LM+'\'+GetShortKeyboardName(KeyboardID) + SRegSubKey_TransientLanguageProfiles; + end + else + begin + RootPath := SRegKey_InstalledKeyboards_LM+'\'+GetShortKeyboardName(KeyboardID) + SRegSubKey_LanguageProfiles; + end; + GetSubKeyList(HKEY_LOCAL_MACHINE,RootPath,KeyList); + TKeymanSentryClient.Client.MessageEvent(Sentry.Client.SENTRY_LEVEL_INFO, + Format('TTIPMaintenance.DoInstall: ElevatedRegistration child process failed CommadLine[%s] kbID[%s] BCP47Tag[%s] CanonicalTag[%s] TempKeyboardID[%s] ' + + 'RegistrationRequired[%s] Exit Code[%d] '#13#10' Registry Values for Languages[%s]', + [Command, KeyboardID, BCP47Tag, CanonicalTag, TemporaryKeyboardID, BoolToStr(RegistrationRequired, True), elevatedExitCode, KeyList]), TRUE); Exit(False); // sentry log end; @@ -296,21 +300,21 @@ begin begin kmcom.Refresh; // Additional Sentry logging added to track TIP crash git hub issue #6619 - TKeymanSentryClient.Client.MessageEvent(Sentry.Client.SENTRY_LEVEL_INFO, - Format('TIPM.InstallTip child process failed CommadLine[%s] kbID[%s] BCP47Tag[%s] CanonicalTag[%s] RegistrationRequired[%d] Exit Code[%d]', - [Command, KeyboardID, BCP47Tag, CanonicalTag, RegistrationRequired, childExitCode] ), TRUE); - // Log the Registry subkeys in the language profile. - if IsTransientLanguageID(LangID) then - begin - RootPath := SRegKey_InstalledKeyboards_LM+'\'+GetShortKeyboardName(KeyboardID) + SRegSubKey_TransientLanguageProfiles; - end - else - begin - RootPath := SRegKey_InstalledKeyboards_LM+'\'+GetShortKeyboardName(KeyboardID) + SRegSubKey_LanguageProfiles; - end; - GetSubKeyList(HKEY_LOCAL_MACHINE,RootPath,KeyList); + if TKeymanSentryClient.Client <> nil then + // Log command line, state of variables and registry subkeys in the language profile. + if IsTransientLanguageID(LangID) then + begin + RootPath := SRegKey_InstalledKeyboards_LM+'\'+GetShortKeyboardName(KeyboardID) + SRegSubKey_TransientLanguageProfiles; + end + else + begin + RootPath := SRegKey_InstalledKeyboards_LM+'\'+GetShortKeyboardName(KeyboardID) + SRegSubKey_LanguageProfiles; + end; + GetSubKeyList(HKEY_LOCAL_MACHINE,RootPath,KeyList); TKeymanSentryClient.Client.MessageEvent(Sentry.Client.SENTRY_LEVEL_INFO, - Format('TIPM.InstallTip child process failed LP Registry Keys[%s]', [KeyList] ), TRUE); + Format('TTIPMaintenance.DoInstall: InstallTip child process failed CommadLine[%s] kbID[%s] BCP47Tag[%s] CanonicalTag[%s] ' + + 'RegistrationRequired[%s] Exit Code[%d] '#13#10' Registry Values for Languages[%s]', + [Command, KeyboardID, BCP47Tag, CanonicalTag, BoolToStr(RegistrationRequired, True), childExitCode, KeyList] ), TRUE); Exit(False); end; diff --git a/windows/src/desktop/kmshell/kmshell.res b/windows/src/desktop/kmshell/kmshell.res index e5a5224915cf8677ef1df7e9ca5df277ce21c8ab..e0b6daf82305264ae376ead4b59cc19c9801d17c 100644 GIT binary patch delta 15 Wcmexk_Qz~O3Co|E{Bj#BSfl|xMFw{O delta 15 Wcmexk_Qz~O35((5Px2cpSfl|yN(R~h From 500e7837e9cadd123b9f8f58d39d1e74a44bbfdc Mon Sep 17 00:00:00 2001 From: Ross Date: Tue, 24 May 2022 13:44:41 +1000 Subject: [PATCH 4/4] fix(windows): Fix indentation --- .../Keyman.Configuration.System.TIPMaintenance.pas | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/windows/src/desktop/kmshell/install/Keyman.Configuration.System.TIPMaintenance.pas b/windows/src/desktop/kmshell/install/Keyman.Configuration.System.TIPMaintenance.pas index f68805bef0..18f64409e2 100644 --- a/windows/src/desktop/kmshell/install/Keyman.Configuration.System.TIPMaintenance.pas +++ b/windows/src/desktop/kmshell/install/Keyman.Configuration.System.TIPMaintenance.pas @@ -259,7 +259,7 @@ begin if TKeymanSentryClient.Client <> nil then TKeymanSentryClient.Client.MessageEvent(Sentry.Client.SENTRY_LEVEL_INFO, Format('TTIPMaintenance.DoInstall: FindInstallationLangID failed kbID[%s] BCP47Tag[%s] CanonicalTag[%s] lang.BCP47Tag[%s] Result LangID[%x] TempKeyboardID[%s] RegistrationRequired[%s]', - [KeyboardID, BCP47Tag, CanonicalTag, lang.BCP47Code, LangId, TemporaryKeyboardID, BoolToStr(RegistrationRequired, True)]), + [KeyboardID, BCP47Tag, CanonicalTag, lang.BCP47Code, LangId, TemporaryKeyboardID, BoolToStr(RegistrationRequired, True)]), TRUE); Exit(False); end; @@ -287,8 +287,9 @@ begin GetSubKeyList(HKEY_LOCAL_MACHINE,RootPath,KeyList); TKeymanSentryClient.Client.MessageEvent(Sentry.Client.SENTRY_LEVEL_INFO, Format('TTIPMaintenance.DoInstall: ElevatedRegistration child process failed CommadLine[%s] kbID[%s] BCP47Tag[%s] CanonicalTag[%s] TempKeyboardID[%s] ' + - 'RegistrationRequired[%s] Exit Code[%d] '#13#10' Registry Values for Languages[%s]', - [Command, KeyboardID, BCP47Tag, CanonicalTag, TemporaryKeyboardID, BoolToStr(RegistrationRequired, True), elevatedExitCode, KeyList]), TRUE); + 'RegistrationRequired[%s] Exit Code[%d] '#13#10' Registry Values for Languages[%s]', + [Command, KeyboardID, BCP47Tag, CanonicalTag, TemporaryKeyboardID, BoolToStr(RegistrationRequired, True), elevatedExitCode, KeyList]), + TRUE); Exit(False); // sentry log end; @@ -313,8 +314,9 @@ begin GetSubKeyList(HKEY_LOCAL_MACHINE,RootPath,KeyList); TKeymanSentryClient.Client.MessageEvent(Sentry.Client.SENTRY_LEVEL_INFO, Format('TTIPMaintenance.DoInstall: InstallTip child process failed CommadLine[%s] kbID[%s] BCP47Tag[%s] CanonicalTag[%s] ' + - 'RegistrationRequired[%s] Exit Code[%d] '#13#10' Registry Values for Languages[%s]', - [Command, KeyboardID, BCP47Tag, CanonicalTag, BoolToStr(RegistrationRequired, True), childExitCode, KeyList] ), TRUE); + 'RegistrationRequired[%s] Exit Code[%d] '#13#10' Registry Values for Languages[%s]', + [Command, KeyboardID, BCP47Tag, CanonicalTag, BoolToStr(RegistrationRequired, True), childExitCode, KeyList] ), + TRUE); Exit(False); end;