Skip to content

Commit 94f8d91

Browse files
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 2399751 commit 94f8d91

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

+251
-162
lines changed

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

Lines changed: 1 addition & 1 deletion
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]

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: 2 additions & 3 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;
@@ -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

0 commit comments

Comments
 (0)