I want to redirect to action after doing some tasks (sending an email), but I can't figure-out how to do that correctly.
Here's my code, but the RedirectToAction
aren't doing anything here!
[HttpPost]
public ActionResult SendEmail(EmailContentViewModel emailDetails)
{
using (MailMessage email = new MailMessage(emailDetails.from, emailDetails.to))
{
email.Subject = emailDetails.subject;
email.Body = emailDetails.body;
email.Priority = emailDetails.MailPriority;
processSendingEmail(email, (result) =>
{
RedirectToAction("ContactResult", "Contact", new { success = result }); //It's not redirecting to the ContactResult page!
});
}
return null;
}
private void processSendingEmail(MailMessage email, Action<bool> callback= null)
{
using (SmtpClient smtpClient = new SmtpClient(_smtpHostName, _smtpPort))
{
bool sentSuccessfully = false;
try
{
//.............//
}
catch(Exception e)
{
//.............//
}
callback?.Invoke(sentSuccessfully);
}
}
Based on Panagiotis Kanavos's response, here is a working code:
[HttpPost]
public async Task<ActionResult> SendEmail(EmailContentViewModel emailDetails)
{
using (MailMessage email = new MailMessage(emailDetails.from, emailDetails.to))
{
email.Subject = emailDetails.subject;
email.Body = emailDetails.body;
email.Priority = emailDetails.MailPriority;
var sentSuccessfully= await processSendingEmail(email);
return RedirectToAction("ContactResult", "Contact", new { success = sentSuccessfully});
}
}
private async Task<bool> processSendingEmail(MailMessage email)
{
var client = new MailKit.Net.Smtp.SmtpClient();
//Configure the client here ...
try
{
var msg = (MimeKit.MimeMessage)email;
await client.SendAsync(msg);
return true;
}
catch (Exception ex)
{
Debug.Fail(ex.Message);
string errorMessage = "";
switch (ex)
{
case SmtpFailedRecipientException f:
errorMessage = $"Failed to send to {f.FailedRecipient}";
break;
case SmtpException s:
errorMessage = "Protocol error";
break;
default:
errorMessage = "Unexpected error";
break;
}
//Do anything you want with the error message
return false;
}
}
Don't use a callback. RedirectToAction creates an ActionResult that should be returned by the action, it doesn't force a redirect.
The proper way to do something asynchronously is to use async/await. Even if your email library doesn't have task-based asynchronous methods you can adapt it to the task-based model using a TaskCompletionSource. That would be rather unusual though as most libraries have moved from older async models like callbacks, events and APM to tasks.
MailMessage
suggests you're using SmtpClient. The SendMailAsync method is task-based, which means you can write
await client.SendMailAsync(email);
For example :
[HttpPost]
public async Task<ActionResult> SendEmail(EmailContentViewModel emailDetails)
{
SmptClient client = ... //Configure the client here
using (MailMessage email = new MailMessage(emailDetails.from, emailDetails.to))
{
email.Subject = emailDetails.subject;
email.Body = emailDetails.body;
email.Priority = emailDetails.MailPriority;
await client.SendMailAsync(email);
return RedirectToAction("ContactResult", "Contact", new { success = true });
};
}
SmptClient is an obsolete class though. Its documentation page warns that :
We don't recommend that you use the SmtpClient class for new development. For more information, see SmtpClient shouldn't be used on GitHub.
That link explains that :
SmtpClient doesn't support many modern protocols. It is compat-only. It's great for one off emails from tools, but doesn't scale to modern requirements of the protocol.
The recommendation is to use newer libraries like MailKit
MailKit allows explicit casting of a MailMessage
to a MimeMessage
which makes it easy to convert the existing code to MailKit:
[HttpPost]
public async Task<ActionResult> SendEmail(EmailContentViewModel emailDetails)
{
var client = new MailKit.Net.Smtp.SmptClient();
/Configure the client here ...
using (MailMessage email = new MailMessage(emailDetails.from, emailDetails.to))
{
email.Subject = emailDetails.subject;
email.Body = emailDetails.body;
email.Priority = emailDetails.MailPriority;
var msg=(MailKit)email;
await client.SendAsync(msg);
return RedirectToAction("ContactResult", "Contact", new { success = true });
};
}
Error Handling
Both MailKit's and the old SmptClient's Send methods either succeed or throw. One option would be to just hide the exception and return a true/false success flag :
try
{
await client.SendAsync(msg);
return RedirectToAction("ContactResult", "Contact", new { success = true});
}
catch
{
return RedirectToAction("ContactResult", "Contact", new { success = false});
}
That's not very helpful though, either for the user or the administrator trying to diagnose possible problems. The methods' documentation explains the types of exceptions that may occur, eg: ArgumentNullException
for a null message, an InvalidOperationException, SmtpFailedRecipientException and more.
At the very least, the code can log the exception before returning failure :
catch(Exception ex)
{
_log.Error(ex);
return RedirectToAction("ContactResult", "Contact", new { success = false});
}
A better idea would be to handle specific exceptions and possibly warn the user :
catch(SmtpFailedRecipientException ex)
{
_log.Error(ex);
return RedirectToAction("ContactResult", "Contact", new { success = false,message=$"Couldn't send the message to {ex.FailedRecipient}"});
}
catch(SmtpException ex)
{
_log.Error(ex);
return RedirectToAction("ContactResult", "Contact", new { success = false,message="Failed to send the message"});
}
catch(Exception ex)
{
_log.Error(ex);
return RedirectToAction("ContactResult", "Contact", new { success = false,message="An unexpected error occured"});
}
Pattern matching in C# 7 makes this easier :
catch(Exception ex)
{
_log.Error(ex);
string message="";
switch (ex)
{
case SmtpFailedRecipientException f:
message=$"Failed to send to {f.FailedRecipient}";
break;
case SmptException s :
message="Protocol error";
break;
default:
message="Unexpected error";
break;
}
return RedirectToAction("ContactResult", "Contact", new { success = false,message=message});
}
Separate method
Refactoring the sending code to a separate method is easy. The try/catch block and the client declaration can be extracted to a separate method :
async Task<string> MySendMethod(MailMessage email)
{
var client = new MailKit.Net.Smtp.SmptClient();
//Configure the client here ...
try
{
var msg=(MailKit)email;
await client.SendAsync(msg);
return "";
}
catch(Exception ex)
{
_log.Error(ex);
switch (ex)
{
case SmtpFailedRecipientException f:
return $"Failed to send to {f.FailedRecipient}";
case SmptException s :
return "Protocol error";
default:
return "Unexpected error";
}
}
}
Instead of returning a RedirectToActionResult
, the method returns a result string. If that is empty, the operation succeeded. The controller action can be rewritten like this:
[HttpPost]
public async Task<ActionResult> SendEmail(EmailContentViewModel emailDetails)
{
using (MailMessage email = new MailMessage(emailDetails.from, emailDetails.to))
{
email.Subject = emailDetails.subject;
email.Body = emailDetails.body;
email.Priority = emailDetails.MailPriority;
var message=await MySendMethod(email);
return RedirectToAction("ContactResult", "Contact",
new { success = String.IsNullOrWhitespace(result),
message=message
});
};
}