Skip to content

Commit 19d031d

Browse files
ldenningtonldennington
committed
trace2: add error tracing statements
Add TRACE2 error tracing statements throughout GCM codebase. Note that this is a best effort - there are certain places (most notably certain static and unsafe methods) where statements were purposefully not added because it is either not safe or was deemed to much lift/churn to do so. Note that there are some instances in which a "parameterized" message is passed to TRACE2. This is used only when PII information could be passed into the message (e.g. through a path or remote URI) and should be redacted for collection purposes. Finally, there are certain instances in which we write an error with no exception thrown. These align with the places where we are currently writing to GCM Trace without throwing an error for the purposes of parity.
1 parent 6e17d99 commit 19d031d

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

41 files changed

+226
-144
lines changed

‎src/shared/Atlassian.Bitbucket.Tests/Cloud/BitbucketOAuth2ClientTest.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ public async Task BitbucketOAuth2Client_GetDeviceCodeAsync()
7070
{
7171
var trace2 = new NullTrace2();
7272
var client = new Bitbucket.Cloud.BitbucketOAuth2Client(httpClient.Object, settings.Object, trace2);
73-
await Assert.ThrowsAsync<InvalidOperationException>(async () => await client.GetDeviceCodeAsync(scopes, ct));
73+
await Assert.ThrowsAsync<Trace2InvalidOperationException>(async () => await client.GetDeviceCodeAsync(scopes, ct));
7474
}
7575

7676
[Theory]
@@ -151,4 +151,4 @@ private string MockClientIdOverride(bool set, string value)
151151
return value;
152152
}
153153
}
154-
}
154+
}

‎src/shared/Atlassian.Bitbucket.UI/Commands/CredentialsCommand.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ private async Task<int> ExecuteAsync(Uri url, string userName, bool showOAuth, b
4848

4949
if (!viewModel.WindowResult || viewModel.SelectedMode == AuthenticationModes.None)
5050
{
51-
throw new Exception("User cancelled dialog.");
51+
throw new Trace2Exception(Context.Trace2, "User cancelled dialog.");
5252
}
5353

5454
switch (viewModel.SelectedMode)

‎src/shared/Atlassian.Bitbucket/BitbucketAuthentication.cs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
using System.Text;
55
using System.Threading;
66
using System.Threading.Tasks;
7-
using Atlassian.Bitbucket.Cloud;
87
using GitCredentialManager;
98
using GitCredentialManager.Authentication;
109
using GitCredentialManager.Authentication.OAuth;
@@ -88,7 +87,7 @@ public async Task<CredentialsPromptResult> GetCredentialsAsync(Uri targetUri, st
8887
// We need at least one mode!
8988
if (modes == AuthenticationModes.None)
9089
{
91-
throw new ArgumentException(@$"Must specify at least one {nameof(AuthenticationModes)}", nameof(modes));
90+
throw new ArgumentOutOfRangeException(nameof(modes), @$"At least one {nameof(AuthenticationModes)} must be supplied");
9291
}
9392

9493
// Shell out to the UI helper and show the Bitbucket u/p prompt
@@ -128,12 +127,12 @@ public async Task<CredentialsPromptResult> GetCredentialsAsync(Uri targetUri, st
128127
{
129128
if (!output.TryGetValue("username", out userName))
130129
{
131-
throw new Exception("Missing username in response");
130+
throw new Trace2Exception(Context.Trace2, "Missing username in response");
132131
}
133132

134133
if (!output.TryGetValue("password", out password))
135134
{
136-
throw new Exception("Missing password in response");
135+
throw new Trace2Exception(Context.Trace2, "Missing password in response");
137136
}
138137

139138
return new CredentialsPromptResult(

‎src/shared/Atlassian.Bitbucket/BitbucketHostProvider.cs

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
using System;
22
using System.Collections.Generic;
3-
using System.Net;
43
using System.Net.Http;
54
using System.Threading.Tasks;
65
using Atlassian.Bitbucket.Cloud;
@@ -79,7 +78,8 @@ public async Task<ICredential> GetCredentialAsync(InputArguments input)
7978
if (StringComparer.OrdinalIgnoreCase.Equals(input.Protocol, "http")
8079
&& BitbucketHelper.IsBitbucketOrg(input))
8180
{
82-
throw new Exception("Unencrypted HTTP is not supported for Bitbucket.org. Ensure the repository remote URL is using HTTPS.");
81+
throw new Trace2Exception(_context.Trace2,
82+
"Unencrypted HTTP is not supported for Bitbucket.org. Ensure the repository remote URL is using HTTPS.");
8383
}
8484

8585
var authModes = await GetSupportedAuthenticationModesAsync(input);
@@ -145,8 +145,9 @@ private async Task<ICredential> GetRefreshedCredentials(InputArguments input, Au
145145
var result = await _bitbucketAuth.GetCredentialsAsync(remoteUri, input.UserName, authModes);
146146
if (result is null || result.AuthenticationMode == AuthenticationModes.None)
147147
{
148-
_context.Trace.WriteLine("User cancelled credential prompt");
149-
throw new Exception("User cancelled credential prompt.");
148+
var message = "User cancelled credential prompt";
149+
_context.Trace.WriteLine(message);
150+
throw new Trace2Exception(_context.Trace2, message);
150151
}
151152

152153
switch (result.AuthenticationMode)
@@ -176,8 +177,10 @@ private async Task<ICredential> GetRefreshedCredentials(InputArguments input, Au
176177
}
177178
catch (OAuth2Exception ex)
178179
{
179-
_context.Trace.WriteLine("Failed to refresh existing OAuth credential using refresh token");
180+
var message = "Failed to refresh existing OAuth credential using refresh token";
181+
_context.Trace.WriteLine(message);
180182
_context.Trace.WriteException(ex);
183+
_context.Trace2.WriteError(message);
181184

182185
// We failed to refresh the AT using the RT; log the refresh failure and fall through to restart
183186
// the OAuth authentication flow
@@ -279,7 +282,7 @@ public async Task<AuthenticationModes> GetSupportedAuthenticationModesAsync(Inpu
279282
try
280283
{
281284
var authenticationMethods = await _restApiRegistry.Get(input).GetAuthenticationMethodsAsync();
282-
285+
283286
var modes = AuthenticationModes.None;
284287

285288
if (authenticationMethods.Contains(AuthenticationMethod.BasicAuth))
@@ -298,10 +301,14 @@ public async Task<AuthenticationModes> GetSupportedAuthenticationModesAsync(Inpu
298301
}
299302
catch (Exception ex)
300303
{
301-
_context.Trace.WriteLine($"Failed to query '{input.GetRemoteUri()}' for supported authentication schemes.");
304+
var format = "Failed to query '{0}' for supported authentication schemes.";
305+
var message = string.Format(format, input.GetRemoteUri());
306+
307+
_context.Trace.WriteLine(message);
302308
_context.Trace.WriteException(ex);
309+
_context.Trace2.WriteError(message, format);
303310

304-
_context.Terminal.WriteLine($"warning: failed to query '{input.GetRemoteUri()}' for supported authentication schemes.");
311+
_context.Terminal.WriteLine($"warning: {message}");
305312

306313
// Fall-back to offering all modes so the user is never blocked from authenticating by at least one mode
307314
return AuthenticationModes.All;
@@ -356,7 +363,8 @@ private async Task<string> ResolveOAuthUserNameAsync(InputArguments input, strin
356363
return result.Response.UserName;
357364
}
358365

359-
throw new Exception($"Failed to resolve username. HTTP: {result.StatusCode}");
366+
throw new Trace2Exception(_context.Trace2,
367+
$"Failed to resolve username. HTTP: {result.StatusCode}");
360368
}
361369

362370
private async Task<string> ResolveBasicAuthUserNameAsync(InputArguments input, string username, string password)
@@ -367,7 +375,8 @@ private async Task<string> ResolveBasicAuthUserNameAsync(InputArguments input, s
367375
return result.Response.UserName;
368376
}
369377

370-
throw new Exception($"Failed to resolve username. HTTP: {result.StatusCode}");
378+
throw new Trace2Exception(_context.Trace2,
379+
$"Failed to resolve username. HTTP: {result.StatusCode}");
371380
}
372381

373382
private async Task<bool> ValidateCredentialsWork(InputArguments input, ICredential credentials, AuthenticationModes authModes)
@@ -404,8 +413,10 @@ private async Task<bool> ValidateCredentialsWork(InputArguments input, ICredenti
404413
}
405414
catch (Exception ex)
406415
{
407-
_context.Trace.WriteLine($"Failed to validate existing credentials using OAuth");
416+
var message = "Failed to validate existing credentials using OAuth";
417+
_context.Trace.WriteLine(message);
408418
_context.Trace.WriteException(ex);
419+
_context.Trace2.WriteError(message);
409420
}
410421
}
411422

@@ -419,8 +430,10 @@ private async Task<bool> ValidateCredentialsWork(InputArguments input, ICredenti
419430
}
420431
catch (Exception ex)
421432
{
422-
_context.Trace.WriteLine($"Failed to validate existing credentials using Basic Auth");
433+
var message = "Failed to validate existing credentials using Basic Auth";
434+
_context.Trace.WriteLine(message);
423435
_context.Trace.WriteException(ex);
436+
_context.Trace2.WriteError(message);
424437
return false;
425438
}
426439
}

‎src/shared/Atlassian.Bitbucket/DataCenter/BitbucketRestApi.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ public BitbucketRestApi(ICommandContext context)
2020
EnsureArgument.NotNull(context, nameof(context));
2121

2222
_context = context;
23-
23+
2424
}
2525

2626
public async Task<RestApiResult<IUserInfo>> GetUserInformationAsync(string userName, string password, bool isBearerToken)
@@ -35,7 +35,7 @@ public async Task<RestApiResult<IUserInfo>> GetUserInformationAsync(string userN
3535
}
3636

3737
// Bitbucket Server/DC doesn't actually provide a REST API we can use to trade an access_token for the owning username,
38-
// therefore this is always going to return a placeholder username, however this call does provide a way to validate the
38+
// therefore this is always going to return a placeholder username, however this call does provide a way to validate the
3939
// credentials we do have
4040
var requestUri = new Uri(ApiUri, "api/1.0/users");
4141
using (HttpRequestMessage request = new HttpRequestMessage(HttpMethod.Get, requestUri))
@@ -131,9 +131,9 @@ public void Dispose()
131131

132132
private HttpClient HttpClient => _httpClient ??= _context.HttpClientFactory.CreateClient();
133133

134-
private Uri ApiUri
134+
private Uri ApiUri
135135
{
136-
get
136+
get
137137
{
138138
var remoteUri = _context.Settings?.RemoteUri;
139139
if (remoteUri == null)

‎src/shared/Core.Tests/Authentication/MicrosoftAuthenticationTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ public async System.Threading.Tasks.Task MicrosoftAuthentication_GetAccessTokenA
2323

2424
var msAuth = new MicrosoftAuthentication(context);
2525

26-
await Assert.ThrowsAsync<InvalidOperationException>(
26+
await Assert.ThrowsAsync<Trace2InvalidOperationException>(
2727
() => msAuth.GetTokenAsync(authority, clientId, redirectUri, scopes, userName));
2828
}
2929
}

‎src/shared/Core.UI/Commands/CredentialsCommand.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ private async Task<int> ExecuteAsync(CommandOptions options)
6363

6464
if (!viewModel.WindowResult)
6565
{
66-
throw new Exception("User cancelled dialog.");
66+
throw new Trace2Exception(Context.Trace2, "User cancelled dialog.");
6767
}
6868

6969
WriteResult(

‎src/shared/Core.UI/Commands/DeviceCodeCommand.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ private async Task<int> ExecuteAsync(string code, string url, bool noLogo)
4141

4242
if (!viewModel.WindowResult)
4343
{
44-
throw new Exception("User cancelled dialog.");
44+
throw new Trace2Exception(Context.Trace2, "User cancelled dialog.");
4545
}
4646

4747
return 0;

‎src/shared/Core.UI/Commands/OAuthCommand.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ private async Task<int> ExecuteAsync(CommandOptions options)
6666

6767
if (!viewModel.WindowResult)
6868
{
69-
throw new Exception("User cancelled dialog.");
69+
throw new Trace2Exception(Context.Trace2, "User cancelled dialog.");
7070
}
7171

7272
var result = new Dictionary<string, string>();

‎src/shared/Core/Authentication/AuthenticationBase.cs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,10 @@ protected internal virtual async Task<IDictionary<string, string>> InvokeHelperA
4646
var process = ChildProcess.Start(Context.Trace2, procStartInfo, Trace2ProcessClass.UIHelper);
4747
if (process is null)
4848
{
49-
throw new Exception($"Failed to start helper process: {path} {args}");
49+
var format = "Failed to start helper process: {0} {1}";
50+
var message = string.Format(format, path, args);
51+
52+
throw new Trace2Exception(Context.Trace2, message, format);
5053
}
5154

5255
// Kill the process upon a cancellation request
@@ -69,7 +72,7 @@ protected internal virtual async Task<IDictionary<string, string>> InvokeHelperA
6972
errorMessage = "Unknown";
7073
}
7174

72-
throw new Exception($"helper error ({exitCode}): {errorMessage}");
75+
throw new Trace2Exception(Context.Trace2, $"helper error ({exitCode}): {errorMessage}");
7376
}
7477

7578
return resultDict;
@@ -85,8 +88,7 @@ protected void ThrowIfUserInteractionDisabled()
8588
Constants.GitConfiguration.Credential.Interactive);
8689

8790
Context.Trace.WriteLine($"{envName} / {cfgName} is false/never; user interactivity has been disabled.");
88-
89-
throw new InvalidOperationException("Cannot prompt because user interactivity has been disabled.");
91+
throw new Trace2InvalidOperationException(Context.Trace2, "Cannot prompt because user interactivity has been disabled.");
9092
}
9193
}
9294

@@ -95,8 +97,7 @@ protected void ThrowIfGuiPromptsDisabled()
9597
if (!Context.Settings.IsGuiPromptsEnabled)
9698
{
9799
Context.Trace.WriteLine($"{Constants.EnvironmentVariables.GitTerminalPrompts} is 0; GUI prompts have been disabled.");
98-
99-
throw new InvalidOperationException("Cannot show prompt because GUI prompts have been disabled.");
100+
throw new Trace2InvalidOperationException(Context.Trace2, "Cannot show prompt because GUI prompts have been disabled.");
100101
}
101102
}
102103

@@ -105,8 +106,7 @@ protected void ThrowIfTerminalPromptsDisabled()
105106
if (!Context.Settings.IsTerminalPromptsEnabled)
106107
{
107108
Context.Trace.WriteLine($"{Constants.EnvironmentVariables.GitTerminalPrompts} is 0; terminal prompts have been disabled.");
108-
109-
throw new InvalidOperationException("Cannot prompt because terminal prompts have been disabled.");
109+
throw new Trace2InvalidOperationException(Context.Trace2, "Cannot prompt because terminal prompts have been disabled.");
110110
}
111111
}
112112

‎src/shared/Core/Authentication/BasicAuthentication.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,12 +85,12 @@ private async Task<ICredential> GetCredentialsByUiAsync(string command, string a
8585

8686
if (!resultDict.TryGetValue("username", out userName))
8787
{
88-
throw new Exception("Missing 'username' in response");
88+
throw new Trace2Exception(Context.Trace2, "Missing 'username' in response");
8989
}
9090

9191
if (!resultDict.TryGetValue("password", out string password))
9292
{
93-
throw new Exception("Missing 'password' in response");
93+
throw new Trace2Exception(Context.Trace2, "Missing 'password' in response");
9494
}
9595

9696
return new GitCredential(userName, password);

‎src/shared/Core/Authentication/MicrosoftAuthentication.cs

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -333,9 +333,11 @@ private async Task RegisterTokenCacheAsync(IPublicClientApplication app, ITrace2
333333
}
334334
catch (MsalCachePersistenceException ex)
335335
{
336+
var message = "Cannot persist Microsoft Authentication data securely!";
336337
Context.Streams.Error.WriteLine("warning: cannot persist Microsoft authentication token cache securely!");
337-
Context.Trace.WriteLine("Cannot persist Microsoft Authentication data securely!");
338+
Context.Trace.WriteLine(message);
338339
Context.Trace.WriteException(ex);
340+
Context.Trace2.WriteError(message);
339341

340342
if (PlatformUtils.IsMacOS())
341343
{
@@ -498,10 +500,12 @@ private void EnsureCanUseEmbeddedWebView()
498500
#if NETFRAMEWORK
499501
if (!Context.SessionManager.IsDesktopSession)
500502
{
501-
throw new InvalidOperationException("Embedded web view is not available without a desktop session.");
503+
throw new Trace2InvalidOperationException(Context.Trace2,
504+
"Embedded web view is not available without a desktop session.");
502505
}
503506
#else
504-
throw new InvalidOperationException("Embedded web view is not available on .NET Core.");
507+
throw new Trace2InvalidOperationException(Context.Trace2,
508+
"Embedded web view is not available on .NET Core.");
505509
#endif
506510
}
507511

@@ -515,17 +519,20 @@ private void EnsureCanUseSystemWebView(IPublicClientApplication app, Uri redirec
515519
{
516520
if (!Context.SessionManager.IsWebBrowserAvailable)
517521
{
518-
throw new InvalidOperationException("System web view is not available without a way to start a browser.");
522+
throw new Trace2InvalidOperationException(Context.Trace2,
523+
"System web view is not available without a way to start a browser.");
519524
}
520525

521526
if (!app.IsSystemWebViewAvailable)
522527
{
523-
throw new InvalidOperationException("System web view is not available on this platform.");
528+
throw new Trace2InvalidOperationException(Context.Trace2,
529+
"System web view is not available on this platform.");
524530
}
525531

526532
if (!redirectUri.IsLoopback)
527533
{
528-
throw new InvalidOperationException("System web view is not available for this service configuration.");
534+
throw new Trace2InvalidOperationException(Context.Trace2,
535+
"System web view is not available for this service configuration.");
529536
}
530537
}
531538

0 commit comments

Comments
 (0)