(maint) Spit up methods throwing exceptions and using async

In general, exceptions shouldn't be thrown when running
inside an async method as they get caught inside an
aggregateexception most/all of the time.

This commit extracts the async code to an async internal
method instead of having the main method being
async. Not all methods are possible to do this way.

Author: AdmiringWorm

Source/GitReleaseManager/GitHubProvider.cs

@@ -28,9 +28,8 @@ namespace GitReleaseManager.Core
 
     public class GitHubProvider : IVcsProvider
     {
-        private readonly ILogger _logger = Log.ForContext<GitHubProvider>();
-
         private readonly Config _configuration;
+        private readonly ILogger _logger = Log.ForContext<GitHubProvider>();
         private readonly IMapper _mapper;
         private GitHubClient _gitHubClient;
 
@@ -41,34 +40,14 @@ public GitHubProvider(IMapper mapper, Config configuration, string userName, str
             CreateClient(userName, password, token);
         }
 
-        public async Task<int> GetNumberOfCommitsBetween(Milestone previousMilestone, Milestone currentMilestone, string user, string repository)
+        public Task<int> GetNumberOfCommitsBetween(Milestone previousMilestone, Milestone currentMilestone, string user, string repository)
         {
             if (currentMilestone is null)
             {
                 throw new ArgumentNullException(nameof(currentMilestone));
             }
 
-            try
-            {
-                if (previousMilestone == null)
-                {
-                    _logger.Verbose("Getting commit count between base '{Base}' and head '{Head}'", "master", currentMilestone.Title);
-                    var gitHubClientRepositoryCommitsCompare = await _gitHubClient.Repository.Commit.Compare(user, repository, "master", currentMilestone.Title).ConfigureAwait(false);
-                    return gitHubClientRepositoryCommitsCompare.AheadBy;
-                }
-
-                _logger.Verbose("Getting commit count between base '{Base}' and head '{Head}'", previousMilestone.Title, "master");
-                var compareResult = await _gitHubClient.Repository.Commit.Compare(user, repository, previousMilestone.Title, "master").ConfigureAwait(false);
-                return compareResult.AheadBy;
-            }
-            catch (NotFoundException)
-            {
-                _logger.Warning("Unable to find tag for milestone, so commit count will be returned as zero");
-
-                // If there is no tag yet the Compare will return a NotFoundException
-                // we can safely ignore
-                return 0;
-            }
+            return GetNumberOfCommitsBetweenInternal(previousMilestone, currentMilestone, user, repository);
         }
 
         public async Task<List<Issue>> GetIssuesAsync(Milestone targetMilestone)
@@ -174,27 +153,14 @@ public async Task<Release> CreateReleaseFromMilestone(string owner, string repos
             return _mapper.Map<Octokit.Release, Release>(release);
         }
 
-        public async Task<Release> CreateReleaseFromInputFile(string owner, string repository, string name, string inputFilePath, string targetCommitish, IList<string> assets, bool prerelease)
+        public Task<Release> CreateReleaseFromInputFile(string owner, string repository, string name, string inputFilePath, string targetCommitish, IList<string> assets, bool prerelease)
         {
             if (!File.Exists(inputFilePath))
             {
                 throw new ArgumentException("Unable to locate input file.");
             }
 
-            _logger.Verbose("Reading release notes from: '{FilePath}'", inputFilePath);
-
-            var inputFileContents = File.ReadAllText(inputFilePath);
-
-            var releaseUpdate = CreateNewRelease(name, name, inputFileContents, prerelease, targetCommitish);
-
-            _logger.Verbose("Creating new release on '{Owner}/{Repository}'", owner, repository);
-            _logger.Debug("{@ReleaseUpdate}", releaseUpdate);
-
-            var release = await _gitHubClient.Repository.Release.Create(owner, repository, releaseUpdate).ConfigureAwait(false);
-
-            await AddAssets(owner, repository, name, assets).ConfigureAwait(false);
-
-            return _mapper.Map<Octokit.Release, Release>(release);
+            return CreateReleaseFromInputFileInternal(owner, repository, name, inputFilePath, targetCommitish, assets, prerelease);
         }
 
         public async Task DiscardRelease(string owner, string repository, string name)
@@ -472,6 +438,24 @@ private async Task<bool> CommentsIncludeString(string owner, string repository,
             return issueComments.Any(c => c.Body.Contains(comment));
         }
 
+        private async Task<Release> CreateReleaseFromInputFileInternal(string owner, string repository, string name, string inputFilePath, string targetCommitish, IList<string> assets, bool prerelease)
+        {
+            _logger.Verbose("Reading release notes from: '{FilePath}'", inputFilePath);
+
+            var inputFileContents = File.ReadAllText(inputFilePath);
+
+            var releaseUpdate = CreateNewRelease(name, name, inputFileContents, prerelease, targetCommitish);
+
+            _logger.Verbose("Creating new release on '{Owner}/{Repository}'", owner, repository);
+            _logger.Debug("{@ReleaseUpdate}", releaseUpdate);
+
+            var release = await _gitHubClient.Repository.Release.Create(owner, repository, releaseUpdate).ConfigureAwait(false);
+
+            await AddAssets(owner, repository, name, assets).ConfigureAwait(false);
+
+            return _mapper.Map<Octokit.Release, Release>(release);
+        }
+
         private Task<IReadOnlyList<Octokit.Issue>> GetIssuesFromMilestoneAsync(string owner, string repository, string milestone, ItemStateFilter state = ItemStateFilter.Closed)
         {
             _logger.Verbose("Finding issues with milestone: '{Milestone}", milestone);
@@ -482,6 +466,31 @@ private async Task<bool> CommentsIncludeString(string owner, string repository,
             });
         }
 
+        private async Task<int> GetNumberOfCommitsBetweenInternal(Milestone previousMilestone, Milestone currentMilestone, string user, string repository)
+        {
+            try
+            {
+                if (previousMilestone == null)
+                {
+                    _logger.Verbose("Getting commit count between base '{Base}' and head '{Head}'", "master", currentMilestone.Title);
+                    var gitHubClientRepositoryCommitsCompare = await _gitHubClient.Repository.Commit.Compare(user, repository, "master", currentMilestone.Title).ConfigureAwait(false);
+                    return gitHubClientRepositoryCommitsCompare.AheadBy;
+                }
+
+                _logger.Verbose("Getting commit count between base '{Base}' and head '{Head}'", previousMilestone.Title, "master");
+                var compareResult = await _gitHubClient.Repository.Commit.Compare(user, repository, previousMilestone.Title, "master").ConfigureAwait(false);
+                return compareResult.AheadBy;
+            }
+            catch (NotFoundException)
+            {
+                _logger.Warning("Unable to find tag for milestone, so commit count will be returned as zero");
+
+                // If there is no tag yet the Compare will return a NotFoundException
+                // we can safely ignore
+                return 0;
+            }
+        }
+
         private async Task<Octokit.Release> GetReleaseFromTagNameAsync(string owner, string repository, string tagName)
         {
             _logger.Verbose("Finding release with tag name: '{TagName}'", tagName);

Source/GitReleaseManager/OctokitExtensions.cs

@@ -28,7 +28,7 @@ public static bool IsPullRequest(this Issue issue)
             return !(issue.PullRequest is null);
         }
 
-        public static async Task<IEnumerable<Issue>> AllIssuesForMilestone(this GitHubClient gitHubClient, Milestone milestone)
+        public static Task<IEnumerable<Issue>> AllIssuesForMilestone(this GitHubClient gitHubClient, Milestone milestone)
         {
             if (gitHubClient is null)
             {
@@ -40,6 +40,25 @@ public static async Task<IEnumerable<Issue>> AllIssuesForMilestone(this GitHubCl
                 throw new ArgumentNullException(nameof(milestone));
             }
 
+            return AllIssuesForMilestoneInternal(gitHubClient, milestone);
+        }
+
+        public static Uri HtmlUrl(this Milestone milestone)
+        {
+            if (milestone is null)
+            {
+                throw new ArgumentNullException(nameof(milestone));
+            }
+
+            var parts = milestone.Url.Split('/');
+            var user = parts[2];
+            var repository = parts[3];
+
+            return new Uri(string.Format(CultureInfo.InvariantCulture, "https://github.com/{0}/{1}/issues?milestone={2}&state=closed", user, repository, milestone.Number));
+        }
+
+        private static async Task<IEnumerable<Issue>> AllIssuesForMilestoneInternal(GitHubClient gitHubClient, Milestone milestone)
+        {
             var closedIssueRequest = new RepositoryIssueRequest
             {
                 Milestone = milestone.Number.ToString(CultureInfo.InvariantCulture),
@@ -63,19 +82,5 @@ public static async Task<IEnumerable<Issue>> AllIssuesForMilestone(this GitHubCl
 
             return openIssues.Union(closedIssues);
         }
-
-        public static Uri HtmlUrl(this Milestone milestone)
-        {
-            if (milestone is null)
-            {
-                throw new ArgumentNullException(nameof(milestone));
-            }
-
-            var parts = milestone.Url.Split('/');
-            var user = parts[2];
-            var repository = parts[3];
-
-            return new Uri(string.Format(CultureInfo.InvariantCulture, "https://github.com/{0}/{1}/issues?milestone={2}&state=closed", user, repository, milestone.Number));
-        }
     }
 }