Add retries when trying to update bundle resources

Fixes 6882 and 6902

6 files changed, 51 insertions, 33 deletions

diff --git a/src/api/wix/WixToolset.Extensibility/Services/IFileSystem.cs b/src/api/wix/WixToolset.Extensibility/Services/IFileSystem.cs
index d633c823..26184462 100644
--- a/src/api/wix/WixToolset.Extensibility/Services/IFileSystem.cs
+++ b/src/api/wix/WixToolset.Extensibility/Services/IFileSystem.cs
@@ -2,6 +2,8 @@
 
 namespace WixToolset.Extensibility.Services
 {
+    using System;
+
     /// <summary>
     /// Abstracts basic file system operations.
     /// </summary>
@@ -16,10 +18,27 @@ namespace WixToolset.Extensibility.Services
         void CopyFile(string source, string destination, bool allowHardlink);
 
         /// <summary>
+        /// Deletes a file.
+        /// </summary>
+        /// <param name="source">The file to delete.</param>
+        /// <param name="throwOnError">Indicates the file must be deleted. Default is a best effort delete.</param>
+        /// <param name="maxRetries">Maximum retry attempts. Default is 4.</param>
+        void DeleteFile(string source, bool throwOnError = false, int maxRetries = 4);
+
+        /// <summary>
         /// Moves a file.
         /// </summary>
         /// <param name="source">The file to move.</param>
         /// <param name="destination">The destination file.</param>
         void MoveFile(string source, string destination);
+
+        /// <summary>
+        /// Executes an action and retries on any exception a few times with short pause
+        /// between each attempt. Primarily intended for use with file system operations
+        /// that might get interrupted by external systems (usually anti-virus).
+        /// </summary>
+        /// <param name="action">Action to execute.</param>
+        /// <param name="maxRetries">Maximum retry attempts. Default is 4.</param>
+        void ExecuteWithRetries(Action action, int maxRetries = 4);
     }
 }
diff --git a/src/wix/WixToolset.Core.Burn/Bundles/CreateBundleExeCommand.cs b/src/wix/WixToolset.Core.Burn/Bundles/CreateBundleExeCommand.cs
index aa4730fb..fdc3dfe3 100644
--- a/src/wix/WixToolset.Core.Burn/Bundles/CreateBundleExeCommand.cs
+++ b/src/wix/WixToolset.Core.Burn/Bundles/CreateBundleExeCommand.cs
@@ -349,17 +349,11 @@ namespace WixToolset.Core.Burn.Bundles
 
             try
             {
-                resources.Save(bundleTempPath);
+                this.FileSystem.ExecuteWithRetries(() => resources.Save(bundleTempPath));
             }
-            catch (IOException)
+            catch (IOException e)
             {
-                // If there was no icon or splash screen then this is unexpected so rethrow.
-                if (bundleInfo.IconSourceFile == null && bundleInfo.SplashScreenSourceFile == null)
-                {
-                    throw;
-                }
-
-                this.Messaging.Write(BurnBackendErrors.FailedToAddIconOrSplashScreenToBundle(bundleInfo.SourceLineNumbers, bundleInfo.IconSourceFile?.Path, bundleInfo.SplashScreenSourceFile?.Path));
+                this.Messaging.Write(BurnBackendErrors.FailedToUpdateBundleResources(bundleInfo.SourceLineNumbers, bundleInfo.IconSourceFile?.Path, bundleInfo.SplashScreenSourceFile?.Path, e.Message));
             }
         }
 
diff --git a/src/wix/WixToolset.Core.Burn/BurnBackendErrors.cs b/src/wix/WixToolset.Core.Burn/BurnBackendErrors.cs
index 04290667..ee6fc0f8 100644
--- a/src/wix/WixToolset.Core.Burn/BurnBackendErrors.cs
+++ b/src/wix/WixToolset.Core.Burn/BurnBackendErrors.cs
@@ -42,7 +42,7 @@ namespace WixToolset.Core.Burn
             return Message(sourceLineNumbers, Ids.ExternalPayloadCollision2, "The location of the symbol related to the previous error.");
         }
 
-        public static Message FailedToAddIconOrSplashScreenToBundle(SourceLineNumber sourceLineNumbers, string iconPath, string splashScreenPath)
+        public static Message FailedToUpdateBundleResources(SourceLineNumber sourceLineNumbers, string iconPath, string splashScreenPath, string detail)
         {
             var additionalDetail = String.Empty;
 
@@ -51,18 +51,18 @@ namespace WixToolset.Core.Burn
             }
             else if (String.IsNullOrEmpty(iconPath))
             {
-                additionalDetail = $" Ensure the splash screen file is a bitmap file at '{splashScreenPath}'";
+                additionalDetail = $" Ensure the splash screen file is a bitmap file at '{splashScreenPath}'.";
             }
             else if (String.IsNullOrEmpty(splashScreenPath))
             {
-                additionalDetail = $" Ensure the bundle icon file is an icon file at '{iconPath}'";
+                additionalDetail = $" Ensure the bundle icon file is an icon file at '{iconPath}'.";
             }
             else
             {
-                additionalDetail = $" Ensure the bundle icon file is an icon file at '{iconPath}' and the splash screen file is a bitmap file at '{splashScreenPath}'";
+                additionalDetail = $" Ensure the bundle icon file is an icon file at '{iconPath}' and the splash screen file is a bitmap file at '{splashScreenPath}'.";
             }
 
-            return Message(sourceLineNumbers, Ids.FailedToAddIconOrSplashScreenToBundle, "Failed to add resources to the bundle.{0}", additionalDetail);
+            return Message(sourceLineNumbers, Ids.FailedToUpdateBundleResources, "Failed to update resources in the bundle.{0} Detail: {1}", additionalDetail, detail);
         }
 
         public static Message PackageCachePayloadCollision(SourceLineNumber sourceLineNumbers, string payloadId, string payloadName, string packageId)
@@ -113,7 +113,7 @@ namespace WixToolset.Core.Burn
             TooManyAttachedContainers = 8008,
             IncompatibleWixBurnSection = 8009,
             UnsupportedRemotePackagePayload = 8010,
-            FailedToAddIconOrSplashScreenToBundle = 8011,
+            FailedToUpdateBundleResources = 8011,
             InvalidBundleManifest = 8012,
             BundleMultipleProviders = 8013,
         } // last available is 8499. 8500 is BurnBackendWarnings.
diff --git a/src/wix/WixToolset.Core/Bind/TransferFilesCommand.cs b/src/wix/WixToolset.Core/Bind/TransferFilesCommand.cs
index 87c2590f..4a0329bc 100644
--- a/src/wix/WixToolset.Core/Bind/TransferFilesCommand.cs
+++ b/src/wix/WixToolset.Core/Bind/TransferFilesCommand.cs
@@ -204,7 +204,7 @@ namespace WixToolset.Core.Bind
             foreach (var file in files)
             {
                 var fileInfo = new FileInfo(file);
-                ExtensibilityServices.FileSystem.ActionWithRetries(() => fileInfo.SetAccessControl(aclReset));
+                this.FileSystem.ExecuteWithRetries(() => fileInfo.SetAccessControl(aclReset));
             }
         }
     }
diff --git a/src/wix/WixToolset.Core/ExtensibilityServices/FileSystem.cs b/src/wix/WixToolset.Core/ExtensibilityServices/FileSystem.cs
index 8bda4966..4269cabe 100644
--- a/src/wix/WixToolset.Core/ExtensibilityServices/FileSystem.cs
+++ b/src/wix/WixToolset.Core/ExtensibilityServices/FileSystem.cs
@@ -12,13 +12,13 @@ namespace WixToolset.Core.ExtensibilityServices
     {
         public void CopyFile(string source, string destination, bool allowHardlink)
         {
-            EnsureDirectoryWithoutFile(destination);
+            this.EnsureDirectoryWithoutFile(destination);
 
             var hardlinked = false;
 
             if (allowHardlink)
             {
-                ActionWithRetries(() => hardlinked = CreateHardLink(destination, source, IntPtr.Zero));
+                this.ExecuteWithRetries(() => hardlinked = CreateHardLink(destination, source, IntPtr.Zero));
             }
 
             if (!hardlinked)
@@ -27,25 +27,30 @@ namespace WixToolset.Core.ExtensibilityServices
                 var er = Marshal.GetLastWin32Error();
 #endif
 
-                ActionWithRetries(() => File.Copy(source, destination, overwrite: true));
+                this.ExecuteWithRetries(() => File.Copy(source, destination, overwrite: true));
+            }
+        }
+
+        public void DeleteFile(string source, bool throwOnError = false, int maxRetries = 4)
+        {
+            try
+            {
+                this.ExecuteWithRetries(() => File.Delete(source), maxRetries);
+            }
+            catch when (!throwOnError)
+            {
+                // Do nothing on best-effort deletes.
             }
         }
 
         public void MoveFile(string source, string destination)
         {
-            EnsureDirectoryWithoutFile(destination);
+            this.EnsureDirectoryWithoutFile(destination);
 
-            ActionWithRetries(() => File.Move(source, destination));
+            this.ExecuteWithRetries(() => File.Move(source, destination));
         }
 
-        /// <summary>
-        /// Executes an action and retries on any exception up to a few times. Primarily
-        /// intended for use with file system operations that might get interrupted by
-        /// external systems (usually anti-virus).
-        /// </summary>
-        /// <param name="action">Action to execute.</param>
-        /// <param name="maxRetries">Maximum retry attempts.</param>
-        internal static void ActionWithRetries(Action action, int maxRetries = 3)
+        public void ExecuteWithRetries(Action action, int maxRetries = 4)
         {
             for (var attempt = 1; attempt <= maxRetries; ++attempt)
             {
@@ -61,16 +66,16 @@ namespace WixToolset.Core.ExtensibilityServices
             }
         }
 
-        private static void EnsureDirectoryWithoutFile(string path)
+        private void EnsureDirectoryWithoutFile(string path)
         {
             var directory = Path.GetDirectoryName(path);
 
             if (!String.IsNullOrEmpty(directory))
             {
-                ActionWithRetries(() => Directory.CreateDirectory(directory));
+                this.ExecuteWithRetries(() => Directory.CreateDirectory(directory));
             }
 
-            ActionWithRetries(() => File.Delete(path));
+            this.ExecuteWithRetries(() => File.Delete(path));
         }
 
         [DllImport("Kernel32.dll", CharSet = CharSet.Unicode, SetLastError = true)]
diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/BundleFixture.cs b/src/wix/test/WixToolsetTest.CoreIntegration/BundleFixture.cs
index ad31fc6d..cbfe6f85 100644
--- a/src/wix/test/WixToolsetTest.CoreIntegration/BundleFixture.cs
+++ b/src/wix/test/WixToolsetTest.CoreIntegration/BundleFixture.cs
@@ -428,7 +428,7 @@ namespace WixToolsetTest.CoreIntegration
                 var message = result.Messages.Where(m => m.Level == MessageLevel.Error).Select(m => m.ToString().Replace(folder, "<testdata>")).ToArray();
                 WixAssert.CompareLineByLine(new[]
                 {
-                    @"Failed to add resources to the bundle. Ensure the bundle icon file is an icon file at '<testdata>\.Data\burn.exe'"
+                    @"Failed to update resources in the bundle. Ensure the bundle icon file is an icon file at '<testdata>\.Data\burn.exe'. Detail: Failed to save resource. Error: 87"
                 }, message);
             }
         }