Exceptions vs User Messages

[sjb-sjb]

I am a fan of NotifyTask because it clarifies the use of async and how async methods capture exceptions in the returned Task.

One thing I feel could be improved in NotifyTask is to recognize that not all exceptions are the same. Some exceptions indicate a system problem (e.g. file system error, network down, etc), while others indicate a program or programmer error (e.g. ArgumentException).

NotifyTask promotes the use of exceptions for user message by providing the bindable ErrorMessage property. It is not that common to use exceptions for user messages because (a) exceptions frequently are not localized or are in other ways not user-friendly and (b) user messages are considered to be "normal" rather than "exceptional" and (c) if you use exceptions for user messages then you have to have a way of distinguishing user-message exceptions from system and programmer errors.

If a system or programmer error occurs, then I want NotifyTask to propagate the exception so that I am forced to deal with the exception. On the other hand if the exception represents a user message then I want NotifyTask to capture the exception and put it into ErrorMessage so that I can show it at a convenient time / place in the user interface.

This is quite easily accomplished by adding enum NotifyTaskExceptions { RethrowAll, RethrowNone, RethrowAllButUserInformationExceptions, RethrowUserInformationExceptionsOnly } and then using it in MonitorTaskAsync to test whether the exception should be rethrown. This provides flexibility with respect to either propagating or capturing exceptions. I am attaching the revised code.

In general, however, my suggestion is that async user notifications should be managed without the use of exceptions, because this avoids the issues (a, b, c) above. The worker/async function in the Model or in a service should be able to provide a progress report and this should then result in PropertyChanged on an object in the ViewModel. One way to handle this, promoted for example in MvvmLight, is to use a Messenger service: post a message from the Model or from the service, and have the ViewModel or the View listen for and apply the message. I am not a fan of this approach because is requires creating a new infrastructure/paradigm of messaging, and because messaging does not by itself directly solve the problem of getting status information from the Model or service to a PropertyChanged event. In addition this approach creates the problem of how to ensure that messages notifying the start of work are always matched by messages notifying the end of work.

My suggestion in this regard is to have a ProgressReport class which is handed in to the worker/async function, much in the same way that a CancellationToken is handed in. ProgressReport should implement INotifyPropertyChanged. By updating the ProgressReport, the worker/async function provides status information, which is then translated into PropertyChanged events by the ProgressReport. Obviously one has to look out for thread safety and for the fact that the PropertyChanged notification may need to be marshalled onto the UI thread. I am attaching a ProgressReport implementation which does this. The fields in the ProgressReport are designed to be convenient for the ProgressBar and ProgressRing UI controls. There is also an Activate function designed to be used in a "using" block so that the worker/async function definitely notifies when work is complete.

ProgressReport uses a NotifyBase class which takes care of marshalling the PropertyChanged onto the UI thread. There is also an additional Property class in there which lets you use lambda notation for property names -- so that the compiler checks the property names -- I got that part off a blog somewhere, unfortunately I forget the reference.

I am aware of IProgress, however this is too limited to address the issues above and also is not well suited to ProgressRing and ProgressBar.

I am hereby entering the attached code into the public domain for use by anyone without restriction.

NotifyTask.txt ProgressReport.txt NotifyBase.txt Property.txt

Hmmm, forgot this:

public partial class UserInformationException: Exception
{
    public UserInformationException() { }

    public UserInformationException(string message) : base(message) { }

    public UserInformationException(string message, Exception innerException) : base(message, innerException) { }
}

}

[StephenCleary]

The purpose of NotifyTask is that it is a task wrapper that exposes the completion of that task via data-bindable properties.

While I agree that showing exception messages to the user is not good UX, the NotifyTask type merely allows that. I envision that normal usage would be to data-bind a generic error message to IsFaulted. However, some people do like to show the user an actual error message (e.g., if the target audience is technical), and ErrorMessage is provided for that purpose.

I also agree that developer errors (boneheaded exceptions) should not be shown to the user, and should be "in the face" of the developer. However, there's not an easy way to determine which exceptions are boneheaded. I'm not going down the road of differing debug/release behavior, and I'm not in a position to change the C# language (or introduce base exception types and expect them to be used system-wide), so from the perspective of this library, the best approach is to treat all exceptions the same, and allow the developer to handle them through data binding. (During debugging, of course, devs can see exceptions when thrown if they choose to, which is far better than the alternative of crashing at runtime if one slips through).

Also, I hate flags.

I think the best approach for end-user messages is to either just update the data-bound property directly, or (if it's an operation-ending error), for the application to define an "end-user message exception". I am also not a fan of message busses; they become unweildly as the application grows.

Regarding progress reports, I don't see why Progress<T> wouldn't work. It can use any T and marshals appropriately. I do have a data-bindable IProgress<T> that I'm planning to add to this library, as well as a rate-limiting IProgress<T> that will probably go into a new library (since it brings in Rx).

That said, a ProgressReport such as you provide is certainly a solution; it's just one that I feel is more appropriate at the application level rather than as part of Mvvm.Async.

[sjb-sjb]

Since I'm writing the application, I don't mind defining the 'end user message exception'. Again, as a framework designer you may want to be more conservative. Actually I don't use the exception approach for messages though. If I were starting from scratch I would just skip the 'end user message exception' idea and just have all exceptions propagated out from NotifyTask. So, I put it in there for you :-)

I take your point on binding a localized message to IsFaulted. I overlooked this when I put together my code. I still prefer the ProgressReport approach, because I can design it to fit nicely with UWP progress reporting controls and because customizing the message is straightforward.

About IProgress, as I commented in my other reply, I feel that this interface should not be perpetuated. It doesn't integrate with property binding and property notification -- the same problem that CanExecute suffers from. I haven't seen your bindable version yet. I would assume that it rights this wrong, but then again, why keep the wrong approach mixed in there alongside the right approach?

All the best, Steve

[StephenCleary]

why keep the wrong approach mixed in there alongside the right approach?

I wouldn't say "right" or "wrong" there.

IProgress<T> is standard. It's also very generic, usable by any type of application. For example, SignalR uses it to send messages over HTTP.

Progress<T> is used by UI apps, but any kind of UI app. It's used by WinForms, etc, where INotifyPropertyChanged simply isn't understood by anything else. For this reason, Progress<T> is more of a simple callback rather than something MVVM-specific.

There's a few important use cases missing here: free-threaded apps (e.g., Console) don't play nicely with Progress<T>; MVVM apps could use more support (specifically, INotifyPropertyChanged); and all UI apps could use progress throttling.

From my perspective, I wouldn't say that Progress<T> is wrong - it's just one implementation for a certain set of uses, and the IProgress<T> concept can easily be extended to cover additional uses (including INotifyPropertyChanged).