If we do the following in a controller method (for instance, to catch duplicate ids)
try:
session.save(user)
session.flush()
except Exception, e:
flash("Error occurred")
return dict(message="End of the line")
we create a situation where TurboGears can throw an error inside of database.py. This is bad as there is no way to catch that error inside of the application code so it propagates out to the web page.
Here's the relevant sections of database.py:
# include "args" to avoid call being pre-cached
[run_with_transaction.when("_use_sa(args)")]
def sa_rwt(func, *args, **kw):
log.debug("New SA transaction")
req = cherrypy.request
req.sa_transaction = make_sa_transaction(session)
try:
retval = func(*args, **kw)
except (cherrypy.HTTPRedirect, cherrypy.InternalRedirect):
log.debug('this is only a redirect')
# If a redirect happens; commit and proceed with redirect
if sa_tr_active(req.sa_transaction):
req.sa_transaction.commit()
raise
except:
log.debug('this is an exception, ROLLBACK now!')
# If any other exception happens; rollback and re-raise error
if sa_tr_active(req.sa_transaction):
req.sa_transaction.rollback()
raise
# If the call was successful; commit and proceed
if sa_tr_active(req.sa_transaction):
log.debug('The transaction was successful, COMMIT now!')
req.sa_transaction.commit()
return retval
[...]
def sa_tr_active(tr):
if hasattr(session, 'context'):
# SA 0.3
return tr.session.transaction
else:
# SA 0.4 (effectively always active - commit or rollback don't fail)
return True
The comment in sa_tr_active() is suspicious as the SQLAlchemy has this to say about transactions:
When using a transactional session, either a rollback() or a close() call is required when an error is raised by flush() or commit(). The flush() error condition will issue a ROLLBACK to the database automatically, but the state of the Session itself remains in an "undefined" state until the user decides whether to rollback or close.
http://www.sqlalchemy.org/docs/04/session.html#unitofwork_managing
I'm not sure if this should be fixed by changing sa_tr_active() to return False when the transaction has encountered an error that should be rolled back or if sa_rwt() should catch errors when it does a commit() and do a rollback() instead but one of those two places needs to change.
