Ticket #1721 (closed defect: invalid)
[PATCH] With SA-0.4, catching an exception on a database can cause exceptions in TG.
|Reported by:||toshio||Owned by:||anonymous|
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.
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.
- Summary changed from With SA-0.4, catching an exception on a database can cause exceptions in TG. to [PATCH] With SA-0.4, catching an exception on a database can cause exceptions in TG.
comment:14 Changed 9 years ago by kvdb
- Priority changed from high to normal
- Status changed from closed to reopened
- Resolution fixed deleted