Install should not silently fail

src/code/InstallHelper.cs

@@ -50,6 +50,7 @@ internal class InstallHelper : PSCmdlet
         private bool _noClobber;
         private bool _savePkg;
         List<string> _pathsToSearch;
+        List<string> _pkgNamesToInstall;
 
         #endregion
 
@@ -111,6 +112,7 @@ public List<PSResourceInfo> InstallPackages(
 
             // Create list of installation paths to search.
             _pathsToSearch = new List<string>();
+            _pkgNamesToInstall = names.ToList();
 
             // _pathsToInstallPkg will only contain the paths specified within the -Scope param (if applicable)
             // _pathsToSearch will contain all resource package subdirectories within _pathsToInstallPkg path locations
@@ -126,7 +128,6 @@ public List<PSResourceInfo> InstallPackages(
 
             // Go through the repositories and see which is the first repository to have the pkg version available
             return ProcessRepositories(
-                packageNames: names,
                 repository: repository,
                 trustRepository: _trustRepository,
                 credential: _credential,
@@ -139,23 +140,22 @@ public List<PSResourceInfo> InstallPackages(
 
         // This method calls iterates through repositories (by priority order) to search for the pkgs to install
         private List<PSResourceInfo> ProcessRepositories(
-            string[] packageNames,
             string[] repository,
             bool trustRepository,
             PSCredential credential,
             bool skipDependencyCheck)
         {
             var listOfRepositories = RepositorySettings.Read(repository, out string[] _);
-            List<string> pkgNamesToInstall = packageNames.ToList();
             var yesToAll = false;
             var noToAll = false;
 
             var findHelper = new FindHelper(_cancellationToken, _cmdletPassedIn);
             List<PSResourceInfo> allPkgsInstalled = new List<PSResourceInfo>();
+
             foreach (var repo in listOfRepositories)
             {
                 // If no more packages to install, then return
-                if (!pkgNamesToInstall.Any()) return allPkgsInstalled;
+                if (!_pkgNamesToInstall.Any()) return allPkgsInstalled;
 
                 string repoName = repo.Name;
                 _cmdletPassedIn.WriteVerbose(string.Format("Attempting to search for packages in '{0}'", repoName));
@@ -187,7 +187,7 @@ private List<PSResourceInfo> ProcessRepositories(
 
                 // Finds parent packages and dependencies
                 IEnumerable<PSResourceInfo> pkgsFromRepoToInstall = findHelper.FindByResourceName(
-                    name: pkgNamesToInstall.ToArray(),
+                    name: _pkgNamesToInstall.ToArray(),
                     type: ResourceType.None,
                     version: _versionRange != null ? _versionRange.OriginalString : null,
                     prerelease: _prerelease,
@@ -216,7 +216,6 @@ private List<PSResourceInfo> ProcessRepositories(
                 // Check to see if the pkgs (including dependencies) are already installed (ie the pkg is installed and the version satisfies the version range provided via param)
                 if (!_reinstall)
                 {
-                    // Removes all of the names that are already installed from the list of names to search for
                     pkgsFromRepoToInstall = FilterByInstalledPkgs(pkgsFromRepoToInstall);
                 }
 
@@ -227,19 +226,29 @@ private List<PSResourceInfo> ProcessRepositories(
 
                 List<PSResourceInfo> pkgsInstalled = InstallPackage(
                     pkgsFromRepoToInstall,
-                    pkgNamesToInstall,
                     repo.Url.AbsoluteUri,
                     credential,
                     isLocalRepo);
 
                 foreach (PSResourceInfo pkg in pkgsInstalled)
                 {
-                    pkgNamesToInstall.Remove(pkg.Name);
+                    _pkgNamesToInstall.RemoveAll(x => x.Equals(pkg.Name, StringComparison.InvariantCultureIgnoreCase));
                 }
 
                 allPkgsInstalled.AddRange(pkgsInstalled);
             }
 
+            // At this only package names left were those which could not be found in registered repositories
+            foreach (string pkgName in _pkgNamesToInstall)
+            {
+                var message = String.Format("Package '{0}' with requested version range {1} could not be installed as it was not found in any registered repositories",
+                    pkgName,
+                    _versionRange.ToString());
+                var ex = new ArgumentException(message);
+                var ResourceNotFoundError = new ErrorRecord(ex, "ResourceNotFoundError", ErrorCategory.ObjectNotFound, null);
+                _cmdletPassedIn.WriteError(ResourceNotFoundError);
+            }
+
             return allPkgsInstalled;
         }
 
@@ -286,14 +295,14 @@ private IEnumerable<PSResourceInfo> FilterByInstalledPkgs(IEnumerable<PSResource
                     pkg.Version));
 
                 filteredPackages.Remove(pkg.Name);
+                _pkgNamesToInstall.RemoveAll(x => x.Equals(pkg.Name, StringComparison.InvariantCultureIgnoreCase));
             }
 
             return filteredPackages.Values.ToArray();
         }
 
         private List<PSResourceInfo> InstallPackage(
             IEnumerable<PSResourceInfo> pkgsToInstall, // those found to be required to be installed (includes Dependency packages as well)
-            List<string> pkgNamesToInstall, // those requested by the user to be installed
             string repoUrl,
             PSCredential credential,
             bool isLocalRepo)
@@ -333,7 +342,7 @@ private List<PSResourceInfo> InstallPackage(
                         string activity = "";
                         string statusDescription = "";
 
-                        if (pkgNamesToInstall.ToList().Contains(pkg.Name, StringComparer.InvariantCultureIgnoreCase))
+                        if (_pkgNamesToInstall.Contains(pkg.Name, StringComparer.InvariantCultureIgnoreCase))
                         {
                             // Installing parent package (one whose name was passed in to install)
                             activityId = 0;
@@ -364,10 +373,16 @@ private List<PSResourceInfo> InstallPackage(
 
                     if (!NuGetVersion.TryParse(createFullVersion, out NuGetVersion pkgVersion))
                     {
-                        _cmdletPassedIn.WriteVerbose(string.Format("Error parsing package '{0}' version '{1}' into a NuGetVersion", 
-                            pkg.Name, pkg.Version.ToString()));
+                        var message = String.Format("{0} package could not be installed with error: could not parse package '{0}' version '{1} into a NuGetVersion",
+                            pkg.Name,
+                            pkg.Version.ToString());
+                        var ex = new ArgumentException(message);
+                        var packageIdentityVersionParseError = new ErrorRecord(ex, "psdataFileNotExistError", ErrorCategory.ReadError, null);
+                        _cmdletPassedIn.WriteError(packageIdentityVersionParseError);
+                        _pkgNamesToInstall.RemoveAll(x => x.Equals(pkg.Name, StringComparison.InvariantCultureIgnoreCase));
                         continue;
                     }
+
                     var pkgIdentity = new PackageIdentity(pkg.Name, pkgVersion);
                     var cacheContext = new SourceCacheContext();
 
@@ -492,11 +507,12 @@ private List<PSResourceInfo> InstallPackage(
                         var moduleManifest = Path.Combine(tempDirNameVersion, pkgIdentity.Id + ".psd1");
                         if (!File.Exists(moduleManifest))
                         {
-                            var message = String.Format("Module manifest file: {0} does not exist. This is not a valid PowerShell module.", moduleManifest);
+                            var message = String.Format("{0} package could not be installed with error: Module manifest file: {1} does not exist. This is not a valid PowerShell module.", pkgIdentity.Id, moduleManifest);
 
                             var ex = new ArgumentException(message);
                             var psdataFileDoesNotExistError = new ErrorRecord(ex, "psdataFileNotExistError", ErrorCategory.ReadError, null);
                             _cmdletPassedIn.WriteError(psdataFileDoesNotExistError);
+                            _pkgNamesToInstall.RemoveAll(x => x.Equals(pkg.Name, StringComparison.InvariantCultureIgnoreCase));
                             continue;
                         }
 
@@ -553,6 +569,7 @@ private List<PSResourceInfo> InstallPackage(
                             "InstallPackageFailed",
                             ErrorCategory.InvalidOperation,
                             _cmdletPassedIn));
+                    _pkgNamesToInstall.RemoveAll(x => x.Equals(pkg.Name, StringComparison.InvariantCultureIgnoreCase));
                 }
                 finally
                 {
@@ -611,11 +628,12 @@ private bool CallAcceptLicense(PSResourceInfo p, string moduleManifest, string t
 
                         if (!File.Exists(LicenseFilePath))
                         {
-                            var exMessage = "License.txt not Found. License.txt must be provided when user license acceptance is required.";
+                            var exMessage = String.Format("{0} package could not be installed with error: License.txt not found. License.txt must be provided when user license acceptance is required.", p.Name);
                             var ex = new ArgumentException(exMessage);
                             var acceptLicenseError = new ErrorRecord(ex, "LicenseTxtNotFound", ErrorCategory.ObjectNotFound, null);
 
                             _cmdletPassedIn.WriteError(acceptLicenseError);
+                            _pkgNamesToInstall.RemoveAll(x => x.Equals(p.Name, StringComparison.InvariantCultureIgnoreCase));
                             success = false;
                         }
 
@@ -638,11 +656,12 @@ private bool CallAcceptLicense(PSResourceInfo p, string moduleManifest, string t
                     // Check if user agreed to license terms, if they didn't then throw error, otherwise continue to install
                     if (!_acceptLicense)
                     {
-                        var message = $"License Acceptance is required for module '{p.Name}'. Please specify '-AcceptLicense' to perform this operation.";
+                        var message = String.Format("{0} package could not be installed with error: License Acceptance is required for module '{0}'. Please specify '-AcceptLicense' to perform this operation.", p.Name);
                         var ex = new ArgumentException(message);
                         var acceptLicenseError = new ErrorRecord(ex, "ForceAcceptLicense", ErrorCategory.InvalidArgument, null);
 
                         _cmdletPassedIn.WriteError(acceptLicenseError);
+                        _pkgNamesToInstall.RemoveAll(x => x.Equals(p.Name, StringComparison.InvariantCultureIgnoreCase));
                         success = false;
                     }
                 }
@@ -687,13 +706,14 @@ private bool DetectClobber(string pkgName, Hashtable parsedMetadataHashtable)
                     duplicateCmdlets.AddRange(duplicateCmds);
 
                     var errMessage = string.Format(
-                        "The following commands are already available on this system: '{0}'. This module '{1}' may override the existing commands. If you still want to install this module '{1}', remove the -NoClobber parameter.",
+                        "{1} package could not be installed with error: The following commands are already available on this system: '{0}'. This module '{1}' may override the existing commands. If you still want to install this module '{1}', remove the -NoClobber parameter.",
                         String.Join(", ", duplicateCmdlets), pkgName);
 
                     var ex = new ArgumentException(errMessage);
                     var noClobberError = new ErrorRecord(ex, "CommandAlreadyExists", ErrorCategory.ResourceExists, null);
 
                     _cmdletPassedIn.WriteError(noClobberError);
+                    _pkgNamesToInstall.RemoveAll(x => x.Equals(pkgName, StringComparison.InvariantCultureIgnoreCase));
                     foundClobber = true;
 
                     return foundClobber;
@@ -716,10 +736,12 @@ private void CreateMetadataXMLFile(string dirNameVersion, string installPath, PS
             // Write all metadata into metadataXMLPath
             if (!pkg.TryWrite(metadataXMLPath, out string error))
             {
-                var message = string.Format("Error parsing metadata into XML: '{0}'", error);
+                var message = string.Format("{0} package could not be installed with error: Error parsing metadata into XML: '{1}'", pkg.Name, error);
                 var ex = new ArgumentException(message);
                 var ErrorParsingMetadata = new ErrorRecord(ex, "ErrorParsingMetadata", ErrorCategory.ParserError, null);
-                WriteError(ErrorParsingMetadata);
+
+                _cmdletPassedIn.WriteError(ErrorParsingMetadata);
+                _pkgNamesToInstall.RemoveAll(x => x.Equals(pkg.Name, StringComparison.InvariantCultureIgnoreCase));
             }
         }
 

src/code/InstallPSResource.cs

@@ -5,6 +5,7 @@
 using NuGet.Versioning;
 using System;
 using System.Collections.Generic;
+using System.Linq;
 using System.Management.Automation;
 
 using Dbg = System.Diagnostics.Debug;

src/code/SavePSResource.cs

@@ -260,7 +260,7 @@ private void ProcessSaveHelper(string[] pkgNames, bool pkgPrerelease, string[] p
                 includeXML: IncludeXML, 
                 skipDependencyCheck: SkipDependencyCheck,
                 savePkg: true,
-                pathsToInstallPkg: new List<string> { _path } );
+                pathsToInstallPkg: new List<string> { _path });
 
             if (PassThru)
             {

test/InstallPSResource.Tests.ps1

@@ -36,7 +36,7 @@ Describe 'Test Install-PSResource for Module' {
     }
 
     It "Install specific module resource by name" {
-        Install-PSResource -Name "TestModule" -Repository $TestGalleryName  
+        Install-PSResource -Name "TestModule" -Repository $TestGalleryName 
         $pkg = Get-Module "TestModule" -ListAvailable
         $pkg.Name | Should -Be "TestModule" 
         $pkg.Version | Should -Be "1.3.0"
@@ -57,9 +57,11 @@ Describe 'Test Install-PSResource for Module' {
     }
 
     It "Should not install resource given nonexistant name" {
-        Install-PSResource -Name NonExistantModule -Repository $TestGalleryName  
+        Install-PSResource -Name "NonExistantModule" -Repository $TestGalleryName -ErrorVariable err -ErrorAction SilentlyContinue
         $pkg = Get-Module "NonExistantModule" -ListAvailable
         $pkg.Name | Should -BeNullOrEmpty
+        $err.Count | Should -Not -Be 0
+        $err[0].FullyQualifiedErrorId | Should -BeExactly "ResourceNotFoundError,Microsoft.PowerShell.PowerShellGet.Cmdlets.InstallPSResource" 
     }
 
     # Do some version testing, but Find-PSResource should be doing thorough testing
@@ -97,7 +99,9 @@ Describe 'Test Install-PSResource for Module' {
     ) {
         param($Version, $Description)
 
-        Install-PSResource -Name "TestModule" -Version $Version -Repository $TestGalleryName
+        Install-PSResource -Name "TestModule" -Version $Version -Repository $TestGalleryName -ErrorAction SilentlyContinue
+        $Error[0].FullyQualifiedErrorId | Should -be "ResourceNotFoundError,Microsoft.PowerShell.PowerShellGet.Cmdlets.InstallPSResource"
+
         $res = Get-Module "TestModule" -ListAvailable
         $res | Should -BeNullOrEmpty
     }