Bhav
Bhav

Reputation: 2185

ASP.Net C# - Catching errors when sending an email

So I've got a function to send an email which I run within a button click event after some validation. However, how do I catch any errors when sending an email? I'm familiar with the try-catch-finally approach so would the code at the bottom be sufficient?

protected void sendEmail(string activationCode, string username, string emailAddress)
{
    SmtpClient SmtpServer = new SmtpClient("smtp.live.com");
    var mail = new MailMessage();
    mail.From = new MailAddress("EMAIL");
    mail.To.Add(emailAddress);
    mail.Subject = "Please activate your account.";
    mail.IsBodyHtml = true;
    string htmlBody;
    htmlBody = "Dear " + username + "<br /><br />";
    htmlBody += "Thank you for registering an account.  Please activate your account by visiting the URL below:<br /><br />";
    htmlBody += "http://localhost:57167/signin.aspx?activate=" + activationCode + "<br /><br />";
    htmlBody += "Thank you.";
    mail.Body = htmlBody;
    SmtpServer.Port = 587;
    SmtpServer.UseDefaultCredentials = false;
    SmtpServer.Credentials = new System.Net.NetworkCredential("EMAIL", "PASSWORD");
    SmtpServer.EnableSsl = true;
    SmtpServer.Send(mail);
}

Is something like this okay?

protected void sendEmail(string activationCode, string username, string emailAddress)
{
    try
    {
    SmtpClient SmtpServer = new SmtpClient("smtp.live.com");
    var mail = new MailMessage();
    mail.From = new MailAddress("EMAIL");
    mail.To.Add(emailAddress);
    mail.Subject = "Please activate your account.";
    mail.IsBodyHtml = true;
    string htmlBody;
    htmlBody = "Dear " + username + "<br /><br />";
    htmlBody += "Thank you for registering an account.  Please activate your account by visiting the URL below:<br /><br />";
    htmlBody += "http://localhost:57167/signin.aspx?activate=" + activationCode + "<br /><br />";
    htmlBody += "Thank you.";
    mail.Body = htmlBody;
    SmtpServer.Port = 587;
    SmtpServer.UseDefaultCredentials = false;
    SmtpServer.Credentials = new System.Net.NetworkCredential("EMAIL", "PASSWORD");
    SmtpServer.EnableSsl = true;
    SmtpServer.Send(mail);
    }
    catch
    {
        lblError.Text = "error message";
    }
}
    }

Upvotes: 0

Views: 11512

Answers (4)

Shibasis Sengupta
Shibasis Sengupta

Reputation: 649

The answer by zaitsman is what I would do generally. On top of that, if its a requirement for you to make a non- blocking call to send the email (so that rest of the application does not wait for the email to deliver and can carry on with rest of the work) you should use TPL and make a non -blocking call to the email sender. Something like this -

 Task emailTask = Task.Factory.StartNew(() => dispatchEmail();

Upvotes: 0

Matthijs
Matthijs

Reputation: 3170

I'd change some things. See code below:

using(SmtpClient SmtpServer = new SmtpClient("smtp.live.com"))
{
    var mail = new MailMessage();
    mail.From = new MailAddress("EMAIL");
    mail.To.Add(emailAddress);
    mail.Subject = "Please activate your account.";
    mail.IsBodyHtml = true;
    string htmlBody;
    htmlBody = "Dear " + username + "<br /><br />";
    htmlBody += "Thank you for registering an account.  Please activate your account by visiting the URL below:<br /><br />";
    htmlBody += "http://localhost:57167/signin.aspx?activate=" + activationCode + "<br /><br />";
    htmlBody += "Thank you.";
    mail.Body = htmlBody;
    SmtpServer.Port = 587;
    SmtpServer.UseDefaultCredentials = false;
    SmtpServer.Credentials = new System.Net.NetworkCredential("EMAIL", "PASSWORD");
    SmtpServer.EnableSsl = true;
    try
    {
        SmtpServer.Send(mail);
    }
    catch(Exception ex)
    {
        lblError.Text = ex.Message;
    }
}
  1. Using statement to dispose of the SmtpClient correctly, whether it fails or not.
  2. Move the assignments outside of the try-catch.
  3. Even though I am against catching general exceptions (you should know what kind of exceptions to expect); use ex.Message to display relevant exception-information.

Upvotes: 1

Rohit
Rohit

Reputation: 10226

You should handle the exception using SmtpException Class by accessing its statuscode property.

Also put your Try blocks only where you think exception's may occur.

        try
        {
            SmtpServer.Send(mail);
        }
        catch (SmtpFailedRecipientsException ex)
        {

        }

Upvotes: 3

zaitsman
zaitsman

Reputation: 9499

Yes, that would be more or less sufficient (depending on what you're trying to do).

The best approach would be to leave the current method as is (first listing), then have another one a la:

public void dispatchEmail(string activationCode, string username, string emailAddress)
{
  try
  {
      this.sendEmail(activationCode, username, emailAddress);
  }
  catch (Exception e) // Note: This is considered bad practice, might want to check for specific exceptions
  {
   lblError.Text = "error message: " + e.ToString();
  }
}

Upvotes: 1

Related Questions