Sending email MVC4 C#

Question

I am trying to send an email inside an action. However, the action always returns a blank screen.

View:

<% using(Html.BeginForm("Sendlink", "Home")) %>
    <% { %>
     <input type="text" id="toemail" value="" />
        <input type="submit" value="Send" />
    <% } %>

Controller:

public ActionResult Sendlink()
{
    return View();
}

[AcceptVerbs(HttpVerbs.Post)]
public ActionResult Sendlink(FormCollection formCollection)
{
    try
    {
        string message = Session["link"].ToString();
        string toemail = formCollection["toemail"];
        MailEngine.Send("mail@mail.com", toemail, "link", message);
        return RedirectToAction("CanvasShare");
    }
    catch
    {

    }
    return null;
}

Class MailEngine:

public static void Send(string from, string to, string subject, string body)
{
    try
    {
        MailMessage mail = new MailMessage(from, to, subject, body);
        SmtpClient client = new SmtpClient("smtp.mymail.com");
        client.DeliveryMethod = SmtpDeliveryMethod.Network;
        client.EnableSsl = false;
        client.Send(mail);
    }
    catch
    {

    }
}

First of all I would not use FormCollection as a parameter, but use SendLink(string toemail). Look into ModelBinding for more information. — Dirk Boer, Jun 21 '13 at 06:30
Your MailEngine.Send is probably throwing an exception causing your SendLink action to return null instead of returning af view. Beware of empty catch blocks. — Jakob Christensen, Jun 21 '13 at 06:34
I changed Formcollection to string toemail, on when I debug, toemail is null. — hncl, Jun 21 '13 at 06:38
You need to set the `name` attribute of input to: `"tomail"` in order to make it bindable. — mipe34, Jun 21 '13 at 06:49

score 3 · Answer 1 · edited May 23 '17 at 10:32

You are using empty catch blocks in your application. This is not really a good idea, for a more in-depth discussion of this, see Why are empty catch blocks a bad idea? and related questions.

What seems to be happening to you is the following:

An exception is thrown somewhere inside the try block of your Sendlink(FormCollection formCollection) method. This exception seems to originate in the RedirectToAction("CanvasShare") call since all other calls inside that try block don't generate exceptions. (Especially since you suppressed exceptions thrown by the MailEngine.Send method.)
The empty catch block in your Sendlink(FormCollection formCollection) is invoked. This is the place where you should generate an error message and display it to the user. However, you have decided to leave it blank so nobody knows that something went wrong and what it was.
The control flow reaches the return null; statement in your Sendlink(FormCollection formCollection) method. My guess is that you put this in there because the compiler complained about a missing return value. This null is now returned and results in an empty view being rendered.

The obvious fix is to check RedirectToAction and find out why it throws an exception. This exception probably indicates a problem in your code or app and you need to take steps to prevent it from occurring.

The next fix is to actually implement error handling in your application. Remove all empty catch blocks and think about whether you want to throw the exception or whether you want to handle it right then and there. Ignoring it is, however, almost never a good idea.

To illustrate the problem in your app: If the exception in RedirectToAction is not thrown, your email sending might still fail. However, your UI does not have any way of finding out that something went wrong because you are ignoring the exception right in the MailClient.Send method. If you ever ship this to production, email sending will fail silently and your customers will wonder why they never received emails. It will then be quite hard for you to find out what the actual problem is and where it occurred.

Thank you so much Chris, this is very useful to implement in my code. — hncl, Jun 21 '13 at 07:02

mipe34 · Accepted Answer · 2013-06-21T07:35:12.147

2

toemail will be always null.

You need to set the name attribute of input to: "toemail" in order to make it bindable.

<input type="text" id="toemail" name="toemail"  />

Neverthless, as others say, it is really not a good idea to use empty catch statements. It hides a potential bug. As in your case, there is a hidden exception in try catch block resulting in null action result, thus the blank screen appeared.

There are several options, how to handle exception in ASP MVC. My favorite one is the combination of exception filter and <CustomErrors mode="On"/> web.config settings.

protected override void OnException(ExceptionContext filterContext)
{
    base.OnException(filterContext);

    if (filterContext.HttpContext.IsCustomErrorEnabled)
    {
        if (filterContext.Exception is SecurityException)
        {
            filterContext.ExceptionHandled = true;
            filterContext.Result = View("FriendlyError");
            //log the exception etc...
        }
    }
}

So when custom error is enabled you can return a friendly error screen on production, or disable it to see the actual exception when debugging.

edited Jun 21 '13 at 07:35

answered Jun 21 '13 at 07:00

mipe34

5,596
3
26
38

While this is certainly a problem for sending email, why would it cause a blank screen to be returned? – Chris Jun 21 '13 at 07:08
@Chris: mail engine will with high probability throw an exception, when `toemail` is null. So the action result is null -> blank screen. – mipe34 Jun 21 '13 at 07:13
`MailEngine` will never throw an exception because all exceptions are caught right inside `MailEngine.Send`. – Chris Jun 21 '13 at 07:15
1

You are right, I have overlooked it. Another potential to throw `NullReferenceException` is `string message = Session["link"].ToString();` – mipe34 Jun 21 '13 at 07:31
Yes, but that is still not something influenced by adding the `name` attribute on the `input` tag. I am wondering why that prompted the change in behaviour. – Chris Jun 21 '13 at 07:33
Me too ;-) I do not see the reason why `RedirectToAction` should throw an Exception. So the last option I see is the coincidently (not)set value of "link" in Session. If I could debug it by myself, it would be lot easier ;-) – mipe34 Jun 21 '13 at 07:41

Sending email MVC4 C#

2 Answers2