[Zope3-dev] Please, no bare 'except:' clauses!

[Shane Hathaway]

This is a multi-part message in MIME format.
--------------060809060406050907090507
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit

Jim Fulton wrote:
> Shane Hathaway wrote:
> 
>> On Mon, 11 Nov 2002, Jim Fulton wrote:
>>
>>
>>>> 1) Don't let DatabaseErrors be so special any more.  Currently, we want
>>>> ConflictErrors to propagate, in order to prevent inconsistent data from
>>>> being committed, and to give the app an opportunity to retry.  But
>>>> excessive exception catching ought not to be so catastrophic.  I think
>>>> any attempt to write or commit after a ConflictError has occurred 
>>>> should
>>>> result in another ConflictError, since the data being written is
>>>> generated from inconsistent data.  AFAICT this would eliminate the need
>>>> to make DatabaseErrors special, and would only change ZODB.
>>>>
>>> This is already the case.
>>>
>>
>> In ZODB 4?
> 
> 
> In ZODB 3. I'm not sure about ZODB 4. In ZODB, a read conflict doesn't
> abort the current transaction. It causes the object's data to remain
> unloaded. Any additional attempts to read the data will generate more
> conflict errors.

Well, I just wrote a little program to find out.  After a ConflictError, 
ZODB 3 lets the application continue as if no conflict has happened. 
Jeremy and I agreed that it should not be possible to commit a 
transaction in which a ConflictError has at any time propagated to the 
application.  ZODB 3 lets conflicting data get committed.

The test program is attached.  It runs two threads.  The data written to 
root['data'] is intended to generate a conflict error (this is good). 
The data written to root[container_name] is not intended to generate a 
conflict error (this is also good).  When the program attempts to write 
conflicting data, all data gets reverted to the original state (this is 
okay).

The data written to root['container3'] is the problem.  If the 
application catches a conflict error and ignores it, it is able to write 
more data, and the new data could be inconsistent with the rest of the 
data.  Here is the output of the test program:

About to commit.  Root data: 'apple', container1 data: 'apple'
Committed.  Root data: 'apple', container1 data: 'apple', container3 
data: 'apple'
About to commit.  Root data: 'orange', container2 data: 'orange'
Got a conflict.  Ignoring.
Committed.  Root data: 'apple', container2 data: None, container3 data: 
'orange'

Everything is fine until the last line.  The app should not have been 
allowed to write "orange" to container3, because that value is out of 
sync with the rest of the data.

This is a real problem.  The "unindex" method of various types of 
ZCatalog indexes currently catch all exceptions, including 
ConflictErrors, allowing changes to the catalog that don't correspond 
with changes to the data or other indexes.  Lots of other places can 
catch all exceptions: "tal:on-error", untrusted Python scripts and DTML 
methods, error reporting, code written by customers, etc.  All these can 
make the database inconsistent.  It doesn't feel right.

Shane

--------------060809060406050907090507
Content-Type: text/plain;
 name="demo.py"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="demo.py"


import time, thread
from ZODB.FileStorage import FileStorage
from ZODB.POSException import ConflictError
from ZODB.DB import DB
from Persistence import PersistentMapping

db = DB(FileStorage('test.fs', create=1))
conn = db.open()
conn.root()['container1'] = PersistentMapping()
conn.root()['container2'] = PersistentMapping()
conn.root()['container3'] = PersistentMapping()
get_transaction().commit()
conn.close()

def changeApp(wait_time, container_name, data):
    get_transaction().begin()
    conn = db.open()
    root = conn.root()
    container = root[container_name]
    container['data'] = data
    root['data'] = data
    time.sleep(wait_time)
    print 'About to commit.  Root data: %r, %s data: %r' % (
        root['data'], container_name, container.get('data'))
    try:
        get_transaction().commit()
    except ConflictError:
        print 'Got a conflict.  Ignoring.'
    root['container3']['data'] = data
    get_transaction().commit()
    print 'Committed.  Root data: %r, %s data: %r, container3 data: %r' % (
        root['data'], container_name, container.get('data'),
        root['container3'].get('data'))

# Generate a conflict
thread.start_new_thread(changeApp, (1, 'container1', 'apple'))
changeApp(2, 'container2', 'orange')


--------------060809060406050907090507--