diff --git a/src/shared/Atlassian.Bitbucket.UI/Commands/CredentialsCommand.cs b/src/shared/Atlassian.Bitbucket.UI/Commands/CredentialsCommand.cs index a17bdf5f3e..c4b1c2b726 100644 --- a/src/shared/Atlassian.Bitbucket.UI/Commands/CredentialsCommand.cs +++ b/src/shared/Atlassian.Bitbucket.UI/Commands/CredentialsCommand.cs @@ -48,7 +48,9 @@ private async Task ExecuteAsync(Uri url, string userName, bool showOAuth, b if (!viewModel.WindowResult || viewModel.SelectedMode == AuthenticationModes.None) { - throw new Exception("User cancelled dialog."); + var message = "User cancelled dialog."; + Context.Trace2.WriteError(message); + throw new Exception(message); } switch (viewModel.SelectedMode) @@ -70,8 +72,11 @@ private async Task ExecuteAsync(Uri url, string userName, bool showOAuth, b break; default: - throw new ArgumentOutOfRangeException(nameof(AuthenticationModes), - "Unknown authentication mode", viewModel.SelectedMode.ToString()); + var actualValue = "Unknown authentication mode"; + var message = viewModel.SelectedMode.ToString(); + + Context.Trace2.WriteError($"{actualValue}: {message}"); + throw new ArgumentOutOfRangeException(nameof(AuthenticationModes), actualValue, message); } return 0; diff --git a/src/shared/Atlassian.Bitbucket/BitbucketAuthentication.cs b/src/shared/Atlassian.Bitbucket/BitbucketAuthentication.cs index 1cacd769e3..a2053b7a7f 100644 --- a/src/shared/Atlassian.Bitbucket/BitbucketAuthentication.cs +++ b/src/shared/Atlassian.Bitbucket/BitbucketAuthentication.cs @@ -88,6 +88,10 @@ public async Task GetCredentialsAsync(Uri targetUri, st // We need at least one mode! if (modes == AuthenticationModes.None) { + var format = @"Must specify at least one {0}"; + var message = string.Format(format, nameof(AuthenticationModes)); + + Context.Trace2.WriteError(message, format); throw new ArgumentException(@$"Must specify at least one {nameof(AuthenticationModes)}", nameof(modes)); } @@ -128,12 +132,16 @@ public async Task GetCredentialsAsync(Uri targetUri, st { if (!output.TryGetValue("username", out userName)) { - throw new Exception("Missing username in response"); + var message = "Missing username in response"; + Context.Trace2.WriteError(message); + throw new Exception(message); } if (!output.TryGetValue("password", out password)) { - throw new Exception("Missing password in response"); + var message = "Missing password in response"; + Context.Trace2.WriteError(message); + throw new Exception(message); } return new CredentialsPromptResult( @@ -172,7 +180,11 @@ public async Task GetCredentialsAsync(Uri targetUri, st return new CredentialsPromptResult(AuthenticationModes.OAuth); case AuthenticationModes.None: - throw new ArgumentOutOfRangeException(nameof(modes), @$"At least one {nameof(AuthenticationModes)} must be supplied"); + var format = "At least one {0} must be supplied"; + var message = string.Format(format, nameof(AuthenticationModes)); + + Context.Trace2.WriteError(message, format); + throw new ArgumentOutOfRangeException(nameof(modes), message); default: var menuTitle = $"Select an authentication method for '{targetUri}'"; @@ -190,7 +202,9 @@ public async Task GetCredentialsAsync(Uri targetUri, st if (choice == oauthItem) goto case AuthenticationModes.OAuth; if (choice == basicItem) goto case AuthenticationModes.Basic; - throw new Exception(); + Exception e = new Exception(); + Context.Trace2.WriteError(e.Message); + throw e; } } } diff --git a/src/shared/Atlassian.Bitbucket/BitbucketHostProvider.cs b/src/shared/Atlassian.Bitbucket/BitbucketHostProvider.cs index f1950aca18..c9fb30815d 100644 --- a/src/shared/Atlassian.Bitbucket/BitbucketHostProvider.cs +++ b/src/shared/Atlassian.Bitbucket/BitbucketHostProvider.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.Net; using System.Net.Http; +using System.Threading; using System.Threading.Tasks; using Atlassian.Bitbucket.Cloud; using GitCredentialManager; @@ -79,7 +80,10 @@ public async Task GetCredentialAsync(InputArguments input) if (StringComparer.OrdinalIgnoreCase.Equals(input.Protocol, "http") && BitbucketHelper.IsBitbucketOrg(input)) { - throw new Exception("Unencrypted HTTP is not supported for Bitbucket.org. Ensure the repository remote URL is using HTTPS."); + var message = + "Unencrypted HTTP is not supported for Bitbucket.org. Ensure the repository remote URL is using HTTPS."; + _context.Trace2.WriteError(message); + throw new Exception(message); } var authModes = await GetSupportedAuthenticationModesAsync(input); @@ -145,8 +149,10 @@ private async Task GetRefreshedCredentials(InputArguments input, Au var result = await _bitbucketAuth.GetCredentialsAsync(remoteUri, input.UserName, authModes); if (result is null || result.AuthenticationMode == AuthenticationModes.None) { - _context.Trace.WriteLine("User cancelled credential prompt"); - throw new Exception("User cancelled credential prompt."); + var message = "User cancelled credential prompt"; + _context.Trace.WriteLine(message); + _context.Trace2.WriteError(message); + throw new Exception(message); } switch (result.AuthenticationMode) @@ -160,8 +166,10 @@ private async Task GetRefreshedCredentials(InputArguments input, Au break; default: - throw new ArgumentOutOfRangeException( - $"Unexpected {nameof(AuthenticationModes)} returned from prompt"); + var format = "Unexpected {0} returned from prompt"; + var message = string.Format(format, nameof(AuthenticationModes)); + _context.Trace2.WriteError(message, format); + throw new ArgumentOutOfRangeException(message); } // Fall through to the start of the interactive OAuth authentication flow @@ -176,8 +184,10 @@ private async Task GetRefreshedCredentials(InputArguments input, Au } catch (OAuth2Exception ex) { - _context.Trace.WriteLine("Failed to refresh existing OAuth credential using refresh token"); + var message = "Failed to refresh existing OAuth credential using refresh token"; + _context.Trace.WriteLine(message); _context.Trace.WriteException(ex); + _context.Trace2.WriteError(message); // We failed to refresh the AT using the RT; log the refresh failure and fall through to restart // the OAuth authentication flow @@ -279,7 +289,7 @@ public async Task GetSupportedAuthenticationModesAsync(Inpu try { var authenticationMethods = await _restApiRegistry.Get(input).GetAuthenticationMethodsAsync(); - + var modes = AuthenticationModes.None; if (authenticationMethods.Contains(AuthenticationMethod.BasicAuth)) @@ -298,8 +308,12 @@ public async Task GetSupportedAuthenticationModesAsync(Inpu } catch (Exception ex) { - _context.Trace.WriteLine($"Failed to query '{input.GetRemoteUri()}' for supported authentication schemes."); + var format = "Failed to query '{0}' for supported authentication schemes."; + var message = string.Format(format, input.GetRemoteUri()); + + _context.Trace.WriteLine(message); _context.Trace.WriteException(ex); + _context.Trace2.WriteError(message, format); _context.Terminal.WriteLine($"warning: failed to query '{input.GetRemoteUri()}' for supported authentication schemes."); @@ -356,7 +370,11 @@ private async Task ResolveOAuthUserNameAsync(InputArguments input, strin return result.Response.UserName; } - throw new Exception($"Failed to resolve username. HTTP: {result.StatusCode}"); + var format = "Failed to resolve username. HTTP: {0}"; + var message = string.Format(format, result.StatusCode); + _context.Trace2.WriteError(message, format); + + throw new Exception(message); } private async Task ResolveBasicAuthUserNameAsync(InputArguments input, string username, string password) @@ -367,7 +385,11 @@ private async Task ResolveBasicAuthUserNameAsync(InputArguments input, s return result.Response.UserName; } - throw new Exception($"Failed to resolve username. HTTP: {result.StatusCode}"); + var format = "Failed to resolve username. HTTP: {0}"; + var message = string.Format(format, result.StatusCode); + _context.Trace2.WriteError(message, format); + + throw new Exception(message); } private async Task ValidateCredentialsWork(InputArguments input, ICredential credentials, AuthenticationModes authModes) @@ -404,8 +426,10 @@ private async Task ValidateCredentialsWork(InputArguments input, ICredenti } catch (Exception ex) { - _context.Trace.WriteLine($"Failed to validate existing credentials using OAuth"); + var message = "Failed to validate existing credentials using OAuth"; + _context.Trace.WriteLine(message); _context.Trace.WriteException(ex); + _context.Trace2.WriteError(message); } } @@ -419,8 +443,10 @@ private async Task ValidateCredentialsWork(InputArguments input, ICredenti } catch (Exception ex) { - _context.Trace.WriteLine($"Failed to validate existing credentials using Basic Auth"); + var message = "Failed to validate existing credentials using Basic Auth"; + _context.Trace.WriteLine(message); _context.Trace.WriteException(ex); + _context.Trace2.WriteError(message); return false; } } diff --git a/src/shared/Atlassian.Bitbucket/DataCenter/BitbucketRestApi.cs b/src/shared/Atlassian.Bitbucket/DataCenter/BitbucketRestApi.cs index 0688e03235..25264960d0 100644 --- a/src/shared/Atlassian.Bitbucket/DataCenter/BitbucketRestApi.cs +++ b/src/shared/Atlassian.Bitbucket/DataCenter/BitbucketRestApi.cs @@ -20,7 +20,7 @@ public BitbucketRestApi(ICommandContext context) EnsureArgument.NotNull(context, nameof(context)); _context = context; - + } public async Task> GetUserInformationAsync(string userName, string password, bool isBearerToken) @@ -35,7 +35,7 @@ public async Task> GetUserInformationAsync(string userN } // Bitbucket Server/DC doesn't actually provide a REST API we can use to trade an access_token for the owning username, - // therefore this is always going to return a placeholder username, however this call does provide a way to validate the + // therefore this is always going to return a placeholder username, however this call does provide a way to validate the // credentials we do have var requestUri = new Uri(ApiUri, "api/1.0/users"); using (HttpRequestMessage request = new HttpRequestMessage(HttpMethod.Get, requestUri)) @@ -131,14 +131,16 @@ public void Dispose() private HttpClient HttpClient => _httpClient ??= _context.HttpClientFactory.CreateClient(); - private Uri ApiUri + private Uri ApiUri { - get + get { var remoteUri = _context.Settings?.RemoteUri; if (remoteUri == null) { - throw new ArgumentException("RemoteUri must be defined to generate Bitbucket DC OAuth2 endpoint Urls"); + var message = "RemoteUri must be defined to generate Bitbucket DC OAuth2 endpoint Urls"; + _context.Trace2.WriteError(message); + throw new ArgumentException(message); } return new Uri(BitbucketHelper.GetBaseUri(remoteUri) + "/rest/"); diff --git a/src/shared/Core.UI/Commands/CredentialsCommand.cs b/src/shared/Core.UI/Commands/CredentialsCommand.cs index 02da8472f6..883b3ae416 100644 --- a/src/shared/Core.UI/Commands/CredentialsCommand.cs +++ b/src/shared/Core.UI/Commands/CredentialsCommand.cs @@ -63,7 +63,9 @@ private async Task ExecuteAsync(CommandOptions options) if (!viewModel.WindowResult) { - throw new Exception("User cancelled dialog."); + var message = "User cancelled dialog."; + Context.Trace2.WriteError(message); + throw new Exception(message); } WriteResult( diff --git a/src/shared/Core.UI/Commands/DeviceCodeCommand.cs b/src/shared/Core.UI/Commands/DeviceCodeCommand.cs index 863e24a1de..34fe24f4f5 100644 --- a/src/shared/Core.UI/Commands/DeviceCodeCommand.cs +++ b/src/shared/Core.UI/Commands/DeviceCodeCommand.cs @@ -41,7 +41,9 @@ private async Task ExecuteAsync(string code, string url, bool noLogo) if (!viewModel.WindowResult) { - throw new Exception("User cancelled dialog."); + var message = "User cancelled dialog."; + Context.Trace2.WriteError(message); + throw new Exception(message); } return 0; diff --git a/src/shared/Core.UI/Commands/OAuthCommand.cs b/src/shared/Core.UI/Commands/OAuthCommand.cs index aa48d7eb17..9aa784647d 100644 --- a/src/shared/Core.UI/Commands/OAuthCommand.cs +++ b/src/shared/Core.UI/Commands/OAuthCommand.cs @@ -66,7 +66,9 @@ private async Task ExecuteAsync(CommandOptions options) if (!viewModel.WindowResult) { - throw new Exception("User cancelled dialog."); + var message = "User cancelled dialog."; + Context.Trace2.WriteError(message); + throw new Exception(message); } var result = new Dictionary(); @@ -81,7 +83,9 @@ private async Task ExecuteAsync(CommandOptions options) break; default: - throw new ArgumentOutOfRangeException(); + var e = new ArgumentOutOfRangeException(); + Context.Trace2.WriteError(e.Message); + throw e; } WriteResult(result); diff --git a/src/shared/Core.UI/HelperApplication.cs b/src/shared/Core.UI/HelperApplication.cs index f5504e79c5..667190bdda 100644 --- a/src/shared/Core.UI/HelperApplication.cs +++ b/src/shared/Core.UI/HelperApplication.cs @@ -58,6 +58,7 @@ private bool WriteException(Exception ex) { ["error"] = ex.Message }); + Context.Trace2.WriteError(ex.Message); return true; } diff --git a/src/shared/Core/Application.cs b/src/shared/Core/Application.cs index 6a6776196d..59d7e1c312 100644 --- a/src/shared/Core/Application.cs +++ b/src/shared/Core/Application.cs @@ -134,18 +134,32 @@ private void OnException(Exception ex, InvocationContext invocationContext) private bool WriteException(Exception ex) { + string format; + string message; + // Try and use a nicer format for some well-known exception types switch (ex) { case GitException gitEx: - Context.Streams.Error.WriteLine("fatal: {0} [{1}]", gitEx.Message, gitEx.ExitCode); + format = "fatal: {0} [{1}]"; + message = string.Format(format, gitEx.Message, gitEx.ExitCode); + + Context.Streams.Error.WriteLine(message); Context.Streams.Error.WriteLine(gitEx.GitErrorMessage); + + format += "{2}{3}"; + message = string.Format(format, gitEx.Message, Environment.NewLine, gitEx.GitErrorMessage); + Context.Trace2.WriteError(message); break; case InteropException interopEx: - Context.Streams.Error.WriteLine("fatal: {0} [0x{1:x}]", interopEx.Message, interopEx.ErrorCode); + format = "fatal: {0} [0x{1:x}]"; + message = string.Format(format, interopEx.Message, interopEx.ErrorCode); + Context.Streams.Error.WriteLine(message); break; default: - Context.Streams.Error.WriteLine("fatal: {0}", ex.Message); + format = "fatal: {0}"; + message = string.Format(format, ex.Message); + Context.Streams.Error.WriteLine(message); break; } diff --git a/src/shared/Core/Authentication/AuthenticationBase.cs b/src/shared/Core/Authentication/AuthenticationBase.cs index d700f815f4..f649e56ccc 100644 --- a/src/shared/Core/Authentication/AuthenticationBase.cs +++ b/src/shared/Core/Authentication/AuthenticationBase.cs @@ -46,7 +46,11 @@ protected AuthenticationBase(ICommandContext context) var process = ChildProcess.Start(Context.Trace2, procStartInfo, Trace2ProcessClass.UIHelper); if (process is null) { - throw new Exception($"Failed to start helper process: {path} {args}"); + var format = "Failed to start helper process: {0} {1}"; + var message = string.Format(format, path, args); + + Context.Trace2.WriteError(message, format); + throw new Exception(message); } // Kill the process upon a cancellation request @@ -69,7 +73,11 @@ protected AuthenticationBase(ICommandContext context) errorMessage = "Unknown"; } - throw new Exception($"helper error ({exitCode}): {errorMessage}"); + var format = "helper error ({0}): {1}"; + var message = string.Format(format, exitCode, errorMessage); + + Context.Trace2.WriteError(message, format); + throw new Exception(message); } return resultDict; @@ -86,7 +94,10 @@ protected void ThrowIfUserInteractionDisabled() Context.Trace.WriteLine($"{envName} / {cfgName} is false/never; user interactivity has been disabled."); - throw new InvalidOperationException("Cannot prompt because user interactivity has been disabled."); + var message = "Cannot prompt because user interactivity has been disabled."; + Context.Trace2.WriteError(message); + + throw new InvalidOperationException(message); } } @@ -96,7 +107,10 @@ protected void ThrowIfGuiPromptsDisabled() { Context.Trace.WriteLine($"{Constants.EnvironmentVariables.GitTerminalPrompts} is 0; GUI prompts have been disabled."); - throw new InvalidOperationException("Cannot show prompt because GUI prompts have been disabled."); + var message = "Cannot show prompt because GUI prompts have been disabled."; + Context.Trace2.WriteError(message); + + throw new InvalidOperationException(message); } } @@ -106,7 +120,10 @@ protected void ThrowIfTerminalPromptsDisabled() { Context.Trace.WriteLine($"{Constants.EnvironmentVariables.GitTerminalPrompts} is 0; terminal prompts have been disabled."); - throw new InvalidOperationException("Cannot prompt because terminal prompts have been disabled."); + var message = "Cannot prompt because terminal prompts have been disabled."; + Context.Trace2.WriteError(message); + + throw new InvalidOperationException(message); } } diff --git a/src/shared/Core/Authentication/BasicAuthentication.cs b/src/shared/Core/Authentication/BasicAuthentication.cs index 9d485105ee..bb550e9499 100644 --- a/src/shared/Core/Authentication/BasicAuthentication.cs +++ b/src/shared/Core/Authentication/BasicAuthentication.cs @@ -85,12 +85,16 @@ private async Task GetCredentialsByUiAsync(string command, string a if (!resultDict.TryGetValue("username", out userName)) { - throw new Exception("Missing 'username' in response"); + var message = "Missing 'username' in response"; + Context.Trace2.WriteError(message); + throw new Exception(message); } if (!resultDict.TryGetValue("password", out string password)) { - throw new Exception("Missing 'password' in response"); + var message = "Missing 'password' in response"; + Context.Trace2.WriteError(message); + throw new Exception(message); } return new GitCredential(userName, password); diff --git a/src/shared/Core/Authentication/MicrosoftAuthentication.cs b/src/shared/Core/Authentication/MicrosoftAuthentication.cs index 491fffd0f3..9ef29a713f 100644 --- a/src/shared/Core/Authentication/MicrosoftAuthentication.cs +++ b/src/shared/Core/Authentication/MicrosoftAuthentication.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.IO; using System.Net.Http; +using System.Threading; using System.Threading.Tasks; using Microsoft.Identity.Client; using Microsoft.Identity.Client.Extensions.Msal; @@ -336,6 +337,7 @@ private async Task RegisterTokenCacheAsync(IPublicClientApplication app, ITrace2 Context.Streams.Error.WriteLine("warning: cannot persist Microsoft authentication token cache securely!"); Context.Trace.WriteLine("Cannot persist Microsoft Authentication data securely!"); Context.Trace.WriteException(ex); + Context.Trace2.WriteError(ex.Message); if (PlatformUtils.IsMacOS()) { @@ -495,13 +497,15 @@ private bool CanUseEmbeddedWebView() private void EnsureCanUseEmbeddedWebView() { + var message = "Embedded web view is not available without a desktop session."; + Context.Trace2.WriteError(message); #if NETFRAMEWORK if (!Context.SessionManager.IsDesktopSession) { - throw new InvalidOperationException("Embedded web view is not available without a desktop session."); + throw new InvalidOperationException(message); } #else - throw new InvalidOperationException("Embedded web view is not available on .NET Core."); + throw new InvalidOperationException(message); #endif } @@ -513,18 +517,26 @@ private bool CanUseSystemWebView(IPublicClientApplication app, Uri redirectUri) private void EnsureCanUseSystemWebView(IPublicClientApplication app, Uri redirectUri) { + string message = ""; + if (!Context.SessionManager.IsWebBrowserAvailable) { - throw new InvalidOperationException("System web view is not available without a way to start a browser."); + message = "System web view is not available without a way to start a browser."; + Context.Trace2.WriteError(message); + throw new InvalidOperationException(message); } if (!app.IsSystemWebViewAvailable) { + message = "System web view is not available on this platform."; + Context.Trace2.WriteError(message); throw new InvalidOperationException("System web view is not available on this platform."); } if (!redirectUri.IsLoopback) { + message = "System web view is not available for this service configuration."; + Context.Trace2.WriteError(message); throw new InvalidOperationException("System web view is not available for this service configuration."); } } diff --git a/src/shared/Core/Authentication/OAuth/OAuth2Client.cs b/src/shared/Core/Authentication/OAuth/OAuth2Client.cs index 3fc6020288..a0fa0dbcfe 100644 --- a/src/shared/Core/Authentication/OAuth/OAuth2Client.cs +++ b/src/shared/Core/Authentication/OAuth/OAuth2Client.cs @@ -125,9 +125,11 @@ public IOAuth2CodeGenerator CodeGenerator { if (queryParams.ContainsKey(kvp.Key)) { - throw new ArgumentException( - $"Extra query parameter '{kvp.Key}' would override required standard OAuth parameters.", - nameof(extraQueryParams)); + var format = "Extra query parameter '{0}' would override required standard OAuth parameters."; + var message = string.Format(format, kvp.Key); + + _trace2.WriteError(message); + throw new ArgumentException(message, nameof(extraQueryParams)); } queryParams[kvp.Key] = kvp.Value; @@ -163,17 +165,26 @@ public IOAuth2CodeGenerator CodeGenerator IDictionary redirectQueryParams = finalUri.GetQueryParameters(); if (!redirectQueryParams.TryGetValue(OAuth2Constants.AuthorizationGrantResponse.StateParameter, out string replyState)) { - throw new OAuth2Exception($"Missing '{OAuth2Constants.AuthorizationGrantResponse.StateParameter}' in response."); + var format = "Missing '{0}' in response."; + var message = string.Format(format, OAuth2Constants.AuthorizationGrantResponse.StateParameter); + _trace2.WriteError(message, format); + throw new OAuth2Exception(message); } if (!StringComparer.Ordinal.Equals(state, replyState)) { - throw new OAuth2Exception($"Invalid '{OAuth2Constants.AuthorizationGrantResponse.StateParameter}' in response. Does not match initial request."); + var format = "Invalid '{0}' in response. Does not match initial request."; + var message = string.Format(format, OAuth2Constants.AuthorizationGrantResponse.StateParameter); + _trace2.WriteError(message, format); + throw new OAuth2Exception(message); } // We expect to have the auth code in the response otherwise terminate the flow (we failed authentication for some reason) if (!redirectQueryParams.TryGetValue(OAuth2Constants.AuthorizationGrantResponse.AuthorizationCodeParameter, out string authCode)) { - throw new OAuth2Exception($"Missing '{OAuth2Constants.AuthorizationGrantResponse.AuthorizationCodeParameter}' in response."); + var format = "Missing '{0}' in response."; + var message = string.Format(format, OAuth2Constants.AuthorizationGrantResponse.AuthorizationCodeParameter); + _trace2.WriteError(message, format); + throw new OAuth2Exception(message); } return new OAuth2AuthorizationCodeResult(authCode, redirectUri, codeVerifier); @@ -183,7 +194,9 @@ public async Task GetDeviceCodeAsync(IEnumerable { if (_endpoints.DeviceAuthorizationEndpoint is null) { - throw new InvalidOperationException("No device authorization endpoint has been configured for this client."); + var message = "No device authorization endpoint has been configured for this client."; + _trace2.WriteError(message); + throw new InvalidOperationException(message); } string scopesStr = string.Join(" ", scopes); @@ -381,12 +394,19 @@ private HttpRequestMessage CreateRequestMessage(HttpMethod method, Uri requestUr protected Exception CreateExceptionFromResponse(string json) { + string message; + if (TryCreateExceptionFromResponse(json, out OAuth2Exception exception)) { + message = exception.Message; + _trace2.WriteError(message); return exception; } - return new OAuth2Exception($"Unknown OAuth error: {json}"); + var format = "Unknown OAuth error: {0}"; + message = string.Format(format, json); + _trace2.WriteError(message, format); + return new OAuth2Exception(message); } protected static bool TryDeserializeJson(string json, out T obj) diff --git a/src/shared/Core/Authentication/OAuthAuthentication.cs b/src/shared/Core/Authentication/OAuthAuthentication.cs index 8da18faad4..045fe5db05 100644 --- a/src/shared/Core/Authentication/OAuthAuthentication.cs +++ b/src/shared/Core/Authentication/OAuthAuthentication.cs @@ -47,7 +47,11 @@ public OAuthAuthentication(ICommandContext context) // We need at least one mode! if (modes == OAuthAuthenticationModes.None) { - throw new ArgumentException(@$"Must specify at least one {nameof(OAuthAuthenticationModes)}", nameof(modes)); + var format = @"Must specify at least one {0}"; + var message = string.Format(format, nameof(OAuthAuthenticationModes)); + + Context.Trace2.WriteError(message, format); + throw new ArgumentException(message, nameof(modes)); } // If there is no mode choice to be made then just return that result @@ -82,7 +86,9 @@ public OAuthAuthentication(ICommandContext context) if (!resultDict.TryGetValue("mode", out string responseMode)) { - throw new Exception("Missing 'mode' in response"); + var message = "Missing 'mode' in response"; + Context.Trace2.WriteError(message); + throw new Exception(message); } switch (responseMode.ToLowerInvariant()) @@ -94,7 +100,11 @@ public OAuthAuthentication(ICommandContext context) return OAuthAuthenticationModes.DeviceCode; default: - throw new Exception($"Unknown mode value in response '{responseMode}'"); + var format = "Unknown mode value in response '{0}'"; + var message = string.Format(format, responseMode); + + Context.Trace2.WriteError(message, format); + throw new Exception(message); } } else @@ -125,7 +135,9 @@ public OAuthAuthentication(ICommandContext context) if (choice == browserItem) goto case OAuthAuthenticationModes.Browser; if (choice == deviceItem) goto case OAuthAuthenticationModes.DeviceCode; - throw new Exception(); + Exception e = new Exception(); + Context.Trace2.WriteError(e.Message); + throw e; } } } @@ -137,7 +149,9 @@ public async Task GetTokenByBrowserAsync(OAuth2Client client, // We require a desktop session to launch the user's default web browser if (!Context.SessionManager.IsDesktopSession) { - throw new InvalidOperationException("Browser authentication requires a desktop session"); + var message = "Browser authentication requires a desktop session"; + Context.Trace2.WriteError(message); + throw new InvalidOperationException(message); } var browserOptions = new OAuth2WebBrowserOptions(); @@ -185,7 +199,9 @@ public async Task GetTokenByDeviceCodeAsync(OAuth2Client clie } catch (OperationCanceledException) { - throw new Exception("User canceled device code authentication"); + var message = "User canceled device code authentication"; + Context.Trace2.WriteError(message); + throw new Exception(message); } // Close the dialog diff --git a/src/shared/Core/Commands/GitCommandBase.cs b/src/shared/Core/Commands/GitCommandBase.cs index 2bb6d0fe53..acf6b31ec8 100644 --- a/src/shared/Core/Commands/GitCommandBase.cs +++ b/src/shared/Core/Commands/GitCommandBase.cs @@ -58,22 +58,30 @@ protected virtual void EnsureMinimumInputArguments(InputArguments input) { if (input.Protocol is null) { - throw new InvalidOperationException("Missing 'protocol' input argument"); + var message = "Missing 'protocol' input argument"; + Context.Trace2.WriteError(message); + throw new InvalidOperationException(message); } if (string.IsNullOrWhiteSpace(input.Protocol)) { - throw new InvalidOperationException("Invalid 'protocol' input argument (cannot be empty)"); + var message = "Invalid 'protocol' input argument (cannot be empty)"; + Context.Trace2.WriteError(message); + throw new InvalidOperationException(message); } if (input.Host is null) { - throw new InvalidOperationException("Missing 'host' input argument"); + var message = "Missing 'host' input argument"; + Context.Trace2.WriteError(message); + throw new InvalidOperationException(message); } if (string.IsNullOrWhiteSpace(input.Host)) { - throw new InvalidOperationException("Invalid 'host' input argument (cannot be empty)"); + var message = "Invalid 'host' input argument (cannot be empty)"; + Context.Trace2.WriteError(message); + throw new InvalidOperationException(message); } } diff --git a/src/shared/Core/Commands/StoreCommand.cs b/src/shared/Core/Commands/StoreCommand.cs index 7a4f078d50..2d6351b319 100644 --- a/src/shared/Core/Commands/StoreCommand.cs +++ b/src/shared/Core/Commands/StoreCommand.cs @@ -23,12 +23,16 @@ protected override void EnsureMinimumInputArguments(InputArguments input) // An empty string username/password are valid inputs, so only check for `null` (not provided) if (input.UserName is null) { - throw new InvalidOperationException("Missing 'username' input argument"); + var message = "Missing 'username' input argument"; + Context.Trace2.WriteError(message); + throw new InvalidOperationException(message); } if (input.Password is null) { - throw new InvalidOperationException("Missing 'password' input argument"); + var message = "Missing 'password' input argument"; + Context.Trace2.WriteError(message); + throw new InvalidOperationException(message); } } } diff --git a/src/shared/Core/CredentialStore.cs b/src/shared/Core/CredentialStore.cs index 3c0d221c83..1b37f2d655 100644 --- a/src/shared/Core/CredentialStore.cs +++ b/src/shared/Core/CredentialStore.cs @@ -95,19 +95,22 @@ private void EnsureBackingStore() default: var sb = new StringBuilder(); - sb.AppendLine(string.IsNullOrWhiteSpace(credStoreName) - ? "No credential store has been selected." - : $"Unknown credential store '{credStoreName}'."); - sb.AppendFormat( - "{3}Set the {0} environment variable or the {1}.{2} Git configuration setting to one of the following options:{3}{3}", + AppendAvailableStoreList(sb); + var format = string.IsNullOrWhiteSpace(credStoreName) + ? "No credential store has been selected." : "Unknown credential store '{0}'." + + "{4}{4}Set the {1} environment variable or the {2}.{3} Git configuration setting to one of the following options:{4}{4}{5}" + + "{4}See {6} for more information."; + var message = string.Format(format, + credStoreName, Constants.EnvironmentVariables.GcmCredentialStore, Constants.GitConfiguration.Credential.SectionName, Constants.GitConfiguration.Credential.CredentialStore, - Environment.NewLine); - AppendAvailableStoreList(sb); - sb.AppendLine(); - sb.AppendLine($"See {Constants.HelpUrls.GcmCredentialStores} for more information."); - throw new Exception(sb.ToString()); + Environment.NewLine, + sb, + Constants.HelpUrls.GcmCredentialStores + ); + _context.Trace2.WriteError(message, format); + throw new Exception(message); } } @@ -166,20 +169,26 @@ private void ValidateWindowsCredentialManager() { if (!PlatformUtils.IsWindows()) { - throw new Exception( - $"Can only use the '{StoreNames.WindowsCredentialManager}' credential store on Windows." + - Environment.NewLine + - $"See {Constants.HelpUrls.GcmCredentialStores} for more information." - ); + var format = "Can only use the '{0}' credential store on Windows.{1}" + + "See {3} for more information."; + var message = string.Format(format, + StoreNames.WindowsCredentialManager, + Environment.NewLine, + Constants.HelpUrls.GcmCredentialStores); + _context.Trace2.WriteError(message, format); + throw new Exception(message); } if (!WindowsCredentialManager.CanPersist()) { - throw new Exception( - $"Unable to persist credentials with the '{StoreNames.WindowsCredentialManager}' credential store." + - Environment.NewLine + - $"See {Constants.HelpUrls.GcmCredentialStores} for more information." - ); + var format = "Unable to persist credentials with the '{0}' credential store.{1}" + + "See {3} for more information."; + var message = string.Format(format, + StoreNames.WindowsCredentialManager, + Environment.NewLine, + Constants.HelpUrls.GcmCredentialStores); + _context.Trace2.WriteError(message, format); + throw new Exception(message); } } @@ -187,11 +196,14 @@ private void ValidateDpapi(out string storeRoot) { if (!PlatformUtils.IsWindows()) { - throw new Exception( - $"Can only use the '{StoreNames.Dpapi}' credential store on Windows." + - Environment.NewLine + - $"See {Constants.HelpUrls.GcmCredentialStores} for more information." - ); + var format = "Can only use the '{0}' credential store on Windows.{1}" + + "See {2} for more information."; + var message = string.Format(format, + StoreNames.Dpapi, + Environment.NewLine, + Constants.HelpUrls.GcmCredentialStores); + _context.Trace2.WriteError(message, format); + throw new Exception(message); } // Check for a redirected credential store location @@ -210,11 +222,14 @@ private void ValidateMacOSKeychain() { if (!PlatformUtils.IsMacOS()) { - throw new Exception( - $"Can only use the '{StoreNames.MacOSKeychain}' credential store on macOS." + - Environment.NewLine + - $"See {Constants.HelpUrls.GcmCredentialStores} for more information." - ); + var format = "Can only use the '{0}' credential store on macOS.{1}" + + "See {2} for more information."; + var message = string.Format(format, + StoreNames.MacOSKeychain, + Environment.NewLine, + Constants.HelpUrls.GcmCredentialStores); + _context.Trace2.WriteError(message, format); + throw new Exception(message); } } @@ -222,20 +237,26 @@ private void ValidateSecretService() { if (!PlatformUtils.IsLinux()) { - throw new Exception( - $"Can only use the '{StoreNames.SecretService}' credential store on Linux." + - Environment.NewLine + - $"See {Constants.HelpUrls.GcmCredentialStores} for more information." - ); + var format = "Can only use the '{0}' credential store on Linux.{1}" + + "See {2} for more information."; + var message = string.Format(format, + StoreNames.SecretService, + Environment.NewLine, + Constants.HelpUrls.GcmCredentialStores); + _context.Trace2.WriteError(message, format); + throw new Exception(message); } if (!_context.SessionManager.IsDesktopSession) { - throw new Exception( - $"Cannot use the '{StoreNames.SecretService}' credential backing store without a graphical interface present." + - Environment.NewLine + - $"See {Constants.HelpUrls.GcmCredentialStores} for more information." - ); + var format = "Cannot use the '{0}' credential backing store without a graphical interface present." + + "See {2} for more information."; + var message = string.Format(format, + StoreNames.SecretService, + Environment.NewLine, + Constants.HelpUrls.GcmCredentialStores); + _context.Trace2.WriteError(message, format); + throw new Exception(message); } } @@ -243,11 +264,14 @@ private void ValidateGpgPass(out string storeRoot, out string execPath) { if (!PlatformUtils.IsPosix()) { - throw new Exception( - $"Can only use the '{StoreNames.Gpg}' credential store on POSIX systems." + - Environment.NewLine + - $"See {Constants.HelpUrls.GcmCredentialStores} for more information." - ); + var format = "Can only use the '{0}' credential store on POSIX systems.{1}" + + "See {2} for more information."; + var message = string.Format(format, + StoreNames.Gpg, + Environment.NewLine, + Constants.HelpUrls.GcmCredentialStores); + _context.Trace2.WriteError(message, format); + throw new Exception(message); } execPath = GetGpgPath(); @@ -258,11 +282,13 @@ private void ValidateGpgPass(out string storeRoot, out string execPath) !_context.Environment.Variables.ContainsKey("GPG_TTY") && !_context.Environment.Variables.ContainsKey("SSH_TTY")) { - throw new Exception( - "GPG_TTY is not set; add `export GPG_TTY=$(tty)` to your profile." + - Environment.NewLine + - $"See {Constants.HelpUrls.GcmCredentialStores} for more information." - ); + var format = "GPG_TTY is not set; add `export GPG_TTY=$(tty)` to your profile.{0}" + + "See {1} for more information."; + var message = string.Format(format, + Environment.NewLine, + Constants.HelpUrls.GcmCredentialStores); + _context.Trace2.WriteError(message, format); + throw new Exception(message); } // Check for a redirected pass store location @@ -279,11 +305,14 @@ private void ValidateGpgPass(out string storeRoot, out string execPath) string gpgIdFile = Path.Combine(storeRoot, ".gpg-id"); if (!_context.FileSystem.FileExists(gpgIdFile)) { - throw new Exception( - $"Password store has not been initialized at '{storeRoot}'; run `pass init ` to initialize the store." + - Environment.NewLine + - $"See {Constants.HelpUrls.GcmCredentialStores} for more information." - ); + var format = "Password store has not been initialized at '{0}'; run `pass init ` to initialize the store.{1}" + + "See {2} for more information."; + var message = string.Format(format, + storeRoot, + Environment.NewLine, + Constants.HelpUrls.GcmCredentialStores); + _context.Trace2.WriteError(message, format); + throw new Exception(message); } } @@ -291,11 +320,15 @@ private void ValidateCredentialCache(out string options) { if (PlatformUtils.IsWindows()) { - throw new Exception( - $"Can not use the '{StoreNames.Cache}' credential store on Windows due to lack of UNIX socket support in Git for Windows." + - Environment.NewLine + - $"See {Constants.HelpUrls.GcmCredentialStores} for more information." - ); + var format = + "Cannot use the '{0}' credential store on Windows due to lack of UNIX socket support in Git for Windows.{1}" + + $"See {Constants.HelpUrls.GcmCredentialStores} for more information."; + var message = string.Format(format, + StoreNames.Cache, + Environment.NewLine, + Constants.HelpUrls.GcmCredentialStores); + _context.Trace2.WriteError(message, format); + throw new Exception(message); } // allow for --timeout and other options @@ -337,7 +370,10 @@ private string GetGpgPath() return gpgPath; } - throw new Exception($"GPG executable does not exist with path '{gpgPath}'"); + var format = "GPG executable does not exist with path '{0}'"; + var message = string.Format(format, gpgPath); + _context.Trace2.WriteError(message, format); + throw new Exception(message); } // If no explicit GPG path is specified, mimic the way `pass` diff --git a/src/shared/Core/GenericHostProvider.cs b/src/shared/Core/GenericHostProvider.cs index c85fc6b0f0..a98bc174e0 100644 --- a/src/shared/Core/GenericHostProvider.cs +++ b/src/shared/Core/GenericHostProvider.cs @@ -194,7 +194,9 @@ private async Task GetOAuthAccessToken(Uri remoteUri, string userNa break; default: - throw new Exception("No authentication mode selected!"); + var message = "No authentication mode selected!"; + Context.Trace2.WriteError(message); + throw new Exception(message); } // Store the refresh token if we have one diff --git a/src/shared/Core/Git.cs b/src/shared/Core/Git.cs index da283bec77..ec4b17e84d 100644 --- a/src/shared/Core/Git.cs +++ b/src/shared/Core/Git.cs @@ -134,7 +134,12 @@ public string GetCurrentRepository() case 128: // Not inside a Git repository return null; default: - _trace.WriteLine($"Failed to get current Git repository (exit={git.ExitCode})"); + var format = "Failed to get current Git repository (exit={0})"; + var message = string.Format(format, git.ExitCode); + + _trace.WriteLine(message); + _trace2.WriteError(message, format); + throw CreateGitException(git, "Failed to get current Git repository"); } } @@ -158,7 +163,12 @@ public IEnumerable GetRemotes() case 128 when stderr.Contains("not a git repository"): // Not inside a Git repository yield break; default: - _trace.WriteLine($"Failed to enumerate Git remotes (exit={git.ExitCode})"); + var format = "Failed to enumerate Git remotes (exit={0})"; + var message = string.Format(format, git.ExitCode); + + _trace.WriteLine(message); + _trace2.WriteError(message, format); + throw CreateGitException(git, "Failed to enumerate Git remotes"); } @@ -210,7 +220,11 @@ public ChildProcess CreateProcess(string args) var process = _processManager.CreateProcess(procStartInfo); if (process is null) { - throw new Exception($"Failed to start Git helper '{args}'"); + var format = "Failed to start Git helper '{0}'"; + var message = string.Format(format, args); + + _trace2.WriteError(message, format); + throw new Exception(message); } if (!(standardInput is null)) @@ -233,7 +247,11 @@ public ChildProcess CreateProcess(string args) errorMessage = "Unknown"; } - throw new Exception($"helper error ({exitCode}): {errorMessage}"); + var format = "helper error ({0}): {1}"; + var message = string.Format(format, exitCode, errorMessage); + + _trace2.WriteError(message, format); + throw new Exception(message); } return resultDict; diff --git a/src/shared/Core/Gpg.cs b/src/shared/Core/Gpg.cs index 70f2d3b550..ea3f433b56 100644 --- a/src/shared/Core/Gpg.cs +++ b/src/shared/Core/Gpg.cs @@ -46,7 +46,9 @@ public string DecryptFile(string path) { if (gpg is null) { - throw new Exception("Failed to start gpg."); + var message = "Failed to start gpg."; + _trace2.WriteError(message); + throw new Exception(message); } gpg.WaitForExit(); @@ -55,7 +57,10 @@ public string DecryptFile(string path) { string stdout = gpg.StandardOutput.ReadToEnd(); string stderr = gpg.StandardError.ReadToEnd(); - throw new Exception($"Failed to decrypt file '{path}' with gpg. exit={gpg.ExitCode}, out={stdout}, err={stderr}"); + var format = "Failed to decrypt file '{0}' with gpg. exit={1}, out={2}, err={3}"; + var message = string.Format(format, path, gpg.ExitCode, stdout, stderr); + _trace2.WriteError(message, format); + throw new Exception(message); } return gpg.StandardOutput.ReadToEnd(); @@ -78,7 +83,9 @@ public void EncryptFile(string path, string gpgId, string contents) { if (gpg is null) { - throw new Exception("Failed to start gpg."); + var message = "Failed to start gpg."; + _trace2.WriteError(message); + throw new Exception(message); } gpg.StandardInput.Write(contents); @@ -90,7 +97,10 @@ public void EncryptFile(string path, string gpgId, string contents) { string stdout = gpg.StandardOutput.ReadToEnd(); string stderr = gpg.StandardError.ReadToEnd(); - throw new Exception($"Failed to encrypt file '{path}' with gpg. exit={gpg.ExitCode}, out={stdout}, err={stderr}"); + var format = "Failed to encrypt file '{0}' with gpg. exit={1}, out={2}, err={3}"; + var message = string.Format(format, path, gpg.ExitCode, stdout, stderr); + _trace2.WriteError(message, format); + throw new Exception(message); } } } diff --git a/src/shared/Core/HostProviderRegistry.cs b/src/shared/Core/HostProviderRegistry.cs index 237c32e541..87fc19ee62 100644 --- a/src/shared/Core/HostProviderRegistry.cs +++ b/src/shared/Core/HostProviderRegistry.cs @@ -66,16 +66,18 @@ public void Register(IHostProvider hostProvider, HostProviderPriority priority) if (StringComparer.OrdinalIgnoreCase.Equals(hostProvider.Id, Constants.ProviderIdAuto)) { - throw new ArgumentException( - $"A host provider cannot be registered with the ID '{Constants.ProviderIdAuto}'", - nameof(hostProvider)); + var format = "A host provider cannot be registered with the ID '{0}'"; + var message = string.Format(format, Constants.ProviderIdAuto); + _context.Trace2.WriteError(message, format); + throw new ArgumentException(message, nameof(hostProvider)); } if (hostProvider.SupportedAuthorityIds.Any(y => StringComparer.OrdinalIgnoreCase.Equals(y, Constants.AuthorityIdAuto))) { - throw new ArgumentException( - $"A host provider cannot be registered with the legacy authority ID '{Constants.AuthorityIdAuto}'", - nameof(hostProvider)); + var format = "A host provider cannot be registered with the legacy authority ID '{0}'"; + var message = string.Format(format, Constants.AuthorityIdAuto); + _context.Trace2.WriteError(message, format); + throw new ArgumentException(message); } if (!_hostProviders.TryGetValue(priority, out ICollection providers)) @@ -151,7 +153,9 @@ public async Task GetProviderAsync(InputArguments input) var uri = input.GetRemoteUri(); if (uri is null) { - throw new Exception("Unable to detect host provider without a remote URL"); + var message = "Unable to detect host provider without a remote URL"; + _context.Trace2.WriteError(message); + throw new Exception(message); } // We can only probe HTTP(S) URLs - for SMTP, IMAP, etc we cannot do network probing @@ -242,6 +246,7 @@ async Task MatchProviderAsync(HostProviderPriority priority, bool { _context.Trace.WriteLine("Failed to set host provider!"); _context.Trace.WriteException(ex); + _context.Trace2.WriteError(ex.Message); _context.Streams.Error.WriteLine("warning: failed to remember result of host provider detection!"); _context.Streams.Error.WriteLine($"warning: try setting this manually: `git config --global {keyName} {match.Id}`"); diff --git a/src/shared/Core/HttpClientFactory.cs b/src/shared/Core/HttpClientFactory.cs index a3ea8ab6f0..0c7c12f320 100644 --- a/src/shared/Core/HttpClientFactory.cs +++ b/src/shared/Core/HttpClientFactory.cs @@ -115,7 +115,10 @@ public HttpClient CreateClient() // Throw exception if cert bundle file not found if (!_fileSystem.FileExists(certBundlePath)) { - throw new FileNotFoundException($"Custom certificate bundle not found at path: {certBundlePath}", certBundlePath); + var format = "Custom certificate bundle not found at path: {0}"; + var message = string.Format(format, certBundlePath); + _trace2.WriteError(message, format); + throw new FileNotFoundException(message, certBundlePath); } Func validationCallback = (cert, chain, errors) => @@ -282,6 +285,7 @@ public bool TryCreateProxy(out IWebProxy proxy) { _trace.WriteLine("Failed to convert proxy bypass hosts to regular expressions; ignoring bypass list"); _trace.WriteException(ex); + _trace2.WriteError(ex.Message); dict["bypass"] = "<< failed to convert >>"; } } diff --git a/src/shared/Core/Interop/Linux/LinuxTerminal.cs b/src/shared/Core/Interop/Linux/LinuxTerminal.cs index 2d483556ff..fc19894d6c 100644 --- a/src/shared/Core/Interop/Linux/LinuxTerminal.cs +++ b/src/shared/Core/Interop/Linux/LinuxTerminal.cs @@ -36,7 +36,9 @@ public TtyContext(ITrace trace, ITrace2 trace2, int fd, bool echo) // Capture current terminal settings so we can restore them later if ((error = Termios_Linux.tcgetattr(_fd, out termios_Linux t)) != 0) { - throw new InteropException("Failed to get initial terminal settings", error); + var message = "Failed to get initial terminal settings"; + trace2.WriteError(message); + throw new InteropException(message, error); } _originalTerm = t; @@ -50,7 +52,9 @@ public TtyContext(ITrace trace, ITrace2 trace2, int fd, bool echo) if ((error = Termios_Linux.tcsetattr(_fd, SetActionFlags.TCSAFLUSH, ref t)) != 0) { - throw new InteropException("Failed to set terminal settings", error); + var message = "Failed to set terminal settings"; + trace2.WriteError(message); + throw new InteropException(message, error); } } diff --git a/src/shared/Core/Interop/MacOS/MacOSTerminal.cs b/src/shared/Core/Interop/MacOS/MacOSTerminal.cs index 0cc339ac22..16868b16ba 100644 --- a/src/shared/Core/Interop/MacOS/MacOSTerminal.cs +++ b/src/shared/Core/Interop/MacOS/MacOSTerminal.cs @@ -36,7 +36,9 @@ public TtyContext(ITrace trace, ITrace2 trace2, int fd, bool echo) // Capture current terminal settings so we can restore them later if ((error = Termios_MacOS.tcgetattr(_fd, out termios_MacOS t)) != 0) { - throw new InteropException("Failed to get initial terminal settings", error); + var message = "Failed to get initial terminal settings"; + trace2.WriteError(message); + throw new InteropException(message, error); } _originalTerm = t; @@ -50,7 +52,9 @@ public TtyContext(ITrace trace, ITrace2 trace2, int fd, bool echo) if ((error = Termios_MacOS.tcsetattr(_fd, SetActionFlags.TCSAFLUSH, ref t)) != 0) { - throw new InteropException("Failed to set terminal settings", error); + var message = "Failed to set terminal settings"; + trace2.WriteError(message); + throw new InteropException(message, error); } } diff --git a/src/shared/GitHub.UI/Commands/CredentialsCommand.cs b/src/shared/GitHub.UI/Commands/CredentialsCommand.cs index 0f168c020c..44f86d1ba4 100644 --- a/src/shared/GitHub.UI/Commands/CredentialsCommand.cs +++ b/src/shared/GitHub.UI/Commands/CredentialsCommand.cs @@ -81,7 +81,9 @@ private async Task ExecuteAsync(CommandOptions options) if (!viewModel.WindowResult) { - throw new Exception("User cancelled dialog."); + var message = "User cancelled dialog."; + Context.Trace2.WriteError(message); + throw new Exception(message); } var result = new Dictionary(); @@ -108,7 +110,9 @@ private async Task ExecuteAsync(CommandOptions options) break; default: - throw new ArgumentOutOfRangeException(); + var ex = new ArgumentOutOfRangeException(); + Context.Trace2.WriteError(ex.Message); + throw ex; } WriteResult(result); diff --git a/src/shared/GitHub.UI/Commands/DeviceCommand.cs b/src/shared/GitHub.UI/Commands/DeviceCommand.cs index 6004055fe0..707cc86f50 100644 --- a/src/shared/GitHub.UI/Commands/DeviceCommand.cs +++ b/src/shared/GitHub.UI/Commands/DeviceCommand.cs @@ -38,6 +38,8 @@ private async Task ExecuteAsync(string code, string url) if (!viewModel.WindowResult) { + var message = "User cancelled dialog."; + Context.Trace2.WriteError(message); throw new Exception("User cancelled dialog."); } diff --git a/src/shared/GitHub.UI/Commands/TwoFactorCommand.cs b/src/shared/GitHub.UI/Commands/TwoFactorCommand.cs index 192116725f..aa975b07b4 100644 --- a/src/shared/GitHub.UI/Commands/TwoFactorCommand.cs +++ b/src/shared/GitHub.UI/Commands/TwoFactorCommand.cs @@ -33,6 +33,8 @@ private async Task ExecuteAsync(bool sms) if (!viewModel.WindowResult) { + var message = "User cancelled dialog."; + Context.Trace2.WriteError(message); throw new Exception("User cancelled dialog."); } diff --git a/src/shared/GitHub/GitHubAuthentication.cs b/src/shared/GitHub/GitHubAuthentication.cs index 55270a8c5c..352d84e5a4 100644 --- a/src/shared/GitHub/GitHubAuthentication.cs +++ b/src/shared/GitHub/GitHubAuthentication.cs @@ -70,10 +70,15 @@ public async Task GetAuthenticationAsync(Uri targetU modes = modes & ~AuthenticationModes.Browser; } + string message; // We need at least one mode! if (modes == AuthenticationModes.None) { - throw new ArgumentException(@$"Must specify at least one {nameof(AuthenticationModes)}", nameof(modes)); + var format = @"Must specify at least one {0}"; + message = string.Format(format, nameof(OAuthAuthenticationModes)); + + Context.Trace2.WriteError(message, format); + throw new ArgumentException(message, nameof(modes)); } // If there is no mode choice to be made and no interaction required, @@ -109,7 +114,9 @@ public async Task GetAuthenticationAsync(Uri targetU if (!resultDict.TryGetValue("mode", out string responseMode)) { - throw new Exception("Missing 'mode' in response"); + message = "Missing 'mode' in response"; + Context.Trace2.WriteError(message); + throw new Exception(message); } switch (responseMode.ToLowerInvariant()) @@ -117,7 +124,9 @@ public async Task GetAuthenticationAsync(Uri targetU case "pat": if (!resultDict.TryGetValue("pat", out string pat)) { - throw new Exception("Missing 'pat' in response"); + message = "Missing 'pat' in response"; + Context.Trace2.WriteError(message); + throw new Exception(message); } return new AuthenticationPromptResult( @@ -132,19 +141,26 @@ public async Task GetAuthenticationAsync(Uri targetU case "basic": if (!resultDict.TryGetValue("username", out userName)) { - throw new Exception("Missing 'username' in response"); + message = "Missing 'username' in response"; + Context.Trace2.WriteError(message); + throw new Exception(message); } if (!resultDict.TryGetValue("password", out string password)) { - throw new Exception("Missing 'password' in response"); + message = "Missing 'password' in response"; + Context.Trace2.WriteError(message); + throw new Exception(message); } return new AuthenticationPromptResult( AuthenticationModes.Basic, new GitCredential(userName, password)); default: - throw new Exception($"Unknown mode value in response '{responseMode}'"); + var format = "Unknown mode value in response '{0}'"; + message = string.Format(format, responseMode); + Context.Trace2.WriteError(message); + throw new Exception(message); } } else @@ -183,7 +199,11 @@ public async Task GetAuthenticationAsync(Uri targetU AuthenticationModes.Pat, new GitCredential(userName, pat)); case AuthenticationModes.None: - throw new ArgumentOutOfRangeException(nameof(modes), @$"At least one {nameof(AuthenticationModes)} must be supplied"); + var format = "At least one {0} must be supplied"; + message = string.Format(format, nameof(AuthenticationModes)); + + Context.Trace2.WriteError(message, format); + throw new ArgumentOutOfRangeException(nameof(modes), message); default: var menuTitle = $"Select an authentication method for '{targetUri}'"; @@ -207,7 +227,9 @@ public async Task GetAuthenticationAsync(Uri targetU if (choice == basicItem) goto case AuthenticationModes.Basic; if (choice == patItem) goto case AuthenticationModes.Pat; - throw new Exception(); + var ex = new Exception(); + Context.Trace2.WriteError(ex.Message); + throw ex; } } } @@ -227,7 +249,9 @@ public async Task GetTwoFactorCodeAsync(Uri targetUri, bool isSms) if (!resultDict.TryGetValue("code", out string authCode)) { - throw new Exception("Missing 'code' in response"); + var message = "Missing 'code' in response"; + Context.Trace2.WriteError(message); + throw new Exception(message); } return authCode; @@ -260,7 +284,9 @@ public async Task GetOAuthTokenViaBrowserAsync(Uri targetUri, // Can we launch the user's default web browser? if (!Context.SessionManager.IsWebBrowserAvailable) { - throw new InvalidOperationException("Browser authentication requires a desktop session"); + var message = "Browser authentication requires a desktop session"; + Context.Trace2.WriteError(message); + throw new InvalidOperationException(message); } var browserOptions = new OAuth2WebBrowserOptions @@ -329,7 +355,9 @@ public async Task GetOAuthTokenViaDeviceCodeAsync(Uri targetU } catch (OperationCanceledException) { - throw new Exception("User canceled device code authentication"); + var message = "User canceled device code authentication"; + Context.Trace2.WriteError(message); + throw new InvalidOperationException(message); } // Close the dialog diff --git a/src/shared/GitHub/GitHubHostProvider.cs b/src/shared/GitHub/GitHubHostProvider.cs index 040a14dfc2..e78177c6de 100644 --- a/src/shared/GitHub/GitHubHostProvider.cs +++ b/src/shared/GitHub/GitHubHostProvider.cs @@ -123,7 +123,10 @@ public override async Task GenerateCredentialAsync(InputArguments i // We should not allow unencrypted communication and should inform the user if (StringComparer.OrdinalIgnoreCase.Equals(input.Protocol, "http")) { - throw new Exception("Unencrypted HTTP is not supported for GitHub. Ensure the repository remote URL is using HTTPS."); + var message = + "Unencrypted HTTP is not supported for GitHub. Ensure the repository remote URL is using HTTPS."; + Context.Trace2.WriteError(message); + throw new Exception(message); } Uri remoteUri = input.GetRemoteUri(); @@ -172,7 +175,9 @@ public override async Task GenerateCredentialAsync(InputArguments i return new GitCredential(userName, token); default: - throw new ArgumentOutOfRangeException(nameof(promptResult)); + var ex = new ArgumentOutOfRangeException(nameof(promptResult)); + Context.Trace2.WriteError(ex.Message); + throw ex; } } @@ -226,7 +231,10 @@ private async Task GeneratePersonalAccessTokenAsync(Uri targetUri return new GitCredential(userInfo.Login, token); } - throw new Exception($"Interactive logon for '{targetUri}' failed."); + var format = "Interactive logon for '{0}' failed."; + var message = string.Format(format, targetUri); + Context.Trace2.WriteError(message); + throw new Exception(message); } internal async Task GetSupportedAuthenticationModesAsync(Uri targetUri) @@ -287,6 +295,7 @@ internal async Task GetSupportedAuthenticationModesAsync(Ur { Context.Trace.WriteLine($"Failed to query '{targetUri}' for supported authentication schemes."); Context.Trace.WriteException(ex); + Context.Trace2.WriteError(ex.Message); Context.Terminal.WriteLine($"warning: failed to query '{targetUri}' for supported authentication schemes."); diff --git a/src/shared/GitLab.UI/Commands/CredentialsCommand.cs b/src/shared/GitLab.UI/Commands/CredentialsCommand.cs index d9fa6c4a57..92e7783eaa 100644 --- a/src/shared/GitLab.UI/Commands/CredentialsCommand.cs +++ b/src/shared/GitLab.UI/Commands/CredentialsCommand.cs @@ -76,7 +76,9 @@ private async Task ExecuteAsync(CommandOptions options) if (!viewModel.WindowResult) { - throw new Exception("User cancelled dialog."); + var message = "User cancelled dialog."; + Context.Trace2.WriteError(message); + throw new Exception(message); } var result = new Dictionary(); @@ -100,7 +102,9 @@ private async Task ExecuteAsync(CommandOptions options) break; default: - throw new ArgumentOutOfRangeException(); + var ex = new ArgumentOutOfRangeException(); + Context.Trace2.WriteError(ex.Message); + throw ex; } WriteResult(result); diff --git a/src/shared/GitLab/GitLabAuthentication.cs b/src/shared/GitLab/GitLabAuthentication.cs index 3f4b658fb1..57f2a02dcd 100644 --- a/src/shared/GitLab/GitLabAuthentication.cs +++ b/src/shared/GitLab/GitLabAuthentication.cs @@ -61,10 +61,15 @@ public async Task GetAuthenticationAsync(Uri targetU modes = modes & ~AuthenticationModes.Browser; } + string message; // We need at least one mode! if (modes == AuthenticationModes.None) { - throw new ArgumentException(@$"Must specify at least one {nameof(AuthenticationModes)}", nameof(modes)); + var format = @"Must specify at least one {0}"; + message = string.Format(format, nameof(OAuthAuthenticationModes)); + + Context.Trace2.WriteError(message, format); + throw new ArgumentException(message, nameof(modes)); } if (Context.Settings.IsGuiPromptsEnabled && Context.SessionManager.IsDesktopSession && @@ -87,7 +92,9 @@ public async Task GetAuthenticationAsync(Uri targetU if (!resultDict.TryGetValue("mode", out string responseMode)) { - throw new Exception("Missing 'mode' in response"); + message = "Missing 'mode' in response"; + Context.Trace2.WriteError(message); + throw new Exception(message); } switch (responseMode.ToLowerInvariant()) @@ -95,7 +102,9 @@ public async Task GetAuthenticationAsync(Uri targetU case "pat": if (!resultDict.TryGetValue("pat", out string pat)) { - throw new Exception("Missing 'pat' in response"); + message = "Missing 'pat' in response"; + Context.Trace2.WriteError(message); + throw new Exception(message); } if (!resultDict.TryGetValue("username", out string patUserName)) @@ -112,19 +121,27 @@ public async Task GetAuthenticationAsync(Uri targetU case "basic": if (!resultDict.TryGetValue("username", out userName)) { - throw new Exception("Missing 'username' in response"); + message = "Missing 'username' in response"; + Context.Trace2.WriteError(message); + throw new Exception(message); } if (!resultDict.TryGetValue("password", out string password)) { - throw new Exception("Missing 'password' in response"); + message = "Missing 'password' in response"; + Context.Trace2.WriteError(message); + throw new Exception(message); } return new AuthenticationPromptResult( AuthenticationModes.Basic, new GitCredential(userName, password)); default: - throw new Exception($"Unknown mode value in response '{responseMode}'"); + var format = "Unknown mode value in response '{0}'"; + message = string.Format(format, responseMode); + + Context.Trace2.WriteError(message, format); + throw new Exception(message); } } else @@ -169,7 +186,10 @@ public async Task GetAuthenticationAsync(Uri targetU return new AuthenticationPromptResult(AuthenticationModes.Browser); case AuthenticationModes.None: - throw new ArgumentOutOfRangeException(nameof(modes), @$"At least one {nameof(AuthenticationModes)} must be supplied"); + var format = @"At least one {0} must be supplied"; + message = string.Format(format, nameof(AuthenticationModes)); + Context.Trace2.WriteError(message, format); + throw new ArgumentOutOfRangeException(nameof(modes), message); default: ThrowIfUserInteractionDisabled(); @@ -192,7 +212,9 @@ public async Task GetAuthenticationAsync(Uri targetU if (choice == basicItem) goto case AuthenticationModes.Basic; if (choice == patItem) goto case AuthenticationModes.Pat; - throw new Exception(); + var ex = new Exception(); + Context.Trace2.WriteError(ex.Message); + throw ex; } } } @@ -206,7 +228,9 @@ public async Task GetOAuthTokenViaBrowserAsync(Uri targetUri, // We require a desktop session to launch the user's default web browser if (!Context.SessionManager.IsDesktopSession) { - throw new InvalidOperationException("Browser authentication requires a desktop session"); + var message = "Browser authentication requires a desktop session"; + Context.Trace2.WriteError(message); + throw new InvalidOperationException(message); } var browserOptions = new OAuth2WebBrowserOptions { }; diff --git a/src/shared/GitLab/GitLabHostProvider.cs b/src/shared/GitLab/GitLabHostProvider.cs index 2836f41c63..805264f581 100644 --- a/src/shared/GitLab/GitLabHostProvider.cs +++ b/src/shared/GitLab/GitLabHostProvider.cs @@ -91,7 +91,10 @@ public override async Task GenerateCredentialAsync(InputArguments i // We should not allow unencrypted communication and should inform the user if (StringComparer.OrdinalIgnoreCase.Equals(input.Protocol, "http")) { - throw new Exception("Unencrypted HTTP is not supported for GitLab. Ensure the repository remote URL is using HTTPS."); + var message = + "Unencrypted HTTP is not supported for GitHub. Ensure the repository remote URL is using HTTPS."; + Context.Trace2.WriteError(message); + throw new Exception(message); } Uri remoteUri = input.GetRemoteUri(); @@ -110,7 +113,9 @@ public override async Task GenerateCredentialAsync(InputArguments i return await GenerateOAuthCredentialAsync(input); default: - throw new ArgumentOutOfRangeException(nameof(promptResult)); + var ex = new ArgumentOutOfRangeException(nameof(promptResult)); + Context.Trace2.WriteError(ex.Message); + throw ex; } } diff --git a/src/shared/Microsoft.AzureRepos/AzureDevOpsRestApi.cs b/src/shared/Microsoft.AzureRepos/AzureDevOpsRestApi.cs index d4b5743248..d0da1e383d 100644 --- a/src/shared/Microsoft.AzureRepos/AzureDevOpsRestApi.cs +++ b/src/shared/Microsoft.AzureRepos/AzureDevOpsRestApi.cs @@ -106,11 +106,15 @@ private Uri GetAuthorityBaseUri() public async Task CreatePersonalAccessTokenAsync(Uri organizationUri, string accessToken, IEnumerable scopes) { const string sessionTokenUrl = "_apis/token/sessiontokens?api-version=1.0&tokentype=compact"; + string message; EnsureArgument.AbsoluteUri(organizationUri, nameof(organizationUri)); if (!UriHelpers.IsAzureDevOpsHost(organizationUri.Host)) { - throw new ArgumentException($"Provided URI '{organizationUri}' is not a valid Azure DevOps hostname", nameof(organizationUri)); + var format = "Provided URI '{0}' is not a valid Azure DevOps hostname"; + message = string.Format(format, nameof(organizationUri)); + _context.Trace2.WriteError(message, format); + throw new ArgumentException(message); } EnsureArgument.NotNull(accessToken, nameof(accessToken)); @@ -142,13 +146,18 @@ public async Task CreatePersonalAccessTokenAsync(Uri organizationUri, st { if (TryGetFirstJsonStringField(responseText, "message", out string errorMessage)) { - throw new Exception($"Failed to create PAT: {errorMessage}"); + var format = "Failed to create PAT: {0}"; + message = string.Format(format, errorMessage); + _context.Trace2.WriteError(message, format); + throw new Exception(message); } } } } - throw new Exception("Failed to create PAT"); + message = "Failed to create PAT"; + _context.Trace2.WriteError(message); + throw new Exception(message); } #region Private Methods @@ -181,7 +190,9 @@ private async Task GetIdentityServiceUriAsync(Uri organizationUri, string a } } - throw new Exception("Failed to find location service"); + var message = "Failed to find location service"; + _context.Trace2.WriteError(message); + throw new Exception(message); } #endregion diff --git a/src/shared/Microsoft.AzureRepos/AzureReposHostProvider.cs b/src/shared/Microsoft.AzureRepos/AzureReposHostProvider.cs index 3dbc56bf65..80cd9d13a8 100644 --- a/src/shared/Microsoft.AzureRepos/AzureReposHostProvider.cs +++ b/src/shared/Microsoft.AzureRepos/AzureReposHostProvider.cs @@ -184,7 +184,9 @@ private async Task GeneratePersonalAccessTokenAsync(InputArguments // We should not allow unencrypted communication and should inform the user if (StringComparer.OrdinalIgnoreCase.Equals(input.Protocol, "http")) { - throw new Exception("Unencrypted HTTP is not supported for Azure Repos. Ensure the repository remote URL is using HTTPS."); + var message = "Unencrypted HTTP is not supported for Azure Repos. Ensure the repository remote URL is using HTTPS."; + _context.Trace2.WriteError(message); + throw new Exception(message); } Uri remoteUri = input.GetRemoteUri(); @@ -227,7 +229,9 @@ private async Task GetAzureAccessTokenAsync(Uri // We should not allow unencrypted communication and should inform the user if (StringComparer.OrdinalIgnoreCase.Equals(remoteUri.Scheme, "http")) { - throw new Exception("Unencrypted HTTP is not supported for Azure Repos. Ensure the repository remote URL is using HTTPS."); + var message = "Unencrypted HTTP is not supported for Azure Repos. Ensure the repository remote URL is using HTTPS."; + _context.Trace2.WriteError(message); + throw new Exception(message); } Uri orgUri = UriHelpers.CreateOrganizationUri(remoteUri, out string orgName);