@joebigelow / wix-1 / commits / 585086c7

Prefer logging messages to throwing out of the Validator

Exceptions could cross process boundaries given the callback nature of validation so try to write error messages as soon as errors happen instead of throwing.

Rob Mensching committed Feb 8, 2022 at 12:13 UTC 585086c7f2a1f5f41e9b4d948e968906ef890ed8
3 files changed +38 -26
src/wix/WixToolset.Core.Native/IWindowsInstallerValidatorCallback.cs
+8 -5
@@ -2,6 +2,8 @@
2
3 namespace WixToolset.Core.Native
4 {
5 + using WixToolset.Data;
6 +
7 /// <summary>
8 /// Callbacks during validation.
9 /// </summary>
@@ -12,16 +14,17 @@ namespace WixToolset.Core.Native
14 /// </summary>
15 bool EncounteredError { get; }
16
15 - /// <summary>
16 - /// Validation blocked by another Windows Installer operation.
17 - /// </summary>
18 - void ValidationBlocked();
19 -
17 /// <summary>
18 /// Validation message from an ICE.
19 /// </summary>
20 /// <param name="message">The validation message.</param>
21 /// <returns>True if validation should continue; otherwise cancel the validation.</returns>
22 bool ValidationMessage(ValidationMessage message);
23 +
24 + /// <summary>
25 + /// Normal message encountered while preparing for ICE validation.
26 + /// </summary>
27 + /// <param name="message">The message to write.</param>
28 + void WriteMessage(Message message);
29 }
30 }
src/wix/WixToolset.Core.Native/WindowsInstallerValidator.cs
+23 -14
@@ -65,7 +65,7 @@ namespace WixToolset.Core.Native
65 {
66 if (!mutex.WaitOne(0))
67 {
68 - this.Callback.ValidationBlocked();
68 + this.Callback.WriteMessage(VerboseMessages.ValidationSerialized());
69 mutex.WaitOne();
70 }
71 }
@@ -124,7 +124,8 @@ namespace WixToolset.Core.Native
124
125 if (!findCubeFile.Found)
126 {
127 - throw new WixException(ErrorMessages.CubeFileNotFound(findCubeFile.Path));
127 + this.Callback.WriteMessage(ErrorMessages.CubeFileNotFound(findCubeFile.Path));
128 + continue;
129 }
130
131 try
@@ -145,10 +146,12 @@ namespace WixToolset.Core.Native
146 {
147 if (0x6E == e.NativeErrorCode) // ERROR_OPEN_FAILED
148 {
148 - throw new WixException(ErrorMessages.CubeFileNotFound(findCubeFile.Path));
149 + this.Callback.WriteMessage(ErrorMessages.CubeFileNotFound(findCubeFile.Path));
150 + }
151 + else
152 + {
153 + this.Callback.WriteMessage(ErrorMessages.UnexpectedException($"Unexpected exception while merging CUB: {findCubeFile.Path}, detail: {e.Message}", e.GetType().ToString(), e.StackTrace));
154 }
150 -
151 - throw;
155 }
156 }
157
@@ -212,7 +215,7 @@ namespace WixToolset.Core.Native
215 {
216 if (!this.Callback.EncounteredError)
217 {
215 - throw e;
218 + this.Callback.WriteMessage(ErrorMessages.UnexpectedException($"Unexpected exception while executing ICE: {action}, detail: {e.Message}", e.GetType().ToString(), e.StackTrace));
219 }
220 }
221
@@ -236,29 +239,29 @@ namespace WixToolset.Core.Native
239 // this would be the temporary copy and there would be no final output becasue
240 // this error occured; and during standalone validation they should know the path
241 // passed in.
239 - throw new WixException(ErrorMessages.ValidationFailedToOpenDatabase());
242 + this.Callback.WriteMessage(ErrorMessages.ValidationFailedToOpenDatabase());
243 }
244 else if (0x64D == e.NativeErrorCode)
245 {
243 - throw new WixException(ErrorMessages.ValidationFailedDueToLowMsiEngine());
246 + this.Callback.WriteMessage(ErrorMessages.ValidationFailedDueToLowMsiEngine());
247 }
248 else if (0x654 == e.NativeErrorCode)
249 {
247 - throw new WixException(ErrorMessages.ValidationFailedDueToInvalidPackage());
250 + this.Callback.WriteMessage(ErrorMessages.ValidationFailedDueToInvalidPackage());
251 }
252 else if (0x658 == e.NativeErrorCode)
253 {
251 - throw new WixException(ErrorMessages.ValidationFailedDueToMultilanguageMergeModule());
254 + this.Callback.WriteMessage(ErrorMessages.ValidationFailedDueToMultilanguageMergeModule());
255 }
256 else if (0x659 == e.NativeErrorCode)
257 {
255 - throw new WixException(WarningMessages.ValidationFailedDueToSystemPolicy());
258 + this.Callback.WriteMessage(WarningMessages.ValidationFailedDueToSystemPolicy());
259 }
260 else
261 {
262 var msg = String.IsNullOrEmpty(this.CurrentIce) ? e.Message : $"Action - '{this.CurrentIce}' {e.Message}";
263
261 - throw new WixException(ErrorMessages.Win32Exception(e.NativeErrorCode, msg));
264 + this.Callback.WriteMessage(ErrorMessages.Win32Exception(e.NativeErrorCode, msg));
265 }
266 }
267 }
@@ -294,9 +297,15 @@ namespace WixToolset.Core.Native
297
298 continueValidation = this.Callback.ValidationMessage(parsedMessage);
299 }
297 - catch
300 + catch (WixException e)
301 + {
302 + this.Callback.WriteMessage(e.Error);
303 + return -1;
304 + }
305 + catch (Exception e)
306 {
299 - return - 1;
307 + this.Callback.WriteMessage(ErrorMessages.UnexpectedException($"Unexpected exception while executing action: {this.CurrentIce}, detail: {e.Message}", e.GetType().ToString(), e.StackTrace));
308 + return -1;
309 }
310 }
311
src/wix/WixToolset.Core.WindowsInstaller/Validate/ValidateDatabaseCommand.cs
+7 -7
@@ -117,20 +117,20 @@ namespace WixToolset.Core.WindowsInstaller.Validate
117 }
118
119 /// <summary>
120 - /// Validation blocked by other installation operation for <see cref="IWindowsInstallerValidatorCallback"/>.
120 + /// Validation message implementation for <see cref="IWindowsInstallerValidatorCallback"/>.
121 /// </summary>
122 - public void ValidationBlocked()
122 + public bool ValidationMessage(ValidationMessage message)
123 {
124 - this.Messaging.Write(VerboseMessages.ValidationSerialized());
124 + this.LogValidationMessage(message);
125 + return true;
126 }
127
128 /// <summary>
128 - /// Validation message implementation for <see cref="IWindowsInstallerValidatorCallback"/>.
129 + /// Normal message encountered while preparing for ICE validation for <see cref="IWindowsInstallerValidatorCallback"/>.
130 /// </summary>
130 - public bool ValidationMessage(ValidationMessage message)
131 + public void WriteMessage(Message message)
132 {
132 - this.LogValidationMessage(message);
133 - return true;
133 + this.Messaging.Write(message);
134 }
135
136 /// <summary>