Breaking change: reworked how errors and async operations are handled

The UpdateManager methods will now throw for all errors, instead of returning ambigous return values. You should wrap all calls to them in try/catch blocks and handle those exceptions.

The Async versions of the UpdateManager methods have been renamed, and now use the BeginFoo/EndFoo scheme, as per the CLR APM. Calling them will give you an object implementing IAsyncResult with which you can poll or block. You can still pass a callback method to be called when done, only now it will have much more info on the process, and can throw if an exception is present.

src/NAppUpdate.Framework/Common/Exceptions.cs

@@ -29,4 +29,12 @@ public FeedReaderException(string message) : base(message) { }
 		public FeedReaderException(string message, Exception ex) : base(message, ex) { }
 		public FeedReaderException(SerializationInfo info, StreamingContext context) : base(info, context) { }
 	}
+
+	[Serializable]
+	public class UserAbortException : NAppUpdateException
+	{
+		public UserAbortException() : base("User abort")
+		{
+		}
+	}
 }

src/NAppUpdate.Framework/Common/UpdateProcessAsyncResult.cs

@@ -0,0 +1,106 @@
+using System;
+using System.Threading;
+
+namespace NAppUpdate.Framework.Common
+{
+	public class UpdateProcessAsyncResult : IAsyncResult
+	{
+		private readonly AsyncCallback _asyncCallback;
+		private readonly Object _asyncState;
+
+		private const Int32 StatePending = 0;
+		private const Int32 StateCompletedSynchronously = 1;
+		private const Int32 StateCompletedAsynchronously = 2;
+		private Int32 _completedState = StatePending;
+
+		private ManualResetEvent _asyncWaitHandle;
+		private Exception _exception;
+
+		public UpdateProcessAsyncResult(AsyncCallback asyncCallback, Object state)
+		{
+			_asyncCallback = asyncCallback;
+			_asyncState = state;
+		}
+
+		public void SetAsCompleted(Exception exception, Boolean completedSynchronously)
+		{
+			// Passing null for exception means no error occurred. 
+			// This is the common case
+			_exception = exception;
+
+			// The m_CompletedState field MUST be set prior calling the callback
+			Int32 prevState = Interlocked.Exchange(ref _completedState,
+			   completedSynchronously ? StateCompletedSynchronously : StateCompletedAsynchronously);
+			if (prevState != StatePending)
+				throw new InvalidOperationException("You can set a result only once");
+
+			// If the event exists, set it
+			if (_asyncWaitHandle != null) _asyncWaitHandle.Set();
+
+			// If a callback method was set, call it
+			if (_asyncCallback != null) _asyncCallback(this);
+		}
+
+		public void EndInvoke()
+		{
+			// This method assumes that only 1 thread calls EndInvoke 
+			// for this object
+			if (!IsCompleted)
+			{
+				// If the operation isn't done, wait for it
+				AsyncWaitHandle.WaitOne();
+				AsyncWaitHandle.Close();
+				_asyncWaitHandle = null;  // Allow early GC
+			}
+
+			// Operation is done: if an exception occured, throw it
+			if (_exception != null) throw _exception;
+		}
+
+		public bool IsCompleted
+		{
+			get
+			{
+				return Thread.VolatileRead(ref _completedState) != StatePending;
+			}
+		}
+
+		public WaitHandle AsyncWaitHandle
+		{
+			get
+			{
+				if (_asyncWaitHandle == null)
+				{
+					bool done = IsCompleted;
+					var mre = new ManualResetEvent(done);
+					if (Interlocked.CompareExchange(ref _asyncWaitHandle, mre, null) != null)
+					{
+						// Another thread created this object's event; dispose 
+						// the event we just created
+						mre.Close();
+					}
+					else
+					{
+						if (!done && IsCompleted)
+						{
+							// If the operation wasn't done when we created 
+							// the event but now it is done, set the event
+							_asyncWaitHandle.Set();
+						}
+					}
+				}
+				return _asyncWaitHandle;
+			}
+		}
+
+		public Object AsyncState { get { return _asyncState; } }
+
+		public Boolean CompletedSynchronously
+		{
+			get
+			{
+				return Thread.VolatileRead(ref _completedState) == StateCompletedSynchronously;
+			}
+		}
+	}
+}

src/NAppUpdate.Framework/NAppUpdate.Framework.csproj

@@ -60,10 +60,10 @@
     <Reference Include="System.Xml" />
   </ItemGroup>
   <ItemGroup>
-    <Compile Include="Common\Errors.cs" />
     <Compile Include="Common\INauFieldsHolder.cs" />
     <Compile Include="Common\NauConfigurations.cs" />
     <Compile Include="Common\NauFieldAttribute.cs" />
+    <Compile Include="Common\UpdateProcessAsyncResult.cs" />
     <Compile Include="Common\UpdateProgressInfo.cs" />
     <Compile Include="Common\WorkScope.cs" />
     <Compile Include="Conditions\FileChecksumCondition.cs" />

src/NAppUpdate.Framework/Tasks/FileUpdateTask.cs

@@ -97,8 +97,8 @@ public override TaskExecutionStatus Execute(bool coldRun)
 				{
 					if (coldRun)
 					{
-						// TODO: log error
-						return TaskExecutionStatus.Failed;
+						ExecutionStatus = TaskExecutionStatus.Failed;
+						throw new UpdateProcessFailedException("Could not replace the file", ex);
 					}
 
 					// Failed hot swap file tasks should now downgrade to cold tasks automatically

src/NAppUpdate.Framework/Tasks/RegistryTask.cs

@@ -5,81 +5,79 @@
 namespace NAppUpdate.Framework.Tasks
 {
 	[Serializable]
-    [UpdateTaskAlias("registryUpdate")]
+	[UpdateTaskAlias("registryUpdate")]
 	public class RegistryTask : UpdateTaskBase
-    {
-        [NauField("keyName", "The full path to the registry key", true)]
-        public string KeyName { get; set; }
+	{
+		[NauField("keyName", "The full path to the registry key", true)]
+		public string KeyName { get; set; }
 
-        [NauField("keyValue", "The value name to set", true)]
-        public string KeyValueName { get; set; }
+		[NauField("keyValue", "The value name to set", true)]
+		public string KeyValueName { get; set; }
 
-        [NauField("valueKind",
-            "Value type; choose one and then set only one value field (leave all blank to remove the key)"
-            , true)]
-        public RegistryValueKind ValueKind { get; set; }
+		[NauField("valueKind",
+			"Value type; choose one and then set only one value field (leave all blank to remove the key)"
+			, true)]
+		public RegistryValueKind ValueKind { get; set; }
 
-        [NauField("value", "A String value to set", false)]
-        public string StringValue { get; set; }
+		[NauField("value", "A String value to set", false)]
+		public string StringValue { get; set; }
 
-        [NauField("value", "A DWord value to set", false)]
-        public Int32? DWordValue { get; set; }
+		[NauField("value", "A DWord value to set", false)]
+		public Int32? DWordValue { get; set; }
 
-        [NauField("value", "A QWord value to set", false)]
-        public Int64? QWordValue { get; set; }
+		[NauField("value", "A QWord value to set", false)]
+		public Int64? QWordValue { get; set; }
 
-        // Get the first non-null value
-        protected object ValueToSet
-        {
-            get
-            {
-                if (StringValue != null)
-                    return StringValue;
-                if (DWordValue != null)
-                    return DWordValue;
-                if (QWordValue != null)
-                    return QWordValue;
-                return null;
-            }
-        }
-        private object originalValue;
+		// Get the first non-null value
+		protected object ValueToSet
+		{
+			get
+			{
+				if (StringValue != null)
+					return StringValue;
+				if (DWordValue != null)
+					return DWordValue;
+				if (QWordValue != null)
+					return QWordValue;
+				return null;
+			}
+		}
+		private object originalValue;
 
-    	public override bool Prepare(Sources.IUpdateSource source)
-        {
-            // No preparation required
-            return true;
-        }
+		public override bool Prepare(Sources.IUpdateSource source)
+		{
+			// No preparation required
+			return true;
+		}
 
 		public override TaskExecutionStatus Execute(bool coldRun /* unused */)
-        {
-            if (String.IsNullOrEmpty(KeyName) || String.IsNullOrEmpty(KeyValueName))
+		{
+			if (String.IsNullOrEmpty(KeyName) || String.IsNullOrEmpty(KeyValueName))
 				return ExecutionStatus = TaskExecutionStatus.Successful;
 
-            try
-            {
-                // Get the current value and store in case we need to rollback
-                // This is also used to prematurely detect incorrect key and value paths
-                originalValue = Registry.GetValue(KeyName, KeyValueName, null);
-            }
-            catch { return ExecutionStatus = TaskExecutionStatus.Failed; }
 
-            try
-            {
-                Registry.SetValue(KeyName, KeyValueName, ValueToSet, ValueKind);
-            }
-			catch { return ExecutionStatus = TaskExecutionStatus.Failed; }
+			// Get the current value and store in case we need to rollback
+			// This is also used to prematurely detect incorrect key and value paths
+			// Any exception thrown in this stage would just keep this task in Pending state
+			originalValue = Registry.GetValue(KeyName, KeyValueName, null);
+
+			try
+			{
+				Registry.SetValue(KeyName, KeyValueName, ValueToSet, ValueKind);
+			}
+			catch (Exception ex)
+			{
+				ExecutionStatus = TaskExecutionStatus.Failed;
+				throw new UpdateProcessFailedException("Error while trying to set the new registry key value", ex);
+			}
 
 			return ExecutionStatus = TaskExecutionStatus.Successful;
-        }
+		}
 
-        public override bool Rollback()
-        {
-            try
-            {
-                Registry.SetValue(KeyName, KeyValueName, originalValue);
-            }
-            catch { return false; }
-            return true;
-        }
-    }
+		public override bool Rollback()
+		{
+			Registry.SetValue(KeyName, KeyValueName, originalValue);
+			return true;
+		}
+	}
 }