Warning: Can't synchronize with repository "(default)" (Unsupported version control system "svn": No module named svn). Look in the Trac log for more information.

Ticket #258 (closed enhancement: fixed)

Opened 13 years ago

Last modified 12 years ago

Even better better error handling

Reported by: Simon Belak <simon.belak@…> Owned by: simon
Priority: normal Milestone: 0.9
Component: TurboGears Version: 0.9a6
Severity: normal Keywords:
Cc:

Description

Attached is an implementation of enhanced error handling as talked about in #219.

I am introducing a new decorator error_handler to declare a method an error handler for a given rule or as default if rule is omitted. Call dispatching is done via RuleDispatch? generic functions.

Basic (and probably most often used) behaviour is for rule to be a method for which the decorated method is the error handler. A single method can be an error handler for multiple methods.

For more specific cases rule can be a string containing an arbitrary Python logical expression (in essence fully exposing generic.when()).

Compatibility with current way of handling errors is fully retained. Only constrain is that one cannot use default error handlers and old-style error handling at the same time (default error handler has precedence).

Basic example:

from turbogears.controllers import error_handler
...
class Root(controllers.RootController):

  def a(self):
    ...
  def b(self):
    ...
  def c(self):
    ...
  def d(self):
    ...
  def e(self):
    ...

  @error_handler(a)
  def a_error(self, errors):
    """ Error for a(). """

  @error_handler(b)
  @error_handler(c)
  def bc_error(self, errors):
    """ Error for b() or c(). """

  @error_handler()
  def default_error(self, errors):
    """ Default error. 
    
    For this example this method would catch all errors in d() and e().
    """

Full-on dispatch:

@error_handler("'emailaddr' in errors")
def email_error(self, errors):
    """ Handles all email address related errors. """ 

When using custom rules, one should (as always when dealing with generic functions) be mindful of ambiguity.

Attachments

controllers.diff Download (2.6 KB) - added by Simon Belak <simon.belak@…> 13 years ago.
controllers.2.diff Download (3.0 KB) - added by Simon Belak <simon.belak@…> 13 years ago.
New proposition.
controllers.3.diff Download (3.1 KB) - added by Simon Belak <simon.belak@…> 13 years ago.
Fixed a bug.
test_errorhandler.py Download (4.4 KB) - added by Simon Belak <simon.belak@…> 13 years ago.
unittest
controllers.4.diff Download (3.4 KB) - added by simon 13 years ago.
Added support for delcaring a method it's own error handler; dispatch_error() is no longer private to the module.
test_errorhandler.2.py Download (5.3 KB) - added by simon 13 years ago.
Added test for recursive error handler.
controllers.5.diff Download (3.4 KB) - added by simon 13 years ago.
controllers.6.diff Download (3.4 KB) - added by anonymous 13 years ago.
Changed names a bit.

Change History

Changed 13 years ago by Simon Belak <simon.belak@…>

comment:1 Changed 13 years ago by Simon Belak <simon.belak@…>

Ultimately this should be merged with #113 in such a way that methods decorated by error_handler could be brought to all relevant (i.e. no validators and inputform) expose() functionality without actually being exposed.

comment:2 Changed 13 years ago by Luca <luca@…>

Just a stupid suggestion: what about

  @error_handler(b, c)
  def bc_error(self, errors):
    """ Error for b() or c(). """

?

comment:3 Changed 13 years ago by Simon Belak <simon.belak@…>

It depends on what will be the most oft-use case. If one for many prevails, than yout suggestion would certanly be the way to go. On the other hand I do like the explicity and clarity of strongly defined number of arguments.

comment:4 Changed 13 years ago by michele

 Link to the discussion.

comment:5 Changed 13 years ago by Simon Belak <simon.belak@…>

Changed 13 years ago by Simon Belak <simon.belak@…>

New proposition.

comment:6 Changed 13 years ago by Simon Belak <simon.belak@…>

Presented is an implementation of JP's idea (see discussion) with some enchantments. It allows defining error handlers as follows:

@expose(html='etc')
# Something evil happend to our DB
@error_handler(sql_error, "isinstance(errors, SQLObjectIntegrityError)")
# Default
@error_handler(submit_error)
@input(form=form)
def submit(self, **data):
    ...

Each exposed method can have multiple error handlers if generic functions are harnessed.

Execution of decorated method is encapsulated in a try-catch block passing any resulting exceptions to an appropriate error handler.

Compatibility with existing way of handling errors is retained.

comment:7 Changed 13 years ago by Simon Belak <simon.belak@…>

  • Status changed from new to assigned

comment:8 Changed 13 years ago by Simon Belak <simon.belak@…>

  • Owner changed from anonymous to Simon Belak
  • Status changed from assigned to new

Changed 13 years ago by Simon Belak <simon.belak@…>

Fixed a bug.

Changed 13 years ago by Simon Belak <simon.belak@…>

unittest

comment:9 Changed 13 years ago by Simon Belak <simon.belak@…>

Had I written the tests when I should have I would have saved myself some embarrassment. :)

comment:10 Changed 13 years ago by simon

  • Status changed from new to assigned
  • Owner changed from Simon Belak to simon

Changed 13 years ago by simon

Added support for delcaring a method it's own error handler; dispatch_error() is no longer private to the module.

Changed 13 years ago by simon

Added test for recursive error handler.

comment:11 Changed 13 years ago by simon

Recursive error handler example:

@expose(...)
@error_handler()
def foo(self, errors, bar="", baz="")
    ...

with specialisation rules:

@expose(...)
@error_handler(dispatch_rules="isinstance(errors, Exception)")
def foo(self, errors, bar="", baz="")
    ...

Looking at the example above, maybe a shorter (single word perhaps) name for dispatch_rules would be in order?

Changed 13 years ago by simon

Changed 13 years ago by anonymous

Changed names a bit.

comment:12 Changed 13 years ago by simon

Have changed 'dispatch_rules' to 'rules'.

comment:13 Changed 13 years ago by kevin

  • Summary changed from [PATCH] Even better better error handling to Even better better error handling

I like the idea, except for one thing: the common idiom for dealing with form validation errors with turbogears widgets is this:

@expose(html="...")
def index(self):
    ...

@expose(html="...")
@validators(form=myform)
def save(self, val1, val2):
    if cherrypy.request.form_errors:
        return index()

Note that index doesn't have an errors parameter. An earlier version of the TurboGears code would introspect the function to see if it had an errors parameter, and pass the errors in if it does. Maybe we should do that?

There's a trick there, of course. We now have 4 decorators:

expose validate error_handler identity.require

In order to introspect the function, we need direct access to it. I think the TurboGears decorators should all set an attribute on the function that has the original function object. That way, the decorators are a bit more flexible in terms of how they are arranged over the method in question.

I'm removing the PATCH tag until this part is worked out.

comment:14 Changed 13 years ago by kevin

  • Status changed from assigned to closed
  • Resolution set to fixed

The spiffy final version of this has been committed in [617].

Note: See TracTickets for help on using tickets.