@joebigelow / wix / commits / bfec456a

Add retries when trying to update bundle resources

Fixes 6882 and 6902

Rob Mensching committed Oct 4, 2022 at 10:36 UTC bfec456aae2ad7abcec0f8c4c8b2f44308e48961
6 files changed +51 -33
src/api/wix/WixToolset.Extensibility/Services/IFileSystem.cs
+19
@@ -2,6 +2,8 @@
2
3 namespace WixToolset.Extensibility.Services
4 {
5 + using System;
6 +
7 /// <summary>
8 /// Abstracts basic file system operations.
9 /// </summary>
@@ -15,11 +17,28 @@ namespace WixToolset.Extensibility.Services
17 /// <param name="allowHardlink">Allow hardlinks.</param>
18 void CopyFile(string source, string destination, bool allowHardlink);
19
20 + /// <summary>
21 + /// Deletes a file.
22 + /// </summary>
23 + /// <param name="source">The file to delete.</param>
24 + /// <param name="throwOnError">Indicates the file must be deleted. Default is a best effort delete.</param>
25 + /// <param name="maxRetries">Maximum retry attempts. Default is 4.</param>
26 + void DeleteFile(string source, bool throwOnError = false, int maxRetries = 4);
27 +
28 /// <summary>
29 /// Moves a file.
30 /// </summary>
31 /// <param name="source">The file to move.</param>
32 /// <param name="destination">The destination file.</param>
33 void MoveFile(string source, string destination);
34 +
35 + /// <summary>
36 + /// Executes an action and retries on any exception a few times with short pause
37 + /// between each attempt. Primarily intended for use with file system operations
38 + /// that might get interrupted by external systems (usually anti-virus).
39 + /// </summary>
40 + /// <param name="action">Action to execute.</param>
41 + /// <param name="maxRetries">Maximum retry attempts. Default is 4.</param>
42 + void ExecuteWithRetries(Action action, int maxRetries = 4);
43 }
44 }
src/wix/WixToolset.Core.Burn/Bundles/CreateBundleExeCommand.cs
+3 -9
@@ -349,17 +349,11 @@ namespace WixToolset.Core.Burn.Bundles
349
350 try
351 {
352 - resources.Save(bundleTempPath);
352 + this.FileSystem.ExecuteWithRetries(() => resources.Save(bundleTempPath));
353 }
354 - catch (IOException)
354 + catch (IOException e)
355 {
356 - // If there was no icon or splash screen then this is unexpected so rethrow.
357 - if (bundleInfo.IconSourceFile == null && bundleInfo.SplashScreenSourceFile == null)
358 - {
359 - throw;
360 - }
361 -
362 - this.Messaging.Write(BurnBackendErrors.FailedToAddIconOrSplashScreenToBundle(bundleInfo.SourceLineNumbers, bundleInfo.IconSourceFile?.Path, bundleInfo.SplashScreenSourceFile?.Path));
356 + this.Messaging.Write(BurnBackendErrors.FailedToUpdateBundleResources(bundleInfo.SourceLineNumbers, bundleInfo.IconSourceFile?.Path, bundleInfo.SplashScreenSourceFile?.Path, e.Message));
357 }
358 }
359
src/wix/WixToolset.Core.Burn/BurnBackendErrors.cs
+6 -6
@@ -42,7 +42,7 @@ namespace WixToolset.Core.Burn
42 return Message(sourceLineNumbers, Ids.ExternalPayloadCollision2, "The location of the symbol related to the previous error.");
43 }
44
45 - public static Message FailedToAddIconOrSplashScreenToBundle(SourceLineNumber sourceLineNumbers, string iconPath, string splashScreenPath)
45 + public static Message FailedToUpdateBundleResources(SourceLineNumber sourceLineNumbers, string iconPath, string splashScreenPath, string detail)
46 {
47 var additionalDetail = String.Empty;
48
@@ -51,18 +51,18 @@ namespace WixToolset.Core.Burn
51 }
52 else if (String.IsNullOrEmpty(iconPath))
53 {
54 - additionalDetail = $" Ensure the splash screen file is a bitmap file at '{splashScreenPath}'";
54 + additionalDetail = $" Ensure the splash screen file is a bitmap file at '{splashScreenPath}'.";
55 }
56 else if (String.IsNullOrEmpty(splashScreenPath))
57 {
58 - additionalDetail = $" Ensure the bundle icon file is an icon file at '{iconPath}'";
58 + additionalDetail = $" Ensure the bundle icon file is an icon file at '{iconPath}'.";
59 }
60 else
61 {
62 - additionalDetail = $" Ensure the bundle icon file is an icon file at '{iconPath}' and the splash screen file is a bitmap file at '{splashScreenPath}'";
62 + additionalDetail = $" Ensure the bundle icon file is an icon file at '{iconPath}' and the splash screen file is a bitmap file at '{splashScreenPath}'.";
63 }
64
65 - return Message(sourceLineNumbers, Ids.FailedToAddIconOrSplashScreenToBundle, "Failed to add resources to the bundle.{0}", additionalDetail);
65 + return Message(sourceLineNumbers, Ids.FailedToUpdateBundleResources, "Failed to update resources in the bundle.{0} Detail: {1}", additionalDetail, detail);
66 }
67
68 public static Message PackageCachePayloadCollision(SourceLineNumber sourceLineNumbers, string payloadId, string payloadName, string packageId)
@@ -113,7 +113,7 @@ namespace WixToolset.Core.Burn
113 TooManyAttachedContainers = 8008,
114 IncompatibleWixBurnSection = 8009,
115 UnsupportedRemotePackagePayload = 8010,
116 - FailedToAddIconOrSplashScreenToBundle = 8011,
116 + FailedToUpdateBundleResources = 8011,
117 InvalidBundleManifest = 8012,
118 BundleMultipleProviders = 8013,
119 } // last available is 8499. 8500 is BurnBackendWarnings.
src/wix/WixToolset.Core/Bind/TransferFilesCommand.cs
+1 -1
@@ -204,7 +204,7 @@ namespace WixToolset.Core.Bind
204 foreach (var file in files)
205 {
206 var fileInfo = new FileInfo(file);
207 - ExtensibilityServices.FileSystem.ActionWithRetries(() => fileInfo.SetAccessControl(aclReset));
207 + this.FileSystem.ExecuteWithRetries(() => fileInfo.SetAccessControl(aclReset));
208 }
209 }
210 }
src/wix/WixToolset.Core/ExtensibilityServices/FileSystem.cs
+21 -16
@@ -12,13 +12,13 @@ namespace WixToolset.Core.ExtensibilityServices
12 {
13 public void CopyFile(string source, string destination, bool allowHardlink)
14 {
15 - EnsureDirectoryWithoutFile(destination);
15 + this.EnsureDirectoryWithoutFile(destination);
16
17 var hardlinked = false;
18
19 if (allowHardlink)
20 {
21 - ActionWithRetries(() => hardlinked = CreateHardLink(destination, source, IntPtr.Zero));
21 + this.ExecuteWithRetries(() => hardlinked = CreateHardLink(destination, source, IntPtr.Zero));
22 }
23
24 if (!hardlinked)
@@ -27,25 +27,30 @@ namespace WixToolset.Core.ExtensibilityServices
27 var er = Marshal.GetLastWin32Error();
28 #endif
29
30 - ActionWithRetries(() => File.Copy(source, destination, overwrite: true));
30 + this.ExecuteWithRetries(() => File.Copy(source, destination, overwrite: true));
31 + }
32 + }
33 +
34 + public void DeleteFile(string source, bool throwOnError = false, int maxRetries = 4)
35 + {
36 + try
37 + {
38 + this.ExecuteWithRetries(() => File.Delete(source), maxRetries);
39 + }
40 + catch when (!throwOnError)
41 + {
42 + // Do nothing on best-effort deletes.
43 }
44 }
45
46 public void MoveFile(string source, string destination)
47 {
36 - EnsureDirectoryWithoutFile(destination);
48 + this.EnsureDirectoryWithoutFile(destination);
49
38 - ActionWithRetries(() => File.Move(source, destination));
50 + this.ExecuteWithRetries(() => File.Move(source, destination));
51 }
52
41 - /// <summary>
42 - /// Executes an action and retries on any exception up to a few times. Primarily
43 - /// intended for use with file system operations that might get interrupted by
44 - /// external systems (usually anti-virus).
45 - /// </summary>
46 - /// <param name="action">Action to execute.</param>
47 - /// <param name="maxRetries">Maximum retry attempts.</param>
48 - internal static void ActionWithRetries(Action action, int maxRetries = 3)
53 + public void ExecuteWithRetries(Action action, int maxRetries = 4)
54 {
55 for (var attempt = 1; attempt <= maxRetries; ++attempt)
56 {
@@ -61,16 +66,16 @@ namespace WixToolset.Core.ExtensibilityServices
66 }
67 }
68
64 - private static void EnsureDirectoryWithoutFile(string path)
69 + private void EnsureDirectoryWithoutFile(string path)
70 {
71 var directory = Path.GetDirectoryName(path);
72
73 if (!String.IsNullOrEmpty(directory))
74 {
70 - ActionWithRetries(() => Directory.CreateDirectory(directory));
75 + this.ExecuteWithRetries(() => Directory.CreateDirectory(directory));
76 }
77
73 - ActionWithRetries(() => File.Delete(path));
78 + this.ExecuteWithRetries(() => File.Delete(path));
79 }
80
81 [DllImport("Kernel32.dll", CharSet = CharSet.Unicode, SetLastError = true)]
src/wix/test/WixToolsetTest.CoreIntegration/BundleFixture.cs
+1 -1
@@ -428,7 +428,7 @@ namespace WixToolsetTest.CoreIntegration
428 var message = result.Messages.Where(m => m.Level == MessageLevel.Error).Select(m => m.ToString().Replace(folder, "<testdata>")).ToArray();
429 WixAssert.CompareLineByLine(new[]
430 {
431 - @"Failed to add resources to the bundle. Ensure the bundle icon file is an icon file at '<testdata>\.Data\burn.exe'"
431 + @"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"
432 }, message);
433 }
434 }