Question

I have some code which is common to a number of my controller actions pulled out into a private "helper" method just for consolidation. The method is expected to "get" an object for me, though it performs some sanity/security checks and could potentially redirect the user to other actions.

private Thingy GetThingy(int id)
{
    var thingy = some_call_to_service_layer(id);

    if( null == thingy )
        Response.Redirect( anotherActionUrl );

    // ... many more checks, all more complex than a null check

    return thingy;
}

public ActionResult ActionOne(int id)
{
    var thingy = GetThingy(id);
    // do more stuff
    return View();
}

// ... n more actions

public ActionResult ActionM(int id)
{
    var thingy = GetThingy(id);
    // do more stuff
    return View();
}

This functions properly with the exception that Elmah then notifies me of an exception:

System.Web.HttpException: Cannot redirect after HTTP headers have been sent.

So, my question is: Is there a more correct way to do what I am trying to do? Essentially, all I want is to cause the current action to stop processing and instead return a RedirectToRouteResult.

Was it helpful?

Solution

You cannot call Response.Redirect() in an action without breaking the flow of execution causing an error. Instead you can return an RedirectToRouteResult object or a RedirectResult object. In your action

return Redirect("/path");
//or
return RedirectToAction("actionname");

As in your case however you want to return a Thingy object you will need to seperate out the logic. You could do something like the following (I'm assuming you want to redirect to different actions otherwise Oenning's code will work)

public ActionResult Get(int id) {

  var thingy = GetThingy(id);

  var result = checkThingy(thingy);
  if (result != null) {
    return result;
  }

  //continue...
}

[NonAction]
private ActionResult CheckThingy(Thingy thingy) {

  //run check on thingy
  //return new RedirectResult("path");

  //run another check
  //new RedirectResult("different/path");

  return null;
}

Update You could put this code in an extension method or a base Controller class

public static class ThingyExtensions {

  public static ActionResult Check(this Thingy thingy) {
    //run checks here
  }

}

OTHER TIPS

One way to do this would be to define an Exception Filter to handle the redirect for you.

First, create a custom exception to represent your redirection:

public class RedirectException : Exception {

    private readonly string _url;

    public RedirectException(string url) {
        _url = url;
    }

    public string Url { get { return _url; }}

}

Then, define your Exception Filter:

public class RedirectExceptionAttribute : FilterAttribute, IExceptionFilter {
    public void OnException(ExceptionContext filterContext) {

        if (filterContext.ExceptionHandled) return;
        if (filterContext.Exception.GetType() != typeof(RedirectException)) return;

        filterContext.Result = new RedirectResult(((RedirectException)filterContext.Exception).Url);
        filterContext.ExceptionHandled = true;
    }
}

Globally register the new Exception Filter so that it applies to all controller actions (if this is what you want):

// in FilterConfig.cs (MVC 4.0)
public static void RegisterGlobalFilters(GlobalFilterCollection filters) {
    filters.Add(new RedirectExceptionAttribute());
}

And now, wherever you want to redirect, just throw a RedirectException:

private Thingy GetThingy(int id)
{
    var thingy = some_call_to_service_layer(id);

    if( null == thingy )
        throw new RedirectException( anotherActionUrl );

    // ... many more checks, all more complex than a null check

    return thingy;
}

public ActionResult ActionOne(int id)
{
    var thingy = GetThingy(id);
    // do more stuff
    return View();
}

// ... n more actions

public ActionResult ActionM(int id)
{
    var thingy = GetThingy(id);
    // do more stuff
    return View();
}

Of course you need to make sure you don't call GetThingy() from within a try/catch, or if you do then make sure you re-throw the exception.

try:

return RedirectToAction("youraction", "yourcontroller");

Hope it helps.

What about this.

public ActionResult ActionOne(int id)
{
    var thingy = GetThingy(id);
    if (thingy == null)
        return RedirectToAction("action", "controller");
    // do more stuff
    return View();
}

private Thingy GetThingy(int id)
{
    var thingy = some_call_to_service_layer(id);

    if( null == thingy )
        return null;

    // ... many more checks, all more complex than a null check

    return thingy;
}

By the way, IMHO the GetThingy(int id) method should be placed somewhere else. Maybe in the same place as the some_call_to_service_layer(id).

Ok, I think I figured it out. Apparently, you can arbitrarily trigger the execution of an ActionResult for just this sort of thing. So the helper method becomes:

private Thingy GetThingy(int id)
{
    var thingy = some_call_to_service_layer(id);

    if( null == thingy )
        RedirectToAction("myAction").ExecuteResult(ControllerContext);

    // ... many more checks, all more complex than a null check

    return thingy;
}

Works nicely, and allows me to keep the code where I wanted. Pretty sweet.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top