Catching exception inside model

Question

Is it normal to catch exception in concept of MVVM inside model? Or how should it be? How to improve this code?

This is simple application that allows user to drop .pfx certificate on the label. LoginModel checks the password for validity and check the publisher. if the validation fails throws an exception that is catching in the LoginViewModel;

Xaml:

<Grid>
    <Grid.RowDefinitions>
        <RowDefinition></RowDefinition>
        <RowDefinition></RowDefinition>
        <RowDefinition></RowDefinition>
    </Grid.RowDefinitions>
    <Grid.ColumnDefinitions>
        <ColumnDefinition></ColumnDefinition>
        <ColumnDefinition></ColumnDefinition>
    </Grid.ColumnDefinitions>
    <Label Grid.Row="0" Grid.Column="0" AllowDrop="True" Content="{Binding UserCertificateResult}">
        <i:Interaction.Triggers>
            <i:EventTrigger EventName="Drop">
                <Command:EventToCommand Command="{Binding DropUserCertificateCommand}" PassEventArgsToCommand="True"/>
            </i:EventTrigger>
        </i:Interaction.Triggers>
    </Label>
    <TextBox  Grid.Row="0" Grid.Column="1" Height="30" Text="{Binding UserPassword}"></TextBox>
    <Button Grid.Row="2" Height="40" Content="Connect" Command="{Binding CreateConnectionCommand}"></Button>
</Grid>

Code:

namespace WpfApplication31
{
    public partial class MainWindow : Window
    {
        public MainWindow()
        {
            InitializeComponent();
            DataContext = new LoginViewModel();
        }
    }

    public class LoginViewModel : ViewModelBase
    {
        LoginModel model = new LoginModel();

        private string userCertificateResult = "place user certificate";

        public string UserCertificateResult
        {
            get
            {
                return userCertificateResult;
            }

            set
            {
                userCertificateResult = value;
                RaisePropertyChanged("UserCertificateResult");
            }
        }

        public RelayCommand CreateConnectionCommand
        {
            get
            {
                return new RelayCommand(CreateConnection);
            }
        }

        public RelayCommand<DragEventArgs> DropUserCertificateCommand
        {
            get
            {
                return new RelayCommand<DragEventArgs>(CheckUserCertificate);
            }
        }

        private string userPassword = "123456";
        public string UserPassword
        {
            get
            {
                return userPassword;
            }

            set
            {
                userPassword = value;
            }
        }

        private X509Certificate2 UserCertificate;

        private void CheckUserCertificate(DragEventArgs args)
        {
            try
            {
                string userCertificatePath = string.Join("", (string[])args.Data.GetData(DataFormats.FileDrop, true));
                UserCertificate = model.IsUserCertificateValid(userCertificatePath, UserPassword);
                UserCertificateResult = "Succesfully";
            }
            catch (Exception ex)
            {
                UserCertificateResult = ex.Message.ToString();
            }
        }

        private void CreateConnection()
        {
            Service.Connect(UserCertificate);
        }
    }

    public class LoginModel
    {
        public X509Certificate2 IsUserCertificateValid(string userCertificatePath, string password)
        {
            X509Certificate2 userCertificate;
            try
            {
                userCertificate = new X509Certificate2(userCertificatePath, password);
                if (userCertificate.Issuer != "CN=GO Certification Authority, OU=Certification, O=Test, L=Test, S=Test, C=Test")
                {
                    throw new InvalidIssuerException("Wrong issuer");
                }
            }
            catch (CryptographicException ex)
            {
                throw new CryptographicException(ex.Message.ToString());
            }
            return userCertificate;
        }
    }

    public static class Service
    {
        public static void Connect(X509Certificate2 userCetificate)
        {
        }
    }

    public class InvalidIssuerException : Exception
    {
        public InvalidIssuerException(string message)
            : base(message)
        {
        }
    }
}

Answer 1

There is nothing inherently wrong with catching exceptions in viewmodel. The rule of the thumb is that you catch exceptions at level where you can handle them. Sometimes this level is viewmodel level.

That being said, exceptions should not be used to control flow. This topic is covered on the Internet pretty well. For example, check this discussion or this MSDN guideline. Exceptions should be used for handling "exceptional" situations. There is nothing exceptional about certificate having a different issuer from the one you need. On the contrary, I would assume, this should be a pretty common situation.

So, instead of throwing exceptions - return a bool value. Might look like this:

var certificate = model.LoadCertificate(userCertificatePath, UserPassword);
var isValid = model.IsCertificateValid(certificate);

or like this

var isValid  = model.TryLoadCertificate(userCertificatePath, UserPassword, out certificate);

or w/e really, there is plenty of options. Just don't throw exceptions.

Also this smells:

catch (CryptographicException ex)
{
    throw new CryptographicException(ex.Message.ToString());
}

What exactly are you doing it for? Just let the original exception through, don't catch it, if you can't do anything about it.

Answer 2

No. ViewModel - as the name itself indicates - should only model the view. Business logic, such as validating a certificate, should be extracted to another class (a service), adhering to the Single Responsibility Principle. This includes handling any errors that occur. Extract CheckUserCertificate elsewhere.