Fix exception logging (#379)

microsoft · Apr 11, 2024 · b7a4368 · b7a4368
1 parent 8871c3e
commit b7a4368
Show file tree

Hide file tree

Showing 21 changed files with 67 additions and 67 deletions.
diff --git a/src/GitHubExtension/Client/GithubClientProvider.cs b/src/GitHubExtension/Client/GithubClientProvider.cs
@@ -94,7 +94,7 @@ public async Task<GitHubClient> GetClientForLoggedInDeveloper(bool logRateLimit
             }
             catch (Exception ex)
             {
-                Log.Error($"Rate limiting not enabled for server.", ex);
+                Log.Error(ex, $"Rate limiting not enabled for server.");
             }
         }
 

diff --git a/src/GitHubExtension/Client/Validation.cs b/src/GitHubExtension/Client/Validation.cs
@@ -238,7 +238,7 @@ public static async Task<bool> IsReachableGitHubEnterpriseServerURL(Uri server)
         }
         catch (Exception ex)
         {
-            Log.Error($"EnterpriseServer {server.AbsoluteUri} could not be probed.", ex);
+            Log.Error(ex, $"EnterpriseServer {server.AbsoluteUri} could not be probed.");
             return false;
         }
 

diff --git a/src/GitHubExtension/DataManager/GitHubDataManager.cs b/src/GitHubExtension/DataManager/GitHubDataManager.cs
@@ -47,7 +47,7 @@ public partial class GitHubDataManager : IGitHubDataManager, IDisposable
         }
         catch (Exception e)
         {
-            Log.Error("Failed creating GitHubDataManager", e);
+            Log.Error(e, "Failed creating GitHubDataManager");
             Environment.FailFast(e.Message, e);
             return null;
         }
@@ -253,7 +253,7 @@ private async Task UpdateDataStoreAsync(DataStoreOperationParameters parameters,
         }
         catch (Exception ex)
         {
-            Log.Error($"Failed Updating DataStore for: {parameters}", ex);
+            Log.Error(ex, $"Failed Updating DataStore for: {parameters}");
             tx.Rollback();
 
             // Rethrow so clients can catch/display an error UX.
@@ -334,7 +334,7 @@ private async Task UpdateDataForRepositoryAsync(DataStoreOperationParameters par
         catch (Exception ex)
         {
             // This is for catching any other unexpected error as well as any we throw.
-            Log.Error($"Failed trying update data for repository: {parameters.Owner}/{parameters.RepositoryName}", ex);
+            Log.Error(ex, $"Failed trying update data for repository: {parameters.Owner}/{parameters.RepositoryName}");
             tx.Rollback();
             throw;
         }
@@ -460,7 +460,7 @@ private async Task UpdatePullRequestsForLoggedInDeveloperIdsAsync(DataStoreOpera
                         Log.Error($"Error updating Check Status for Pull Request #{octoPull.Number}: {e.Message}");
 
                         // Put the full stack trace in debug if this occurs to reduce log spam.
-                        Log.Debug($"Error updating Check Status for Pull Request #{octoPull.Number}.", e);
+                        Log.Debug(e, $"Error updating Check Status for Pull Request #{octoPull.Number}.");
                     }
 
                     var commitCombinedStatus = await devId.GitHubClient.Repository.Status.GetCombined(dsRepository.InternalId, dsPullRequest.HeadSha);
@@ -486,7 +486,7 @@ private async Task UpdatePullRequestsForLoggedInDeveloperIdsAsync(DataStoreOpera
                         Log.Error($"Error updating Reviews for Pull Request #{octoPull.Number}: {e.Message}");
 
                         // Put the full stack trace in debug if this occurs to reduce log spam.
-                        Log.Debug($"Error updating Reviews for Pull Request #{octoPull.Number}.", e);
+                        Log.Debug(e, $"Error updating Reviews for Pull Request #{octoPull.Number}.");
                     }
                 }
 
@@ -551,7 +551,7 @@ private async Task UpdatePullRequestsAsync(Repository repository, Octokit.GitHub
                 Log.Error($"Error updating Check Status for Pull Request #{pull.Number}: {e.Message}");
 
                 // Put the full stack trace in debug if this occurs to reduce log spam.
-                Log.Debug($"Error updating Check Status for Pull Request #{pull.Number}.", e);
+                Log.Debug(e, $"Error updating Check Status for Pull Request #{pull.Number}.");
             }
 
             var commitCombinedStatus = await client.Repository.Status.GetCombined(repository.InternalId, dsPullRequest.HeadSha);

diff --git a/src/GitHubExtension/DataManager/GitHubDataManagerUpdate.cs b/src/GitHubExtension/DataManager/GitHubDataManagerUpdate.cs
@@ -27,7 +27,7 @@ public static async Task Update()
         }
         catch (Exception ex)
         {
-            Log.Error("Update failed unexpectedly.", ex);
+            Log.Error(ex, "Update failed unexpectedly.");
         }
 
         lastUpdateTime = DateTime.Now;

diff --git a/src/GitHubExtension/DataManager/GitHubSearchManger.cs b/src/GitHubExtension/DataManager/GitHubSearchManger.cs
@@ -33,7 +33,7 @@ public GitHubSearchManager()
         }
         catch (Exception e)
         {
-            Log.Error(Name, "Failed creating GitHubSearchManager", e);
+            Log.Error(e, "Failed creating GitHubSearchManager");
             Environment.FailFast(e.Message, e);
             return null;
         }

diff --git a/src/GitHubExtension/DataModel/DataObjects/Notification.cs b/src/GitHubExtension/DataModel/DataObjects/Notification.cs
@@ -98,7 +98,7 @@ public bool Toasted
                     // Catch errors so we do not throw for something like this. The local ToastState
                     // will still be set even if the datastore update fails. This could result in a
                     // toast later being shown twice, however, so report it as an error.
-                    Log.Error("Failed setting Notification ToastState for Notification Id = {Id}", ex);
+                    Log.Error(ex, "Failed setting Notification ToastState for Notification Id = {Id}");
                 }
             }
         }
@@ -194,7 +194,7 @@ private bool ShowFailedCheckRunToast()
         }
         catch (Exception ex)
         {
-            Log.Error($"Failed creating the Notification for {this}", ex);
+            Log.Error(ex, $"Failed creating the Notification for {this}");
             return false;
         }
 
@@ -224,7 +224,7 @@ private bool ShowSucceededCheckRunToast()
         }
         catch (Exception ex)
         {
-            Log.Error($"Failed creating the Notification for {this}", ex);
+            Log.Error(ex, $"Failed creating the Notification for {this}");
             return false;
         }
 
@@ -267,7 +267,7 @@ private bool ShowNewReviewToast()
         }
         catch (Exception ex)
         {
-            Log.Error($"Failed creating the Notification for {this}", ex);
+            Log.Error(ex, $"Failed creating the Notification for {this}");
             return false;
         }
 

diff --git a/src/GitHubExtension/DataModel/DataStore.cs b/src/GitHubExtension/DataModel/DataStore.cs
@@ -66,7 +66,7 @@ public bool Create(bool deleteExistingDatabase = false)
                     // if we had a problem opening the DB and fetching the pragma, then
                     // we surely cannot reuse it.
                     deleteExistingDatabase = true;
-                    Log.Error($"Unable to open existing database to verify schema. Deleting database.", e);
+                    Log.Error(e, $"Unable to open existing database to verify schema. Deleting database.");
                 }
             }
 
@@ -92,16 +92,16 @@ public bool Create(bool deleteExistingDatabase = false)
                 {
                     if ((uint)e.HResult == 0x80070020)
                     {
-                        Log.Fatal($"Sharing Violation Error; datastore exists and cannot be deleted ({DataStoreFilePath})", e);
+                        Log.Fatal(e, $"Sharing Violation Error; datastore exists and cannot be deleted ({DataStoreFilePath})");
                     }
                     else
                     {
-                        Log.Fatal($"I/O Error ({DataStoreFilePath})", e);
+                        Log.Fatal(e, $"I/O Error ({DataStoreFilePath})");
                     }
                 }
                 catch (UnauthorizedAccessException e)
                 {
-                    Log.Fatal($"Access Denied ({e})", e);
+                    Log.Fatal(e, $"Access Denied ({e})");
                 }
             }
         }
@@ -125,7 +125,7 @@ public bool Create(bool deleteExistingDatabase = false)
             }
             catch (Exception e)
             {
-                Log.Fatal($"Failed creating directory: ({directory})", e);
+                Log.Fatal(e, $"Failed creating directory: ({directory})");
             }
         }
 
@@ -163,7 +163,7 @@ public void Open()
         }
         catch (SqliteException e)
         {
-            Log.Fatal($"Failed to open connection: {Connection.ConnectionString}", e);
+            Log.Fatal(e, $"Failed to open connection: {Connection.ConnectionString}");
         }
 
         Log.Debug($"Opened DataStore at {DataStoreFilePath}");
@@ -259,7 +259,7 @@ private void Execute(string sql)
         }
         catch (SqliteException e)
         {
-            Log.Error($"Failure executing SQL Command: {command.CommandText}", e);
+            Log.Error(e, $"Failure executing SQL Command: {command.CommandText}");
         }
     }
 

diff --git a/src/GitHubExtension/DeveloperId/CredentialVault.cs b/src/GitHubExtension/DeveloperId/CredentialVault.cs
@@ -130,7 +130,7 @@ public void SaveCredentials(string loginId, SecureString? accessToken)
         }
         catch (Exception ex)
         {
-            Log.Error($"Retrieving credentials from Credential Manager has failed unexpectedly: {loginId} : ", ex);
+            Log.Error(ex, $"Retrieving credentials from Credential Manager has failed unexpectedly: {loginId} : ");
             throw;
         }
         finally
@@ -217,7 +217,7 @@ public void RemoveAllCredentials()
             }
             catch (Exception ex)
             {
-                Log.Error($"Deleting credentials from Credential Manager has failed unexpectedly: {credential} : ", ex);
+                Log.Error(ex, $"Deleting credentials from Credential Manager has failed unexpectedly: {credential} : ");
             }
         }
     }

diff --git a/src/GitHubExtension/DeveloperId/DeveloperIdProvider.cs b/src/GitHubExtension/DeveloperId/DeveloperIdProvider.cs
@@ -75,7 +75,7 @@ private DeveloperIdProvider()
         }
         catch (Exception ex)
         {
-            Log.Error($"Error while restoring DeveloperIds: {ex.Message}. Proceeding without restoring.", ex);
+            Log.Error(ex, $"Error while restoring DeveloperIds: {ex.Message}. Proceeding without restoring.");
         }
     }
 
@@ -131,7 +131,7 @@ public DeveloperId LoginNewDeveloperIdWithPAT(Uri hostAddress, SecureString pers
         }
         catch (Exception ex)
         {
-            Log.Error($"Error while logging in with PAT to {hostAddress.AbsoluteUri} : ", ex);
+            Log.Error(ex, $"Error while logging in with PAT to {hostAddress.AbsoluteUri} : ");
             throw;
         }
     }
@@ -151,7 +151,7 @@ public DeveloperId LoginNewDeveloperIdWithPAT(Uri hostAddress, SecureString pers
             catch (Exception ex)
             {
                 OAuthRequests.Remove(oauthRequest);
-                Log.Error($"Unable to complete OAuth request: ", ex);
+                Log.Error(ex, $"Unable to complete OAuth request: ");
             }
         }
 
@@ -180,7 +180,7 @@ public ProviderOperationResult LogoutDeveloperId(IDeveloperId developerId)
         }
         catch (Exception ex)
         {
-            Log.Error($"LoggedOut event signaling failed: ", ex);
+            Log.Error(ex, $"LoggedOut event signaling failed: ");
         }
 
         return new ProviderOperationResult(ProviderOperationStatus.Success, null, "The developer account has been logged out successfully", "LogoutDeveloperId succeeded");
@@ -264,7 +264,7 @@ private void SaveOrOverwriteDeveloperId(DeveloperId newDeveloperId, SecureString
                 }
                 catch (Exception ex)
                 {
-                    Log.Error($"Updated event signaling failed: ", ex);
+                    Log.Error(ex, $"Updated event signaling failed: ");
                 }
             }
             catch (InvalidOperationException)
@@ -288,7 +288,7 @@ private void SaveOrOverwriteDeveloperId(DeveloperId newDeveloperId, SecureString
             }
             catch (Exception ex)
             {
-                Log.Error($"LoggedIn event signaling failed: ", ex);
+                Log.Error(ex, $"LoggedIn event signaling failed: ");
             }
         }
     }
@@ -350,7 +350,7 @@ private void RestoreDeveloperIds(IEnumerable<string> loginIdsAndUrls)
             }
             catch (Exception ex)
             {
-                Log.Error($"Error while restoring DeveloperId {loginIdOrUrl} : ", ex);
+                Log.Error(ex, $"Error while restoring DeveloperId {loginIdOrUrl} : ");
 
                 // If we are unable to restore a DeveloperId, remove it from CredentialManager to avoid
                 // the same error next time, and to force the user to login again
@@ -373,7 +373,7 @@ private void ReplaceSavedLoginIdWithUrl(DeveloperId developerId)
         }
         catch (Exception ex)
         {
-            Log.Error($"Error while replacing {developerId.LoginId} with {developerId.Url} in CredentialManager: ", ex);
+            Log.Error(ex, $"Error while replacing {developerId.LoginId} with {developerId.Url} in CredentialManager: ");
         }
     }
 

diff --git a/src/GitHubExtension/DeveloperId/LoginUIController.cs b/src/GitHubExtension/DeveloperId/LoginUIController.cs
@@ -98,7 +98,7 @@ public IAsyncOperation<ProviderOperationResult> OnAction(string action, string i
                         }
                         catch (Exception ex)
                         {
-                            Log.Error($"Error: ", ex);
+                            Log.Error(ex, $"Error: ");
                             new LoginFailedPage().UpdateExtensionAdaptiveCard(_loginUI);
                             operationResult = new ProviderOperationResult(ProviderOperationStatus.Failure, ex, "Error occurred in login page", ex.Message);
                         }
@@ -149,13 +149,13 @@ public IAsyncOperation<ProviderOperationResult> OnAction(string action, string i
                         }
                         catch (UriFormatException ufe)
                         {
-                            Log.Error($"Error: ", ufe);
+                            Log.Error(ufe, $"Error: ");
                             operationResult = new EnterpriseServerPage(hostAddress: enterprisePageInputPayload.EnterpriseServer, errorText: $"{Resources.GetResource("LoginUI_EnterprisePage_UriErrorText")}").UpdateExtensionAdaptiveCard(_loginUI);
                             break;
                         }
                         catch (Exception ex)
                         {
-                            Log.Error($"Error: ", ex);
+                            Log.Error(ex, $"Error: ");
                             operationResult = new EnterpriseServerPage(hostAddress: enterprisePageInputPayload.EnterpriseServer, errorText: $"{Resources.GetResource("LoginUI_EnterprisePage_GenericErrorText")} : {ex}").UpdateExtensionAdaptiveCard(_loginUI);
                             break;
                         }
@@ -166,7 +166,7 @@ public IAsyncOperation<ProviderOperationResult> OnAction(string action, string i
                         }
                         catch (Exception ex)
                         {
-                            Log.Error($"Error: ", ex);
+                            Log.Error(ex, $"Error: ");
                             operationResult = new LoginFailedPage().UpdateExtensionAdaptiveCard(_loginUI);
                         }
 
@@ -220,13 +220,13 @@ public IAsyncOperation<ProviderOperationResult> OnAction(string action, string i
                             }
                             catch (UriFormatException ufe)
                             {
-                                Log.Error($"Error: ", ufe);
+                                Log.Error(ufe, $"Error: ");
                                 operationResult = new ProviderOperationResult(ProviderOperationStatus.Failure, null, $"Error: {ufe}", $"Error: {ufe}");
                                 break;
                             }
                             catch (Exception ex)
                             {
-                                Log.Error($"Error: ", ex);
+                                Log.Error(ex, $"Error: ");
                                 operationResult = new ProviderOperationResult(ProviderOperationStatus.Failure, null, $"Error: {ex}", $"Error: {ex}");
                                 break;
                             }
@@ -277,12 +277,12 @@ public IAsyncOperation<ProviderOperationResult> OnAction(string action, string i
                         {
                             if (ex.Message.Contains("Bad credentials") || ex.Message.Contains("Not Found"))
                             {
-                                Log.Error($"Unauthorized Error: ", ex);
+                                Log.Error(ex, $"Unauthorized Error: ");
                                 operationResult = new EnterpriseServerPATPage(hostAddress: _hostAddress, errorText: $"{Resources.GetResource("LoginUI_EnterprisePATPage_BadCredentialsErrorText")} {_hostAddress.OriginalString}", inputPAT: new NetworkCredential(null, enterprisePATPageInputPayload?.PAT).SecurePassword).UpdateExtensionAdaptiveCard(_loginUI);
                                 break;
                             }
 
-                            Log.Error($"Error: ", ex);
+                            Log.Error(ex, $"Error: ");
                             operationResult = new EnterpriseServerPATPage(hostAddress: _hostAddress, errorText: $"{Resources.GetResource("LoginUI_EnterprisePATPage_GenericErrorPrefix")} {ex}", inputPAT: new NetworkCredential(null, enterprisePATPageInputPayload?.PAT).SecurePassword).UpdateExtensionAdaptiveCard(_loginUI);
                             break;
                         }

diff --git a/src/GitHubExtension/Helpers/Resources.cs b/src/GitHubExtension/Helpers/Resources.cs
@@ -23,7 +23,7 @@ public static string GetResource(string identifier, ILogger? log = null)
         }
         catch (Exception ex)
         {
-            log?.Error($"Failed loading resource: {identifier}", ex);
+            log?.Error(ex, $"Failed loading resource: {identifier}");
 
             // If we fail, load the original identifier so it is obvious which resource is missing.
             return identifier;

diff --git a/src/GitHubExtension/Notifications/NotificationHandler.cs b/src/GitHubExtension/Notifications/NotificationHandler.cs
@@ -47,7 +47,7 @@ public static void NotificationActivation(AppNotificationActivatedEventArgs args
             }
             catch (Exception ex)
             {
-                Log.Error($"Failed launching Uri for {args.Arguments["htmlurl"]}", ex);
+                Log.Error(ex, $"Failed launching Uri for {args.Arguments["htmlurl"]}");
             }
 
             return;
-Original file line number
+Diff line change
@@ Expand Up @@
                 }
                 catch (Exception ex)
                 {
-                    Log.Error($"Rate limiting not enabled for server.", ex);
+                    Log.Error(ex, $"Rate limiting not enabled for server.");
                 }
             }
@@ Expand Down @@