[ZODB-Dev] misleading exception

[Tim Peters]

[dvd]
> Hi all, I'm still working with savepoints and today I spend about tewo
> hours to find an error, misleaded by a strange Exception

As Dieter said (thanks, Dieter!), the current transaction should always be
explicitly committed or aborted before closing any Connection involved in
the transaction.  ZODB tries to raise ConnectionStateError if you close a
Connection without explicitly finalizing the transaction, but something
about savepoints isn't getting caught here.  I'm not positive it _should_
raise ConnectionStateError in this case.  But, if it doesn't, then it should
guarantee to clean up all the bits left behind by the savepoint machinery.

I opened a bug report for you, with a simplified test case, at

    http://www.zope.org/Collectors/Zope/1941

Note that the test case does minimal logging setup.  This is because your
test generates an ERROR log message that you didn't see, pointing closer to
the true cause of the problems:

"""
ERROR:txn.3296:Error in tpc_abort() on manager <Connection at 00a26c10>
Traceback (most recent call last):
  File "C:\code\zodb3.5\src\transaction\_transaction.py", line 474, in
_cleanup
    rm.tpc_abort(self)
  File "C:\code\zodb3.5\src\ZODB\Connection.py", line 620, in tpc_abort
    self._storage.tpc_abort(transaction)
  File "C:\code\zodb3.5\src\ZODB\BaseStorage.py", line 193, in tpc_abort
    self._abort()
  File "C:\code\zodb3.5\src\ZODB\FileStorage\FileStorage.py", line 939, in
_abort
    if self._nextpos:
AttributeError: 'FileStorage' object has no attribute '_nextpos'
"""

Following are just random hints:

> ...
>     try: os.unlink(fname)
>     except Exception: pass
>     try: os.unlink(fname + '.index')
>     except Exception: pass
>     try: os.unlink(fname + '.tmp')
>     except Exception: pass

This code didn't accomplish anything for you, because `os` wasn't imported
by the time it ran.  The catch-all "except Exception:" clauses suppressed
the NameError on `os`.  The implementation of FileStorage.cleanup()
demonstrates safer practice:

    def cleanup(self):
        """Remove all files created by this storage."""
        for ext in '', '.old', '.tmp', '.lock', '.index', '.pack':
            try:
                os.remove(self._file_name + ext)
            except OSError, e:
                if e.errno != errno.ENOENT:
                    raise

BTW, your code used a mixture of tabs and spaces for indentation.  That's a
Bad Idea for code posted to a mailing list.  All-tabs or all-spaces works
much better.  The Python and Zope projects' standards mandate 4-space
indents (with no hard tab characters).

>     fstorage = FileStorage.FileStorage(fname)
>     db = DB(fstorage)
>     try:
>         conn = db.open()
>         try:
> 	    try:

That's an example of why mixing tabs with spaces sucks.  I can't know what
that looks like in _your_ email reader, but in _mine_ it looks like both
"try:" statements are indented the same.

> 		    transaction.get().commit()

`transaction.commit()` is a shortcut way to spell that.

> 	    transaction.get().abort()

Likewise `transaction.abort()` is a shortcut for that.

>     finally:
>         db.close()
>         fstorage.close()

While I think this is obscure, closing a DB automatically closes the storage
it wraps.  My natural inclination is also to close both explicitly, but I
get ragged on for that -- the `fstorage.close()` isn't necessary.

> ...
> The strange is that the exception is raised when i try to commit the
> transaction in the second call of CreateDatabase

You confused the transaction machinery, that's all <wink>.