views:

7239

answers:

7

I am just getting started on ASP.NET MVC so bear with me. I've searched around this site and various others and have seen a few implementations of this.

EDIT: I forgot to mention I am using RC2

Using URL Routing:

routes.MapRoute(
            "Error",
            "{*url}",
            new { controller = "Errors", action = "NotFound" }  //404s
        );

The above seems to take care of requests like this (assuming default route tables setup by initial MVC project): "/blah/blah/blah/blah"

Overriding HandleUnknownAction() in the controller itself:

//404s - handle here (bad action requested
    protected override void HandleUnknownAction(string actionName) {
        ViewData["actionName"] = actionName;
        View("NotFound").ExecuteResult(this.ControllerContext);
    }

However the previous strategies do not handle a request to a Bad/Unknown controller. For example, I do not have a "/IDoNotExist", if I request this I get the generic 404 page from the web server and not my 404 if I use routing + override.

So finally, my question is: Is there any way to catch this type of request using a route or something else in the MVC framework itself?

OR should I just default to using Web.Config customErrors as my 404 handler and forget all this? I assume if I go with customErrors I'll have to store the generic 404 page outside of /Views due to the Web.Config restrictions on direct access. Anyway any best practices or guidance is appreciated.

+52  A: 

The code is taken from http://blogs.microsoft.co.il/blogs/shay/archive/2009/03/06/real-world-error-hadnling-in-asp-net-mvc-rc2.aspx and works in ASP.net MVC 1.0 as well

Here's how I handle http exceptions:

protected void Application_Error(object sender, EventArgs e)
{
   Exception exception = Server.GetLastError();
   // Log the exception.

   ILogger logger = Container.Resolve<ILogger>();
   logger.Error(exception);

   Response.Clear();

   HttpException httpException = exception as HttpException;

   RouteData routeData = new RouteData();
   routeData.Values.Add("controller", "Error");

   if (httpException == null)
   {
       routeData.Values.Add("action", "Index");
   }
   else //It's an Http Exception, Let's handle it.
   {
       switch (httpException.GetHttpCode())
       {
          case 404:
              // Page not found.
              routeData.Values.Add("action", "HttpError404");
              break;
          case 500:
              // Server error.
              routeData.Values.Add("action", "HttpError500");
              break;

           // Here you can handle Views to other error codes.
           // I choose a General error template  
           default:
              routeData.Values.Add("action", "General");
              break;
      }
  }           

  // Pass exception details to the target error View.
  routeData.Values.Add("error", exception);

  // Clear the error on server.
  Server.ClearError();

  // Avoid IIS7 getting in the middle
  Response.TrySkipIisCustomErrors = true; 

  // Call target Controller and pass the routeData.
  IController errorController = new ErrorController();
  errorController.Execute(new RequestContext(    
       new HttpContextWrapper(Context), routeData));
}
Shay Jacoby
I'd like to add that this is fairly easy to write unit tests for the above code as well. I only had to make a few small changes. +1
Page Brooks
I'm not sure I understand why you're checking for an httpException. isn't the point of this handler to catch - for instance - a NullReferenceException and then GENERATE an http exception. when would Server.GetLastError() ever actually be an HttpException unless you had made an Http call within your actual business logic. i removed this from my implementation but would love to know if it is justified and i missed something
Simon_Weaver
update: checking for an http 404 is definitely required, but i'm still not quite sure when you'd ever get a 500. also you need to also explicitly set the Response.StatusCode = 404 or 500 otherwise google will start indexing these pages if you are returning a 200 status code which this code currently does
Simon_Weaver
@Simon_Weaver: agreed! This needs to return 404 status codes. It's essentially broken as a 404 solution until it does. Check this: http://www.codinghorror.com/blog/2007/03/creating-user-friendly-404-pages.html
cottsak
There is an underlying flaw with this whole suggestion - by the time execution has bubbled up to the Global.asax, too much of the HttpContext is missing. You can not route back into your controllers like the example suggests. Refer to the comments in the blog link at top.
cottsak
A: 

Shay Jacoby:

if you do a url on your example like this:

/Home/About/test/broken

then it doesnt seem to use the general error code that you specify, do you know why this is?

Andi
+1  A: 

It looks like this works only on GET requests. If I have an exception which is caused after a POST then this ain't working.

There seems to be an innerexception in the Context, "This operation requires IIS integrated pipeline mode."

Does anybody know how to handle an exception on a post?

patheems
Sounds like this is something to do with IIS 6 as the integrated pipeline mode only exists in IIS 7. I don't have a solution for you (I use IIS 7 and haven't hit this error), but sounds like a good place to start.
Brian
I have notice that it does not catch security error (like posting <script> in a textbox)
Eduardo Molteni
A: 

I am getting the following error at

errorController.Execute(new RequestContext(    
       new HttpContextWrapper(Context), routeData));

"The SessionStateTempDataProvider requires SessionState to be enabled."

Googling for a soln. I cam to learn that the error was solved by putting "" in th web.config.

But didnt work in my case. Is it happening for the POST request?

Rahat
+34  A: 

Requirements for 404

The following are my requirements for a 404 solution and below i show how i implement it:

  • I want to handle matched routes with bad actions
  • I want to handle matched routes with bad controllers
  • I want to handle un-matched routes (arbitrary urls that my app can't understand) - i don't want these bubbling up to the Global.asax or IIS because then i can't redirect back into my MVC app properly
  • I want a way to handle in the same manner as above, custom 404s - like when an ID is submitted for an object that does not exist (maybe deleted)
  • I want all my 404s to return an MVC view (not a static page) to which i can pump more data later if necessary (good 404 designs) and they must return the HTTP 404 status code

Solution

I think you should save Application_Error in the Global.asax for higher things, like unhandled exceptions and logging (like Shay Jacoby's answer shows) but not 404 handling. This is why my suggestion keeps the 404 stuff out of the Global.asax file.

Step 1: Have a common place for 404-error logic

This is a good idea for maintainability. Use an ErrorController so that future improvements to your well designed 404 page can adapt easily. Also, make sure your response has the 404 code!

public class ErrorController : MyController
{
    #region Http404

    public ActionResult Http404(string url)
    {
        Response.StatusCode = (int)HttpStatusCode.NotFound;
        var model = new NotFoundViewModel();
        // If the url is relative ('NotFound' route) then replace with Requested path
        model.RequestedUrl = Request.Url.OriginalString.Contains(url) & Request.Url.OriginalString != url ?
            Request.Url.OriginalString : url;
        // Dont get the user stuck in a 'retry loop' by
        // allowing the Referrer to be the same as the Request
        model.ReferrerUrl = Request.UrlReferrer != null &&
            Request.UrlReferrer.OriginalString != model.RequestedUrl ?
            Request.UrlReferrer.OriginalString : null;

        // TODO: insert ILogger here

        return View("NotFound", model);
    }
    public class NotFoundViewModel
    {
        public string RequestedUrl { get; set; }
        public string ReferrerUrl { get; set; }
    }

    #endregion
}

Step 2: Use a base Controller class so you can easily invoke your custom 404 action and wire up HandleUnknownAction

404s in ASP.NET MVC need to be caught at a number of places. The first is HandleUnknownAction.

The InvokeHttp404 method creates a common place for re-routing to the ErrorController and our new Http404 action. Think DRY!

public abstract class MyController : Controller
{
    #region Http404 handling

    protected override void HandleUnknownAction(string actionName)
    {
        // If controller is ErrorController dont 'nest' exceptions
        if (this.GetType() != typeof(ErrorController))
        this.InvokeHttp404(HttpContext);
    }

    public ActionResult InvokeHttp404(HttpContextBase httpContext)
    {
        IController errorController = ObjectFactory.GetInstance<ErrorController>();
        var errorRoute = new RouteData();
        errorRoute.Values.Add("controller", "Error");
        errorRoute.Values.Add("action", "Http404");
        errorRoute.Values.Add("url", httpContext.Request.Url.OriginalString);
        errorController.Execute(new RequestContext(
             httpContext, errorRoute));

        return new EmptyResult();
    }

    #endregion
}

Step 3: Use Dependency Injection in your Controller Factory and wire up 404 HttpExceptions

Like so (it doesn't have to be StructureMap):

public class StructureMapControllerFactory : DefaultControllerFactory
{
    protected override IController GetControllerInstance(Type controllerType)
    {
        try
        {
            if (controllerType == null)
                return base.GetControllerInstance(controllerType);
        }
        catch (HttpException ex)
        {
            if (ex.GetHttpCode() == (int)HttpStatusCode.NotFound)
            {
                IController errorController = ObjectFactory.GetInstance<ErrorController>();
                ((ErrorController)errorController).InvokeHttp404(RequestContext.HttpContext);

                return errorController;
            }
            else
                throw ex;
        }

        return ObjectFactory.GetInstance(controllerType) as Controller;
    }
}

I think its better to catch errors closer to where they originate. This is why i prefer the above to the Application_Error handler.

This is the second place to catch 404s.

Step 4: Add a NotFound route to Global.asax for urls that fail to be parsed into your app

This route should point to our Http404 action. Notice the url param will be a relative url because the routing engine is stripping the domain part here? That is why we have all that conditional url logic in Step 1.

        routes.MapRoute("NotFound", "{*url}", 
            new { controller = "Error", action = "Http404" });

This is the third and final place to catch 404s in an MVC app that you don't invoke yourself. If you don't catch unmatched routes here then MVC will pass the problem up to ASP.NET (Global.asax) and you don't really want that in this situation.

Step 5: Finally, invoke 404s when your app can't find something

Like when a bad ID is submitted to my Loans controller:

    //
    // GET: /Detail/ID

    public ActionResult Detail(int ID)
    {
        Loan loan = this._svc.GetLoans().WithID(ID);
        if (loan == null)
            return this.InvokeHttp404(HttpContext);
        else
            return View(loan);
    }

It would be nice if all this could be hooked up in fewer places with less code but i think this solution is more maintainable, more testable and fairly pragmatic.

Thanks for the feedback so far. I'd love to get more.

NOTE: This has been edited significantly from my original answer but the purpose/requirements are the same - this is why i have not added a new answer

cottsak
Dude - i really really like this :) I used to throw Http404Exceptions, etc and handled that in a OnError attribute type thingy .. but this damn good, too :)
Pure.Krome
thanks. i hope it can be improved on
cottsak
@cottsak buddy, with your StructureMap IoC code .. when (or how) can an HttpException get thrown? A simple example, please? And secondly, why does your code in Step #5 invoke404 and then return EmptyResult??
Pure.Krome
@Krome: #1- "The controller for path '/dfsdf' could not be found or it does not implement IController." is the `.Message` from requesting the path '/dfsdf' - i guess when a route is not matched, MVC throws a `HttpException` like this. #2- Step 5 is maybe not an ideal usage example but for now it seems to be ok for me. I need to return the `EmptyResult` as part of the action. The fact that it's "empty" makes no difference as in the `InvokeHttp404` method i re-route again. help?
cottsak
@cottsak, I really like this too. One odd thing happens for me when executing step 4: the `Http404` action gets executed twice **on the same request**, once when the controller factory calls `InvokeHttp404` and once when `HandleUnknownAction` calls `InvokeHttp404`. In my example, the non-existant URL is http://localhost:64434/jakds. `HandleUnknownAction` gets the default action named "index" as a parameter. I think I have to change the route data rather than call `InvokeHttp404` in the controller factory.
flipdoubt
@flipdoubt: In the case where you have a url that matches a route and parses a controller name and it's bad, then with my code, you will have the `Http404` action invoked twice. As far as i can see there is nothing i can do about that as the internals of the routing engine dictate the checking 1st on the controller and then 2nd on the action.
cottsak
@flipdoubt: I've made some changes but it still does not remove the two runs through the `Http404` action for the "bad controller" case. I'm going to leave it like it is. This is not ideal but it does not affect the actual response to the client so i think it's ok for now.
cottsak
Wow, great writeup. This is definitely a better approach than a single Application_Error handler, especially if you have custom controller factories and RESTful services.
Igor Zevaka
Thanks for the full write-up. One addition is that when running under IIS7 you need to add set the property "TrySkipIisCustomErrors" to true. Otherwise IIS will still return the default 404 page.We added Response.TrySkipIiisCustomErrors=true; after the line in Step 5 that sets the status code.http://msdn.microsoft.com/en-us/library/system.web.httpresponse.tryskipiiscustomerrors.aspx
Rick
@Rick: thanks for adding that
cottsak
Pretty robust and DRY, maybe I am lacking much exp, but I don't see the problem in handling the 404 (Error) in the Global.asax, that seems pretty DRY and KISS to me.
UberNeet
Thanks, helped me. However it doesn't seem to catch a direct url to a valid .aspx file, eg http://localhost:50096/views/home/index.aspx (when that path/file exists) still throws the default 404. Any ideas how to catch this one?
Seba Illingworth
@Seba: I'd suspect that it's not catching that url because MVC routing normally ignores valid static paths to valid static resources - eg: the aspx files, images and css in content/script folders. So i guess in the internals of the routing engine this url is ignored under the assumption that you are referencing that view for some other purpose than normal routing (because, as you know, you normally access views via a route, not directly). Anything that can be parsed by your routes as a "route" would be caught by this 404 handling solution, which is by design.
cottsak
+1 - Absolutely incredible answer
Sergio Tapia
+3  A: 

I really like cottsaks solution and think its very clearly explained. my only addition was to alter step 2 as follows

public abstract class MyController : Controller
{

    #region Http404 handling

    protected override void HandleUnknownAction(string actionName)
    {
        //if controller is ErrorController dont 'nest' exceptions
        if(this.GetType() != typeof(ErrorController))
        this.InvokeHttp404(HttpContext);
    }

    public ActionResult InvokeHttp404(HttpContextBase httpContext)
    {
        IController errorController = ObjectFactory.GetInstance<ErrorController>();
        var errorRoute = new RouteData();
        errorRoute.Values.Add("controller", "Error");
        errorRoute.Values.Add("action", "Http404");
        errorRoute.Values.Add("url", httpContext.Request.Url.OriginalString);
        errorController.Execute(new RequestContext(
             httpContext, errorRoute));

        return new EmptyResult();
    }

    #endregion
}

Basically this stops urls containing invalid actions AND controllers from triggering the exception routine twice. eg for urls such as asdfsdf/dfgdfgd

Dave Lowe
A: 

The only way I could get @cottsak's method to work for invalid controllers was to modify the existing route request in the CustomControllerFactory, like so:

public class CustomControllerFactory : DefaultControllerFactory
{
    protected override IController GetControllerInstance(RequestContext requestContext, Type controllerType)
    {
        try
        {
            if (controllerType == null)
                return base.GetControllerInstance(requestContext, controllerType); 
            else
                return ObjectFactory.GetInstance(controllerType) as Controller;
        }
        catch (HttpException ex)
        {
            if (ex.GetHttpCode() == (int)HttpStatusCode.NotFound)
            {
                requestContext.RouteData.Values["controller"] = "Error";
                requestContext.RouteData.Values["action"] = "Http404";
                requestContext.RouteData.Values.Add("url", requestContext.HttpContext.Request.Url.OriginalString);

                return ObjectFactory.GetInstance<ErrorController>();
            }
            else
                throw ex;
        }
    }
}

I should mention I'm using MVC 2.0.

Dave K