Issue #1118 open

Error in code example

Sebastian Rahlf
created an issue

If I'm not mistaken, there is an error in http://docs.cherrypy.org/stable/concepts/dispatching.html#replacing-page-handlers

It should read {{{

!python

to_skip = (KeyboardInterrupt, SystemException, cherrypy.HTTPRedirect) def PgSQLWrapper(next_handler, *args, kwargs): trans.begin() try: result = next_handler(*args, kwargs) trans.commit() except Exception, e: if not isinstance(e, to_skip): trans.rollback() raise # <---- this indent was wrong trans.end() return result

cherrypy.tools.pgsql = cherrypy._cptools.HandlerWrapperTool(PgSQLWrapper) }}}

Comments (2)

  1. Jason R. Coombs
    • changed status to open

    Hmm. I see several other issues with the example.

    1. KeyboardInterrupt is not a subclass of Exception, so it will never be caught, even though the code seems to imply that it would be.
    2. There's no such exception SystemException (unless you're programming in Java or .Net).
    3. It's not clear why one would not want to rollback the transaction on any exception.
    4. It's better to use the Python exception syntax for filtering exceptions.

    I believe you're right about the indentation as well. The raise statement should probably only be invoked when the transaction is rolled back. This would ensure that one of .end() or .rollback() is called in all cases, which seems like a reasonable expectation.

    Based on these issues, I've rewritten the example as:

    def PgSQLWrapper(next_handler, *args, **kwargs):
        trans.begin()
        try:
            result = next_handler(*args, **kwargs)
            trans.commit()
        except cherrypy.HTTPRedirect:
            # a redirect is considered normal behavior, so...
            pass
            # Is there a reason not to commit the transaction?
        except Exception, e:
            # an unexpected error occurred, roll back the transaction
            trans.rollback()
            raise
        trans.end()
        return result
    
    cherrypy.tools.pgsql = cherrypy._cptools.HandlerWrapperTool(PgSQLWrapper)
    
    

    This code addresses the five identified issues, but raises a sixth. Is it reasonable not to commit or end the transaction on an HTTPRedirect?

  2. Sebastian Rahlf reporter

    Frankly, I only spotted the indentation error.

    In Python 2.5 they changed a few things around: SystemExit, KeyboardInterrupt, GeneratorExit and Exception all inherit from BaseException. So how about this:

    def PgSQLWrapper(next_handler, *args, **kwargs):
        trans.begin()
        try:
            result = next_handler(*args, **kwargs)
            trans.commit()
        except cherrypy.HTTPRedirect:
            # a redirect is considered normal behavior, so...
            trans.end()
        except (SystemExit, KeyboardInterrupt, GeneratorExit):
            trans.rollback()
        except Exception, e:
            # an unexpected error occurred, roll back the transaction
            trans.rollback()
            raise
        else:
            trans.end()
        return result
    

    Disclaimer: I've started to work through the tutorial just now. So whether or not it is better to commit or end the transaction on an HTTPRedirect is beyond me.

  3. Log in to comment