PISTON_DISPLAY_ERRORS and transaction handling

45 views
Skip to first unread message

Dan Fairs

unread,
Feb 26, 2010, 7:53:09 AM2/26/10
to django...@googlegroups.com
Hi,

I just noticed something that may be a bug (in Piston or indeed Django), or may be intentional.

If PISTON_DISPLAY_ERRORS is set to True and an exception is raised in a handler, then Piston will return an HttpResponseServerError. If you're using Django's TransactionMiddleware, then this will prevent the transaction being rolled back; rollback is handled in the middleware's process_exception() method.

Is this by design? It feels to me like Django's transaction middleware should look for 5xx responses and roll back the transaction, as well as rolling back in process_exception().

Obviously, I can create my own transaction middleware with the above behaviour for my own, but was interested in what others' thoughts were on this - and particularly on whether this is a bug, and if so, whether it's in Piston or Django?

Cheers,
Dan

--
Dan Fairs <d...@fezconsulting.com> | http://www.fezconsulting.com/

jespern

unread,
Feb 26, 2010, 7:56:52 AM2/26/10
to django-piston

It's both, actually. While I would argue that Django *should* look for
5xx's, Piston is also at fault here. It's (intentionally)
circumventing parts of Django to give a better user experience.

Not sure what to do about this. Maybe warn if PISTON_DISPLAY_ERRORS is
on together with the transaction middleware?


Jesper

Dan Fairs

unread,
Feb 26, 2010, 8:28:41 AM2/26/10
to django...@googlegroups.com
> Not sure what to do about this. Maybe warn if PISTON_DISPLAY_ERRORS is
> on together with the transaction middleware?


That'd help.

Of course, it's not just the transaction middleware affected by this: it's any middleware that has a process_exception() method. Another possibility therefore might be to have Piston itself invoke process_exception() for all installed middlewares before returning the server error response, mimicking:

django/handlers/base.py:97,101

for middleware_method in self._exception_middleware:
response = middleware_method(request, e)
if response:
return response
raise

I haven't investigated far enough to figure out if there would be any other problems with doing that.

I do agree there's also an issue with Django too. I'll post a message on django-users to see what the feeling is about it there.

Cheers,
Dan

--
Dan Fairs <dan....@gmail.com> | http://www.fezconsulting.com/

Reply all
Reply to author
Forward
0 new messages