Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

Commit 208c34e

Browse files
author
Meaghan Lewis
authored
Merge pull request #1853 from github/fixes/1305-better-failures
Better failure modes
2 parents d8980c3 + 20453cb commit 208c34e

File tree

25 files changed

+546
-88
lines changed

25 files changed

+546
-88
lines changed

‎src/GitHub.App/GitHub.App.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,7 @@
260260
<Compile Include="ViewModels\GitHubPane\NavigationViewModel.cs" />
261261
<Compile Include="ViewModels\GitHubPane\GitHubPaneViewModel.cs" />
262262
<Compile Include="SampleData\PullRequestCheckViewModelDesigner.cs" />
263+
<Compile Include="ViewModels\GitHubPane\LoginFailedViewModel.cs" />
263264
<Compile Include="ViewModels\GitHubPane\PullRequestCheckType.cs" />
264265
<Compile Include="ViewModels\GitHubPane\PullRequestFilesViewModel.cs" />
265266
<Compile Include="ViewModels\GitHubPane\PullRequestListItemViewModel.cs" />

‎src/GitHub.App/SampleData/SampleViewModels.cs‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using System;
22
using System.Collections.Generic;
33
using System.Collections.ObjectModel;
4+
using System.ComponentModel;
45
using System.Diagnostics.CodeAnalysis;
56
using System.Reactive;
67
using System.Threading.Tasks;
@@ -201,8 +202,15 @@ class Conn : IConnection
201202

202203
public Octokit.User User => null;
203204
public bool IsLoggedIn => true;
205+
public bool IsLoggingIn => false;
204206

205207
public Exception ConnectionError => null;
208+
209+
event PropertyChangedEventHandler INotifyPropertyChanged.PropertyChanged
210+
{
211+
add { }
212+
remove { }
213+
}
206214
}
207215

208216
public RepositoryPublishViewModelDesigner()
@@ -421,12 +429,21 @@ public void Login()
421429
{
422430
}
423431

432+
public void Retry()
433+
{
434+
}
435+
424436
public bool OpenRepository()
425437
{
426438
return true;
427439
}
428440

441+
public string ErrorMessage { get; set; }
429442
public IConnection SectionConnection { get; }
443+
public bool IsLoggingIn { get; set; }
444+
public bool ShowLogin { get; set; }
445+
public bool ShowLogout { get; set; }
446+
public bool ShowRetry { get; set; }
430447
public ICommand Clone { get; }
431448
}
432449

‎src/GitHub.App/Services/StandardUserErrors.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -138,8 +138,8 @@ public static class StandardUserErrors
138138
{ ErrorType.EnterpriseConnectFailed, Map(Defaults("Connecting to GitHub Enterprise instance failed", "Could not find a GitHub Enterprise instance at '{0}'. Double check the URL and your internet/intranet connection.")) },
139139
{ ErrorType.LaunchEnterpriseConnectionFailed, Map(Defaults("Failed to launch the enterprise connection.")) },
140140
{ ErrorType.LogFileError, Map(Defaults("Could not open the log file", "Could not find or open the log file.")) },
141-
{ ErrorType.LoginFailed, Map(Defaults("login failed", "Unable to retrieve your user info from the server. A proxy server might be interfering with the request.")) },
142-
{ ErrorType.LogoutFailed, Map(Defaults("logout failed", "Logout failed. A proxy server might be interfering with the request.")) },
141+
{ ErrorType.LoginFailed, Map(Defaults("Login failed", "Unable to retrieve your user info from the server. A proxy server might be interfering with the request.")) },
142+
{ ErrorType.LogoutFailed, Map(Defaults("Logout failed", "Logout failed. A proxy server might be interfering with the request.")) },
143143
{ ErrorType.RepoCreationAsPrivateNotAvailableForFreePlan, Map(Defaults("Failed to create private repository", "You are currently on a free plan and unable to create private repositories. Either make the repository public or upgrade your account on the website to a plan that allows for private repositories.")) },
144144
{ ErrorType.RepoCreationFailed, Map(Defaults("Failed to create repository", "An error occurred while creating the repository. You might need to open a shell and debug the state of this repo.")) },
145145
{ ErrorType.RepoExistsOnDisk, Map(Defaults("Failed to create repository", "A repository named '{0}' exists in the directory\n'{1}'.")) },
@@ -209,7 +209,7 @@ static IObservable<RecoveryOptionResult> DisplayErrorMessage(this Exception exce
209209
return userError.Throw();
210210
}
211211

212-
static UserError GetUserFriendlyError(this Exception exception, ErrorType errorType, params object[] messageArgs)
212+
public static UserError GetUserFriendlyError(this Exception exception, ErrorType errorType, params object[] messageArgs)
213213
{
214214
return Translator.Value.GetUserError(errorType, exception, messageArgs);
215215
}

‎src/GitHub.App/ViewModels/GitHubPane/GitHubPaneViewModel.cs‎

Lines changed: 43 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ public sealed class GitHubPaneViewModel : ViewModelBase, IGitHubPaneViewModel, I
4444
readonly ILoggedOutViewModel loggedOut;
4545
readonly INotAGitHubRepositoryViewModel notAGitHubRepository;
4646
readonly INotAGitRepositoryViewModel notAGitRepository;
47+
readonly ILoginFailedViewModel loginFailed;
4748
readonly SemaphoreSlim navigating = new SemaphoreSlim(1);
4849
readonly ObservableAsPropertyHelper<ContentOverride> contentOverride;
4950
readonly ObservableAsPropertyHelper<bool> isSearchEnabled;
@@ -52,6 +53,7 @@ public sealed class GitHubPaneViewModel : ViewModelBase, IGitHubPaneViewModel, I
5253
readonly ReactiveCommand<Unit> showPullRequests;
5354
readonly ReactiveCommand<object> openInBrowser;
5455
readonly ReactiveCommand<object> help;
56+
IDisposable connectionSubscription;
5557
Task initializeTask;
5658
IViewModel content;
5759
ILocalRepositoryModel localRepository;
@@ -68,7 +70,8 @@ public GitHubPaneViewModel(
6870
INavigationViewModel navigator,
6971
ILoggedOutViewModel loggedOut,
7072
INotAGitHubRepositoryViewModel notAGitHubRepository,
71-
INotAGitRepositoryViewModel notAGitRepository)
73+
INotAGitRepositoryViewModel notAGitRepository,
74+
ILoginFailedViewModel loginFailed)
7275
{
7376
Guard.ArgumentNotNull(viewModelFactory, nameof(viewModelFactory));
7477
Guard.ArgumentNotNull(apiClientFactory, nameof(apiClientFactory));
@@ -80,6 +83,7 @@ public GitHubPaneViewModel(
8083
Guard.ArgumentNotNull(loggedOut, nameof(loggedOut));
8184
Guard.ArgumentNotNull(notAGitHubRepository, nameof(notAGitHubRepository));
8285
Guard.ArgumentNotNull(notAGitRepository, nameof(notAGitRepository));
86+
Guard.ArgumentNotNull(loginFailed, nameof(loginFailed));
8387

8488
this.viewModelFactory = viewModelFactory;
8589
this.apiClientFactory = apiClientFactory;
@@ -89,6 +93,7 @@ public GitHubPaneViewModel(
8993
this.loggedOut = loggedOut;
9094
this.notAGitHubRepository = notAGitHubRepository;
9195
this.notAGitRepository = notAGitRepository;
96+
this.loginFailed = loginFailed;
9297

9398
var contentAndNavigatorContent = Observable.CombineLatest(
9499
this.WhenAnyValue(x => x.Content),
@@ -389,6 +394,8 @@ async Task UpdateContent(ILocalRepositoryModel repository)
389394
log.Debug("UpdateContent called with {CloneUrl}", repository?.CloneUrl);
390395

391396
LocalRepository = repository;
397+
connectionSubscription?.Dispose();
398+
connectionSubscription = null;
392399
Connection = null;
393400
Content = null;
394401
navigator.Clear();
@@ -410,26 +417,56 @@ async Task UpdateContent(ILocalRepositoryModel repository)
410417
var isDotCom = HostAddress.IsGitHubDotComUri(repositoryUrl);
411418
var client = await apiClientFactory.Create(repository.CloneUrl);
412419
var isEnterprise = isDotCom ? false : await client.IsEnterprise();
420+
var notGitHubRepo = true;
413421

414-
if ((isDotCom || isEnterprise) && await IsValidRepository(client))
422+
if (isDotCom || isEnterprise)
415423
{
416424
var hostAddress = HostAddress.Create(repository.CloneUrl);
417425

426+
notGitHubRepo = false;
427+
418428
Connection = await connectionManager.GetConnection(hostAddress);
429+
Connection?.WhenAnyValue(
430+
x => x.IsLoggedIn,
431+
x => x.IsLoggingIn,
432+
(_, __) => Unit.Default)
433+
.Skip(1)
434+
.Throttle(TimeSpan.FromMilliseconds(100))
435+
.ObserveOn(RxApp.MainThreadScheduler)
436+
.Subscribe(_ => UpdateContent(LocalRepository).Forget());
419437

420438
if (Connection?.IsLoggedIn == true)
421439
{
422-
log.Debug("Found a GitHub repository: {CloneUrl}", repository?.CloneUrl);
423-
Content = navigator;
424-
await ShowDefaultPage();
440+
if (await IsValidRepository(client) == true)
441+
{
442+
log.Debug("Found a GitHub repository: {CloneUrl}", repository?.CloneUrl);
443+
Content = navigator;
444+
await ShowDefaultPage();
445+
}
446+
else
447+
{
448+
notGitHubRepo = true;
449+
}
450+
}
451+
else if (Connection?.IsLoggingIn == true)
452+
{
453+
log.Debug("Found a GitHub repository: {CloneUrl} and logging in", repository?.CloneUrl);
454+
Content = null;
455+
}
456+
else if (Connection?.ConnectionError != null)
457+
{
458+
log.Debug("Found a GitHub repository: {CloneUrl} with login error", repository?.CloneUrl);
459+
loginFailed.Initialize(Connection.ConnectionError.GetUserFriendlyError(ErrorType.LoginFailed));
460+
Content = loginFailed;
425461
}
426462
else
427463
{
428464
log.Debug("Found a a GitHub repository but not logged in: {CloneUrl}", repository?.CloneUrl);
429465
Content = loggedOut;
430466
}
431467
}
432-
else
468+
469+
if (notGitHubRepo)
433470
{
434471
log.Debug("Not a GitHub repository: {CloneUrl}", repository?.CloneUrl);
435472
Content = notAGitHubRepository;

‎src/GitHub.App/ViewModels/GitHubPane/LoginFailedViewModel.cs‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
using System;
2+
using System.ComponentModel.Composition;
3+
using GitHub.Services;
4+
using ReactiveUI;
5+
6+
namespace GitHub.ViewModels.GitHubPane
7+
{
8+
/// <summary>
9+
/// The view model for the "Login Failed" view in the GitHub pane.
10+
/// </summary>
11+
[Export(typeof(ILoginFailedViewModel))]
12+
[PartCreationPolicy(CreationPolicy.NonShared)]
13+
public class LoginFailedViewModel : PanePageViewModelBase, ILoginFailedViewModel
14+
{
15+
readonly ITeamExplorerServices teServices;
16+
UserError loginError;
17+
18+
/// <summary>
19+
/// Initializes a new instance of the <see cref="LoginFailedViewModel"/> class.
20+
/// </summary>
21+
[ImportingConstructor]
22+
public LoginFailedViewModel(ITeamExplorerServices teServices)
23+
{
24+
this.teServices = teServices;
25+
OpenTeamExplorer = ReactiveCommand.Create().OnExecuteCompleted(_ => DoOpenTeamExplorer());
26+
}
27+
28+
/// <inheritdoc/>
29+
public UserError LoginError
30+
{
31+
get => loginError;
32+
private set => this.RaiseAndSetIfChanged(ref loginError, value);
33+
}
34+
35+
/// <inheritdoc/>
36+
public ReactiveCommand<object> OpenTeamExplorer { get; }
37+
38+
public void Initialize(UserError error)
39+
{
40+
LoginError = error;
41+
}
42+
43+
void DoOpenTeamExplorer() => teServices.ShowConnectPage();
44+
}
45+
}

‎src/GitHub.Exports.Reactive/GitHub.Exports.Reactive.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,7 @@
203203
<Compile Include="ViewModels\GitHubPane\IIssueListViewModelBase.cs" />
204204
<Compile Include="ViewModels\GitHubPane\ILoggedOutViewModel.cs" />
205205
<Compile Include="ViewModels\GitHubPane\INavigationViewModel.cs" />
206+
<Compile Include="ViewModels\GitHubPane\ILoginFailedViewModel.cs" />
206207
<Compile Include="ViewModels\GitHubPane\IPanePageViewModel.cs" />
207208
<Compile Include="ViewModels\GitHubPane\IPullRequestCheckViewModel.cs" />
208209
<Compile Include="ViewModels\GitHubPane\IPullRequestFilesViewModel.cs" />

‎src/GitHub.Exports.Reactive/ViewModels/GitHubPane/ILoginFailedViewModel.cs‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
using ReactiveUI;
2+
3+
namespace GitHub.ViewModels.GitHubPane
4+
{
5+
/// <summary>
6+
/// Defines the view model for the "Login Failed" view in the GitHub pane.
7+
/// </summary>
8+
public interface ILoginFailedViewModel : IPanePageViewModel
9+
{
10+
/// <summary>
11+
/// Gets a description of the login failure.
12+
/// </summary>
13+
UserError LoginError { get; }
14+
15+
/// <summary>
16+
/// Gets a command which opens the Team Explorer Connect page.
17+
/// </summary>
18+
ReactiveCommand<object> OpenTeamExplorer { get; }
19+
20+
/// <summary>
21+
/// Initializes the view model with an error.
22+
/// </summary>
23+
/// <param name="error">The error.</param>
24+
void Initialize(UserError error);
25+
}
26+
}

‎src/GitHub.Exports/Models/IConnection.cs‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
using System;
2+
using System.ComponentModel;
3+
using System.Threading.Tasks;
24
using GitHub.Primitives;
35
using Octokit;
46

@@ -7,7 +9,7 @@ namespace GitHub.Models
79
/// <summary>
810
/// Represents a configured connection to a GitHub account.
911
/// </summary>
10-
public interface IConnection
12+
public interface IConnection : INotifyPropertyChanged
1113
{
1214
/// <summary>
1315
/// Gets the host address of the GitHub instance.
@@ -32,6 +34,11 @@ public interface IConnection
3234
/// </summary>
3335
bool IsLoggedIn { get; }
3436

37+
/// <summary>
38+
/// Gets a value indicating whether a login is currently being attempted on the connection.
39+
/// </summary>
40+
bool IsLoggingIn { get; }
41+
3542
/// <summary>
3643
/// Gets the exception that occurred when trying to log in, if <see cref="IsLoggedIn"/> is
3744
/// false.

0 commit comments

Comments
 (0)