Warning: Can't synchronize with repository "(default)" (Unsupported version control system "svn": No module named svn). Look in the Trac log for more information.

Ticket #1454 (closed defect: fixed)

Opened 12 years ago

Last modified 12 years ago

InvalidRequestError not handled properly in SQLAlchemy transaction handling

Reported by: Chris Arndt Owned by: anonymous
Priority: normal Milestone: 1.0.4
Component: TurboGears Version: 1.0.3
Severity: normal Keywords:
Cc:

Description (last modified by Chris Arndt) (diff)

In database.py in function 'sa_rwt', lines 365-369 if an "InvalidRequestError" is raised, "retval" is not set.

Traceback:

  File "/Library/Frameworks/Python.framework/Versions/2.4/lib/
python2.4/site-packages/CherryPy-2.2.1-py2.4.egg/cherrypy/
_cphttptools.py", line 105, in _run
    self.main()
  File "/Library/Frameworks/Python.framework/Versions/2.4/lib/
python2.4/site-packages/CherryPy-2.2.1-py2.4.egg/cherrypy/
_cphttptools.py", line 254, in main
    body = page_handler(*virtual_path, **self.params)
  File "<string>", line 3, in index
  File "/Library/Frameworks/Python.framework/Versions/2.4/lib/
python2.4/site-packages/TurboGears-1.0.3.2-py2.4.egg/turbogears/
identity/conditions.py", line 235, in require
    return fn(self, *args, **kwargs)
  File "<string>", line 3, in index
  File "/Library/Frameworks/Python.framework/Versions/2.4/lib/
python2.4/site-packages/TurboGears-1.0.3.2-py2.4.egg/turbogears/
controllers.py", line 342, in expose
    output = database.run_with_transaction(
  File "<string>", line 5, in run_with_transaction
  File "/Library/Frameworks/Python.framework/Versions/2.4/lib/
python2.4/site-packages/TurboGears-1.0.3.2-py2.4.egg/turbogears/
database.py", line 375, in sa_rwt
    return retval
UnboundLocalError: local variable 'retval' referenced before assignment

Comment from Christoph Zwerschke:

Yes, instead of "pass" it should probably be:

retval = dispatch_exception(e,args,kw)

Also, I think the rollback() call 3 lines later should go into a try-except block, just like the commit() call.

So the whole except block could be modified like this (untested):

    except Exception, e:
        if isinstance(e, (cherrypy.HTTPRedirect,
                cherrypy.InternalRedirect)):
            try:
                req.sa_transaction.commit()
            except Exception,e:
                pass
            else:
                raise
        elif not isinstance(e, InvalidRequestError):
            try:
                req.sa_transaction.rollback()
            except Exception,e:
                pass
        retval = dispatch_exception(e,args,kw)

See also #1377

Attachments

ticket1454.patch Download (4.1 KB) - added by chrisz 12 years ago.
Suggested patch for ticket 1454

Change History

comment:1 Changed 12 years ago by Chris Arndt

  • Description modified (diff)

Changed 12 years ago by chrisz

Suggested patch for ticket 1454

comment:2 Changed 12 years ago by chrisz

If nobody sees a problem with this, I can check it in for both 1.0 and 1.1.

comment:3 Changed 12 years ago by Chris Arndt

Looks good to me.

comment:4 Changed 12 years ago by chrisz

  • Status changed from new to closed
  • Resolution set to fixed

Fixed in r3380.

Note: See TracTickets for help on using tickets.