Zope's current transaction behaviour is essentially: ## request starts transaction.begin() try: object= REQUEST.traverse(...) mapply(object,...) transaction.commit() except: transaction.abort() handle_error() ## request ends This is flawed as error handling is done outside of a transaction. Potential changes during the error handling spill over uncontrolled into another request and are there either committed or aborted as part of this request. Andrew Athan (<mailto:aathan@memeplex.com>) has had lots of serious inconsistencies in Zope's session data. After extensive analysis, he found out that reading the session data during error handling led to these error conditions (reading session data causes writes to the administrative data). I suggest, we let Zope perform error handling in its own transaction after the original transaction had been aborted. When error handling succeeds, its transaction is committed, otherwise aborted. The new behaviour would look something like this: ## request starts transaction.begin() try: object= REQUEST.traverse(...) mapply(object,...) transaction.commit() except: transaction.abort() transaction.begin() transaction.note('%s (application error handling)' % '/'.join(object.getPhysicalPath) ) try: handle_error() transaction.commit() except: default_handle_error() # Zope's default error handling # it should not have side effects # and is executed inside the # error handling transaction transaction.abort() ## request ends Dieter
I am +1 on this. I suspect that before committing, though, we should ask people who make use of Zope's transaction manager in "advanced" ways like Phillip Eby. - C On Sun, 2003-02-02 at 10:40, Dieter Maurer wrote:
The new behaviour would look something like this:
## request starts transaction.begin() try: object= REQUEST.traverse(...) mapply(object,...) transaction.commit() except: transaction.abort() transaction.begin() transaction.note('%s (application error handling)' % '/'.join(object.getPhysicalPath) ) try: handle_error() transaction.commit() except: default_handle_error() # Zope's default error handling # it should not have side effects # and is executed inside the # error handling transaction transaction.abort() ## request ends
Chris McDonough wrote:
I am +1 on this. I suspect that before committing, though, we should ask people who make use of Zope's transaction manager in "advanced" ways like Phillip Eby.
As I noted in the Collector issue, I believe this change is a good idea overall. As Chris suggests, we should take note of users of ZPatterns and TransactionAgents (and other add-ons?), as these do things at transaction boundaries such as clear computed fields. The error handlers currently in use may expect these computed fields to be still available. By conincidence I'm implementing something similar for error handling in Zope 3 right now. Here's the kind of thing I'm doing in zope 3, using the same notation Dieter used. First, the simple case where the error handler will definitely not need to alter any state: ## request starts transaction.begin() try: object= REQUEST.traverse(...) result = mapply(object,...) transaction.commit() except: try: try: result = handle_error() except: result = default_handle_error() # Zope's default error handling # it must not have side effects finally: transaction.abort() request.response.setBody(result) ## request ends If there's an exception handler that has side-effects, it must explicitly say so, and we get this: ## request starts transaction.begin() try: object= REQUEST.traverse(...) result = mapply(object,...) transaction.commit() except: try: try: # this call returns a special # 'I have side-effects' token. result = handle_error() except: result = default_handle_error() # Zope's default error handling # it should not have side effects finally: transaction.abort() if result is I_HAVE_SIDE_EFFECTS: transaction.begin() transaction.note('%s (application error handling)' % '/'.join(object.getPhysicalPath)) try: result = handle_error_with_sideeffects() transaction.commit() except: try: result = default_handle_error() # Zope's default error handling # it should not have side effects finally: transaction.abort() else: request.response.setBody(result) ## request ends -- Steve Alexander
All that's fine by me, and allowing an error handler to have side effects explicitly is probably a good idea... will anybody volunteer to do this for 2.7? - C On Mon, 2003-02-03 at 03:44, Steve Alexander wrote:
By conincidence I'm implementing something similar for error handling in Zope 3 right now.
Here's the kind of thing I'm doing in zope 3, using the same notation Dieter used. First, the simple case where the error handler will definitely not need to alter any state:
## request starts transaction.begin() try: object= REQUEST.traverse(...) result = mapply(object,...) transaction.commit() except: try: try: result = handle_error() except: result = default_handle_error() # Zope's default error handling # it must not have side effects finally: transaction.abort() request.response.setBody(result) ## request ends
If there's an exception handler that has side-effects, it must explicitly say so, and we get this:
## request starts transaction.begin() try: object= REQUEST.traverse(...) result = mapply(object,...) transaction.commit() except: try: try: # this call returns a special # 'I have side-effects' token. result = handle_error() except: result = default_handle_error() # Zope's default error handling # it should not have side effects finally: transaction.abort()
if result is I_HAVE_SIDE_EFFECTS: transaction.begin() transaction.note('%s (application error handling)' % '/'.join(object.getPhysicalPath)) try: result = handle_error_with_sideeffects() transaction.commit() except: try: result = default_handle_error() # Zope's default error handling # it should not have side effects finally: transaction.abort() else: request.response.setBody(result) ## request ends
Chris McDonough wrote:
I am +1 on this. I suspect that before committing, though, we should ask people who make use of Zope's transaction manager in "advanced" ways like Phillip Eby.
I've spoken to Phillip Eby, and he sees no problem with the approaches we've discussed. So, +1 from me. -- Steve Alexander
On Sunday 02 February 2003 3:40 pm, Dieter Maurer wrote:
This is flawed as error handling is done outside of a transaction.
Excellent analysis. A futher problem is that this could cause dangling references, and a subsequent POSKeyError, since persistent objects can be passed from one transaction to the next inside the exception and traceback. The same applies to your prorosed fix. Is there a need to allow the error handling transaction to commit? I propose it always be aborted. -- Toby Dickenson http://www.geminidataloggers.com/people/tdickenson
Toby Dickenson schrieb:
On Sunday 02 February 2003 3:40 pm, Dieter Maurer wrote:
This is flawed as error handling is done outside of a transaction.
Excellent analysis. A futher problem is that this could cause dangling references, and a subsequent POSKeyError, since persistent objects can be passed from one transaction to the next inside the exception and traceback.
The same applies to your prorosed fix. Is there a need to allow the error handling transaction to commit? I propose it always be aborted.
When exactly can we get these dangling references? We are experiencing POSKeyErrors quite frequently these days. It's always the same: Some objects seem to become "dangling references" because of something we don't know yet. It's always objects that have been commited recently, though I don't know exactly if there always is an error involved that might interrupt the commit in some way (No feedback from the users). Then when the database is packed the objects are removed, and POSKeyErrors are the result. We are heavily depending on sessions, so the scenario you are describing could be our problem. How hard would it be to get this patched for Zope 2.6.1 final? At least as an option that can be activated when needed? -- iuveno AG Joachim Werner _________________ Wittelsbacherstr. 23b 90475 Nürnberg joachim.werner@iuveno.de www.iuveno.de Tel.: +49 (0) 911/ 9 88 39 84
On Mon, 2003-02-03 at 11:11, Joachim Werner wrote:
We are heavily depending on sessions, so the scenario you are describing could be our problem.
Are you making a reference to a session (or another persistent object) in your standard_error_message? If not, this is probably not the problem. - C
Toby Dickenson wrote at 2003-2-3 10:40 +0000:
On Sunday 02 February 2003 3:40 pm, Dieter Maurer wrote:
This is flawed as error handling is done outside of a transaction.
Excellent analysis. A futher problem is that this could cause dangling references, and a subsequent POSKeyError, since persistent objects can be
passed from one transaction to the next inside the exception and tracebac k.
The same applies to your prorosed fix. I see...
Difficult to handle when the error handling expects a true traceback object -- unless we postpone the transaction abort until error handling is complete. But then, error handling can not commit things.
Is there a need to allow the error handling transaction to commit? I propose it always be aborted. I think it can be useful:
* a colleague uses Jens' transactional mail host (throughout Zope) and he sends error mails out of "standard_error_message". When error handling actions would be aborted, he would need to use a non transactional mail host * in an earlier project, we put error records in "standard_error_message" into a relational database in order to use SQL for statistical evaluation. If it is impossible, these use cases can be handled differently, of course... Dieter
Your basic point makes sense, although I'm not entirely clear on how transaction management is integrated into the Zope application. Speaking for ZODB alone, I believe we've recommended that people call get_transaction().abort() if they catch an exception. I can't recall getting into any nuances of error handling, but it certainly makes sense to call abort() after the error handling. Note that a new transaction is begin implicitly any time a persistence object is referenced. So the error handling code does execute in the context of a transaction. It may be that it's just not the transaction you'd like it to be in. I assume that some work done during error handling is getting committed with the next successful request. Yuck. How does user-level error handling get invoked in Zope? Jeremy
Jeremy Hylton wrote at 2003-2-3 10:45 -0500:
... Note that a new transaction is begin implicitly any time a persistence object is referenced. So the error handling code does execute in the context of a transaction. I know... It may be that it's just not the transaction you'd like it to be in. I assume that some work done during error handling is getting committed with the next successful request. Yuck. That's the problem.
How does user-level error handling get invoked in Zope? From "ZPublisher.Publish". It essentially executes the code outlined in my original post:
transaction.begin() try: main_action() transaction.commit() except: transaction.abort() handle_error() There is some magic to determine "main_action" and "handle_error" irrelevant to our current discussion. Dieter
On Sunday 02 February 2003 3:40 pm, Dieter Maurer wrote:
Zope's current transaction behaviour is essentially:
1 ## request starts 2 transaction.begin() 3 try: 4 object= REQUEST.traverse(...) 5 mapply(object,...) 6 transaction.commit() 7 except: 8 transaction.abort() 9 handle_error() 10 ## request ends
This is flawed as error handling is done outside of a transaction.
Potential changes during the error handling spill over uncontrolled into another request and are there either committed or aborted as part of this request.
A visit to the dentist has given me some time to think about this, and I think there is a flaw in this explanation. Surely any changes that leak out from one request will will be be aborted before they can do any damage, during the call to transaction.begin() on line 2. The doc string from transaction.begin is: '''Begin a new transaction. This aborts any transaction in progres. ''' (I should have a chance to experiment with this tomorrow) -- Toby Dickenson http://www.geminidataloggers.com/people/tdickenson
Toby Dickenson wrote at 2003-2-3 17:04 +0000:
On Sunday 02 February 2003 3:40 pm, Dieter Maurer wrote:
Zope's current transaction behaviour is essentially:
1 ## request starts 2 transaction.begin() 3 try: 4 object= REQUEST.traverse(...) 5 mapply(object,...) 6 transaction.commit() 7 except: 8 transaction.abort() 9 handle_error() 10 ## request ends
This is flawed as error handling is done outside of a transaction.
Potential changes during the error handling spill over uncontrolled into another request and are there either committed or aborted as part of this request. ...
and I think there is a flaw in this explanation. Surely any changes that leak out fro m one request will will be be aborted before they can do any damage, during the call to transaction.begin() on line 2.
The doc string from transaction.begin is:
'''Begin a new transaction.
This aborts any transaction in progres. '''
(I should have a chance to experiment with this tomorrow) Then, we must find another explanation for Andrews observation:
He observed heavily inconsistent (Zope) session data when he accessed the session during error handling. The problem went away when he did not do that. @Andrew: Can you check your log files whether the events would be consistent with rollbacks at the start of the next request. Dieter
On Mon, 2003-02-03 at 14:12, Dieter Maurer wrote:
Then, we must find another explanation for Andrews observation:
He observed heavily inconsistent (Zope) session data when he accessed the session during error handling. The problem went away when he did not do that.
@Andrew: Can you check your log files whether the events would be consistent with rollbacks at the start of the next request.
I don't think the symptoms are consistent with this, at least from what I understand. It seemed from Andrew's observations that the data put into a session from earlier transactions was lost too (not just the data put in during the transaction during the request which invoked the error handler, nor just the data put into the database via the transaction which was running concurrently with the aborted transaction). It was as if the internal data structures of the session data container were becoming out of sync. I don't have a particularly good explanation for this behavior. - C
participants (6)
-
Chris McDonough -
Dieter Maurer -
jeremy＠zope.com -
Joachim Werner -
Steve Alexander -
Toby Dickenson