ASP.NET MVC

Re: HandleErrorAttribute / HttpContext.IsCustomErrorEnabled - Reasons for Change?

Woil — Tue, 09 Sep 2008 18:12:55 GMT

levib:

If an exception is thrown and we redirect to an error view, we lose that information, which defeats the purpose of using ModelState (and hence binders) in the first place.

But if an exception IS thrown an ugly error message will be shown instead of the one that I've specified? I realize you're trying to get users to use the ModelState, but perhaps there's a better way to accomplish this. Sorry to be argumentative.

In my case all of my "views" are just ajax json objects anyway... no angle brackets here... so my "view" packages up the exceptions and the js code displays them to the user real nice.

Re: HandleErrorAttribute / HttpContext.IsCustomErrorEnabled - Reasons for Change?

levib — Tue, 09 Sep 2008 18:04:52 GMT

Woil:
Just to clarify, I should put all of binding errors into the ModelStateDictionary and then handle them individually in my controller actions?

Correct. Binders and ModelState are meant to be used together. They were designed to work with one another.

Woil:
If a bind error is thrown why not catch it with the custom errors?

The ModelState objects are responsible for keeping track of the user's original input, even if that input was in error. If an exception is thrown and we redirect to an error view, we lose that information, which defeats the purpose of using ModelState (and hence binders) in the first place.

Re: HandleErrorAttribute / HttpContext.IsCustomErrorEnabled - Reasons for Change?

Woil — Tue, 09 Sep 2008 17:50:41 GMT

Just to clarify, I should put all of binding errors into the ModelStateDictionary and then handle them individually in my controller actions?

public ActionResult ViewProduct(Product p)
{
if(!ViewData.ModelState.IsValid)
//redirect to error display view

...

That seems a bit excessive if I instead I could actually handle the exception thrown via the Exception view. Why not both? If a bind error is thrown why not catch it with the custom errors?

Re: HandleErrorAttribute / HttpContext.IsCustomErrorEnabled - Reasons for Change?

levib — Tue, 09 Sep 2008 17:21:14 GMT

Parameter binders should not throw exceptions if the ModelStateDictionary is explicitly passed to them. Instead, they should populate that dictionary.

Re: HandleErrorAttribute / HttpContext.IsCustomErrorEnabled - Reasons for Change?

Woil — Tue, 09 Sep 2008 17:17:22 GMT

Also worthy of discussion: Parameter Binding errors are not caught by the ExceptionFilters. If a parameter does not bind correctly there is no way to catch the error with the HandleError attribute on a action or controller. A review of ControllerActionInvoker shows the code as:

public virtual bool InvokeAction(ControllerContext controllerContext, string actionName) {
...
IDictionary<string, object> parameters = GetParameterValues(methodInfo);
FilterInfo filterInfo = GetFiltersForActionMethod(methodInfo);
try {
//call action
}
catch (Exception ex) {
// something blew up, so execute the exception filters
ExceptionContext exceptionContext = InvokeExceptionFilters(ex, filterInfo.ExceptionFilters);
...
}

Instead, I think it would be better as:

public virtual bool InvokeAction(ControllerContext controllerContext, string actionName) {
...
FilterInfo filterInfo = GetFiltersForActionMethod(methodInfo);
try {
IDictionary<string, object> parameters = GetParameterValues(methodInfo);
//call action
}
catch (Exception ex) {
// something blew up, so execute the exception filters
ExceptionContext exceptionContext = InvokeExceptionFilters(ex, filterInfo.ExceptionFilters);
...
}

That way you could handle exceptions in parsing action parameters.

Re: HandleErrorAttribute / HttpContext.IsCustomErrorEnabled - Reasons for Change?

Woil — Tue, 09 Sep 2008 17:07:52 GMT

From the HandleErrorAttribute.cs in the Mvc Preview 5, note the comment:

// If custom errors are disabled, we need to let the normal ASP.NET exception handler
// execute so that the user can see useful debugging information.
if (filterContext.ExceptionHandled || !filterContext.HttpContext.IsCustomErrorEnabled) {
return;
}

I want the custom errors disabled for most exceptions. I want to see that juicy stack trace goodness. Eventually when I deploy I'll turn them off of course.

But before and after I deploy I want certain exceptions that I specify to be handled by the view I specify as they are being thrown for business logic reasons. Here's how I would write it:

public HandleErrorAttribute()
{
IgnoreWhileDebugging = true;
}
public bool IgnoreWhileDebugging { get; set; }
public virtual void OnException(ExceptionContext filterContext) {
if (filterContext == null) {
throw new ArgumentNullException("filterContext");
}

// If custom errors are disabled, we need to let the normal ASP.NET exception handler
// execute so that the user can see useful debugging information.
if (filterContext.ExceptionHandled || (!filterContext.HttpContext.IsCustomErrorEnabled && IgnoreWhileDebugging)) {
return;
}

//use here

[HandleError(ExceptionType = typeof(ValidationException), View = "ValidationError",IgnoreWhileDebugging = false)]
[HandleError()]
public class ProductController : Controller { ...

Re: HandleErrorAttribute / HttpContext.IsCustomErrorEnabled - Reasons for Change?

Haacked — Tue, 09 Sep 2008 01:07:54 GMT

You can change your debug="false" or in web.config set the <customErrors mode="On" />.

HandleErrorAttribute / HttpContext.IsCustomErrorEnabled - Reasons for Change?

Woil — Mon, 08 Sep 2008 22:19:20 GMT

I skimmed over it in the release notes, but noticed it when my rescues weren't working anymore. If (HttpContext.IsCustomErrorEnabled) then all [HandleError] attributes are ignored in version 5. For some errors this might be the ideal solution, but for others it isn't. In my case, I throw exceptions when a user doesn't fill out a form correctly. (For example putting letters in a phone number box.) These exceptions are then handled by the view and displayed nicely to the user. (All with JSON / ExtJs.)

In short: I ALWAYS want my specified error handling for certain exception types, regardless of debug or release mode. I understand that it would be nice to provide this feature to some users. How about:

[HandleError(ExceptionType = typeof(ValidationException),IgnoreForDebug = false, View = "ValidationError")]
public class ProductController {

...

Thanks for keeping the process open to comment.

-Will