On 6 mars 2013, at 08:49, Ian Kelly <ian.g.ke...@gmail.com> wrote:

> On Tue, Mar 5, 2013 at 3:13 PM, Aymeric Augustin
> <aymeric.augus...@polytechnique.org> wrote:
>> In the mean time, I discovered that it's impossible to implement
>> TransactionMiddleware reliably as a middleware, because there's no guarantee
>> that process_response or process_exception will be called.

I verified the code; here are a few scenarios where TransactionMiddleware
fails simply because it's implemented as a middleware.

(1) Another middleware's process_request returns a response before
    TM.process_request can run. TM.process_response/process_exception
    attempts to close a transaction than was never opened.

(2) Another middleware's process_request raises an exception after
    TM.process_request has run. TM.process_exception is never called.

(3) Any middleware's process_view raises an exception.
    TM.process_exception is never called.

(4) Any middleware's process_template_response raises an exception.
    TM.process_exception is never called.

(5) Rendering a TemplateResponse raises an exception.
    TM.process_exception is never called.

(6) The view returns normally. Another middleware's process_response
    raises an exception before TM.process_response can run.

(7) The view returns an exception. Another middleware's process_exception
     raises an exception before TM.process_exception can run.

(8) The view returns an exception. Another middleware's process_exception
    returns a response before TM.process_exception can run. Another
    middleware's process_response raises an exception before
    TM.process_response can run.


I've implemented atomic as a context manager so that Python itself
guarantee that calls to __enter__ and __exit__ will be balanced.

Django's middleware API requires to give up this property, and I'm very
reluctant to do that. If a transaction accidentally stays open, that's a major
data-loss bug, all the more since I added persistent connections.

(This doesn't happen in Django 1.5 because the HTTP handler unwinds the
"transaction management" stack and closes the connection in request_finished.)


> Perhaps this could be fixed.  We could add another middleware method
> called process_finally or somesuch, that would be guaranteed to be
> called on every request after the other methods.  It would take a
> request and response as arguments, and any return value would be
> ignored.  If it raised an exception, the exception would be caught,
> logged, and otherwise ignored.

I tried to implement this idea. It turns out we'd also need:
- a process_beforeall method that would be guaranteed to be called on
  every request before the other methods, to cater for case (1).
- a way to tell process_finally if an unhandled exception occurred, and
  it isn't clear to me if an exception dealt with by another middleware's
  process_exception should be considered handled or not.

Then TransactionMiddleware would look like this:

class TransactionMiddleware(object):
    def __init__(self):
        self.atomic = transaction.atomic()

    def process_beforeall(self, request):
        self.atomic.__enter__()

    def process_finally(self, request, response, exception):
        if exception:
            traceback = exception.__traceback__ if six.PY3 else 
sys.exc_info()[2]
            self.atomic.__exit__(type(exception), exception, traceback)
        else:
            self.atomic.__exit__(None, None, None)
        return response


BaseHandler.get_response already has 8 "except" clauses, nested
three-levels deep, and despite this abundant error handling there's
many cases where calls aren't paired. I'm not thrilled by the idea of
making it even more complex.

There would certainly be some value in making the middleware API
more predictable. However, in the short term, adding two methods
only to support TransactionMiddleware feels a lot like shoehorning.

-- 
Aymeric.



-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.


Reply via email to