Fix format exception bug (#156)

microsoft · Sep 2, 2021 · 371490a · 371490a
1 parent c03c4bf
commit 371490a
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 24 deletions.
diff --git a/src/WingetCreateCLI/Commands/NewCommand.cs b/src/WingetCreateCLI/Commands/NewCommand.cs
@@ -18,6 +18,7 @@ namespace Microsoft.WingetCreateCLI.Commands
     using Microsoft.WingetCreateCLI.Telemetry.Events;
     using Microsoft.WingetCreateCore;
     using Microsoft.WingetCreateCore.Common;
+    using Microsoft.WingetCreateCore.Common.Exceptions;
     using Microsoft.WingetCreateCore.Models;
     using Microsoft.WingetCreateCore.Models.DefaultLocale;
     using Microsoft.WingetCreateCore.Models.Installer;
@@ -110,18 +111,17 @@ public override async Task<bool> Execute()
                     return false;
                 }
 
-                if (!PackageParser.ParsePackages(
-                    packageFiles,
-                    this.InstallerUrls,
-                    manifests,
-                    out List<PackageParser.DetectedArch> detectedArchs))
+                try
                 {
-                    Logger.ErrorLocalized(nameof(Resources.PackageParsing_Error));
+                    PackageParser.ParsePackages(packageFiles, this.InstallerUrls, manifests, out List<PackageParser.DetectedArch> detectedArchs);
+                    DisplayMismatchedArchitectures(detectedArchs);
+                }
+                catch (ParsePackageException exception)
+                {
+                    exception.ParseFailedInstallerUrls.ForEach(i => Logger.ErrorLocalized(nameof(Resources.PackageParsing_Error), i));
                     return false;
                 }
 
-                DisplayMismatchedArchitectures(detectedArchs);
-
                 Console.WriteLine(Resources.NewCommand_Header);
                 Console.WriteLine();
                 Logger.InfoLocalized(nameof(Resources.ManifestDocumentation_HelpText), ManifestDocumentationUrl);

diff --git a/src/WingetCreateCore/Common/PackageParser.cs b/src/WingetCreateCore/Common/PackageParser.cs
@@ -82,8 +82,7 @@ public static void SetHttpMessageHandler(HttpMessageHandler httpMessageHandler)
         /// <param name="urls">Installer urls. </param>
         /// <param name="manifests">Wrapper object for manifest object models.</param>
         /// <param name="detectedArchOfInstallers">List of DetectedArch objects that represent each installers detected architectures.</param>
-        /// <returns>True if packages were successfully parsed and metadata extracted, false otherwise.</returns>
-        public static bool ParsePackages(
+        public static void ParsePackages(
             IEnumerable<string> paths,
             IEnumerable<string> urls,
             Manifests manifests,
@@ -97,16 +96,20 @@ public static bool ParsePackages(
 
             InstallerManifest installerManifest = manifests.InstallerManifest = new InstallerManifest();
             DefaultLocaleManifest defaultLocaleManifest = manifests.DefaultLocaleManifest = new DefaultLocaleManifest();
+            List<string> parseFailedInstallerUrls = new List<string>();
 
             foreach (var package in paths.Zip(urls, (path, url) => (path, url)))
             {
                 if (!ParsePackage(package.path, package.url, manifests, ref detectedArchOfInstallers))
                 {
-                    return false;
+                    parseFailedInstallerUrls.Add(package.url);
                 }
             }
 
-            return true;
+            if (parseFailedInstallerUrls.Any())
+            {
+                throw new ParsePackageException(parseFailedInstallerUrls);
+            }
         }
 
         /// <summary>

diff --git a/src/WingetCreateTests/WingetCreateTests/UnitTests/PackageParserTests.cs b/src/WingetCreateTests/WingetCreateTests/UnitTests/PackageParserTests.cs
@@ -46,10 +46,8 @@ public void ParseExeInstallerFile()
         {
             var testExeInstallerPath = TestUtils.MockDownloadFile(TestConstants.TestExeInstaller);
             Assert.That(testExeInstallerPath, Is.Not.Null.And.Not.Empty);
-
             Manifests manifests = new Manifests();
-
-            Assert.IsTrue(PackageParser.ParsePackages(new[] { testExeInstallerPath }, new[] { TestConstants.TestExeInstaller }, manifests, out _));
+            Assert.DoesNotThrow(() => PackageParser.ParsePackages(new[] { testExeInstallerPath }, new[] { TestConstants.TestExeInstaller }, manifests, out _));
             Assert.AreEqual("WingetCreateTestExeInstaller", manifests.DefaultLocaleManifest.PackageName);
             Assert.AreEqual("Microsoft Corporation", manifests.DefaultLocaleManifest.Publisher);
             Assert.AreEqual("MicrosoftCorporation.WingetCreateTestExeInstaller", manifests.VersionManifest.PackageIdentifier);
@@ -65,10 +63,8 @@ public void ParseMsiInstallerFile()
         {
             var testMsiInstallerPath = TestUtils.MockDownloadFile(TestConstants.TestMsiInstaller);
             Assert.That(testMsiInstallerPath, Is.Not.Null.And.Not.Empty);
-
             Manifests manifests = new Manifests();
-
-            Assert.IsTrue(PackageParser.ParsePackages(new[] { testMsiInstallerPath }, new[] { TestConstants.TestExeInstaller }, manifests, out _));
+            Assert.DoesNotThrow(() => PackageParser.ParsePackages(new[] { testMsiInstallerPath }, new[] { TestConstants.TestExeInstaller }, manifests, out _));
             Assert.AreEqual("WingetCreateTestMsiInstaller", manifests.DefaultLocaleManifest.PackageName);
             Assert.AreEqual("Microsoft Corporation", manifests.DefaultLocaleManifest.Publisher);
             Assert.AreEqual("MicrosoftCorporation.WingetCreateTestMsiInstaller", manifests.VersionManifest.PackageIdentifier);
@@ -84,10 +80,8 @@ public void ParseMsixInstallerFile()
         {
             var testMsixInstallerPath = TestUtils.MockDownloadFile(TestConstants.TestMsixInstaller);
             Assert.That(testMsixInstallerPath, Is.Not.Null.And.Not.Empty);
-
             Manifests manifests = new Manifests();
-
-            Assert.IsTrue(PackageParser.ParsePackages(new[] { testMsixInstallerPath }, new[] { TestConstants.TestMsixInstaller }, manifests, out _));
+            Assert.DoesNotThrow(() => PackageParser.ParsePackages(new[] { testMsixInstallerPath }, new[] { TestConstants.TestMsixInstaller }, manifests, out _));
             Assert.AreEqual("WingetCreateTestMsixInstaller", manifests.DefaultLocaleManifest.PackageName);
             Assert.AreEqual("Microsoft Corporation", manifests.DefaultLocaleManifest.Publisher);
             Assert.AreEqual("1.0.1.0", manifests.VersionManifest.PackageVersion);
@@ -108,10 +102,8 @@ public void ParseMultipleInstallers()
             Assert.That(testExeInstallerPath, Is.Not.Null.And.Not.Empty);
             var testMsixInstallerPath = TestUtils.MockDownloadFile(TestConstants.TestMsixInstaller);
             Assert.That(testMsixInstallerPath, Is.Not.Null.And.Not.Empty);
-
             Manifests manifests = new Manifests();
-
-            Assert.IsTrue(PackageParser.ParsePackages(
+            Assert.DoesNotThrow(() => PackageParser.ParsePackages(
                 new[] { testExeInstallerPath, testMsiInstallerPath, testMsixInstallerPath },
                 new[] { TestConstants.TestExeInstaller, TestConstants.TestMsiInstaller, TestConstants.TestMsixInstaller },
                 manifests,