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.