Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix exception logging #379

Merged
merged 1 commit into from
Apr 11, 2024
Merged

Fix exception logging #379

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/GitHubExtension/Client/GithubClientProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.");
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/GitHubExtension/Client/Validation.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
12 changes: 6 additions & 6 deletions src/GitHubExtension/DataManager/GitHubDataManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
Expand All @@ -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}.");
}
}

Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/GitHubExtension/DataManager/GitHubDataManagerUpdate.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/GitHubExtension/DataManager/GitHubSearchManger.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
8 changes: 4 additions & 4 deletions src/GitHubExtension/DataModel/DataObjects/Notification.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}");
}
}
}
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
}

Expand Down
14 changes: 7 additions & 7 deletions src/GitHubExtension/DataModel/DataStore.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.");
}
}

Expand All @@ -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})");
}
}
}
Expand All @@ -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})");
}
}

Expand Down Expand Up @@ -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}");
Expand Down Expand Up @@ -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}");
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/GitHubExtension/DeveloperId/CredentialVault.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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} : ");
}
}
}
Expand Down
16 changes: 8 additions & 8 deletions src/GitHubExtension/DeveloperId/DeveloperIdProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.");
}
}

Expand Down Expand Up @@ -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;
}
}
Expand All @@ -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: ");
}
}

Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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)
Expand All @@ -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: ");
}
}
}
Expand Down Expand Up @@ -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
Expand All @@ -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: ");
}
}

Expand Down
16 changes: 8 additions & 8 deletions src/GitHubExtension/DeveloperId/LoginUIController.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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;
}
Expand All @@ -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);
}

Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion src/GitHubExtension/Helpers/Resources.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/GitHubExtension/Notifications/NotificationHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading
Loading