Reputation: 2185
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
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
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;
}
}
ex.Message
to display relevant exception-information.Upvotes: 1
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
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