openjpa-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Marc Prud'hommeaux <mprud...@apache.org>
Subject Re: Exceptions thrown from callbacks
Date Tue, 06 Feb 2007 02:48:49 GMT


> I don't recall the details about why we made that decision, but I  
> prefer
> wrapping in a RollbackException for consistency. It sucks to have  
> to do:

I agree that that sucks. Of course, if they want their application to  
be portable, then they'll need to put in that logic anyway :)

> Do you have any opinion about the rollback vs. mark-for-rollback  
> distinction?

Well, section 3.5 suggests that it should be rolled back immediately,  
but then if we start wrapping in a PersistenceException, then 3.7  
says that it should just be marked for rollback. Tricky. I guess I  
favor immediate rollback, just since section 3.5 is so explicit in  
saying that it should happen.



On Feb 5, 2007, at 6:27 PM, Patrick Linskey wrote:

>> As for whether to wrap the exceptions during non-flush/commit times
>> (e.g., during a find() call), I'm a little less keen on it, but not
>> really opposed. The reason is that the clause "Lifecycle callback
>> methods may throw runtime exceptions" suggests to me (and,
>> apparently, to the CTS authors) that they restricted the exception
>> type to be runtime exceptions because they expect the unmodified
>> exception will be thrown straight up the application. Summary: +0
>
> I don't recall the details about why we made that decision, but I  
> prefer
> wrapping in a RollbackException for consistency. It sucks to have  
> to do:
>
> try {
>     em.find(...);
>     em.commit();
> } catch (RollbackException re) {
>     if (re.getCause() instanceof MySpecialException) {
>         // custom logic
>     } else {
>         throw re;
>     }
> } catch (MySpecialException mse) {
>     // same custom logic as above
> }
>
>
> Do you have any opinion about the rollback vs. mark-for-rollback
> distinction?
>
> -Patrick
>
> -- 
> Patrick Linskey
> BEA Systems, Inc.
>
> ______________________________________________________________________ 
> _
> Notice:  This email message, together with any attachments, may  
> contain
> information  of  BEA Systems,  Inc.,  its subsidiaries  and   
> affiliated
> entities,  that may be confidential,  proprietary,  copyrighted   
> and/or
> legally privileged, and is intended solely for the use of the  
> individual
> or entity named in this message. If you are not the intended  
> recipient,
> and have received this message in error, please immediately return  
> this
> by email and then delete it.
>
>> -----Original Message-----
>> From: Marc Prud'hommeaux [mailto:mprudhomapache@gmail.com] On
>> Behalf Of Marc Prud'hommeaux
>> Sent: Monday, February 05, 2007 6:20 PM
>> To: open-jpa-dev@incubator.apache.org
>> Subject: Re: Exceptions thrown from callbacks
>>
>>
>> I don't have any strong opinions either way. Wrapping is useful,
>> because we can usually provide the FailedObject so that the user can
>> more easily attempt some recovery. And I do agree that if a callback
>> exception occurs during a commit()/flush() operation then we should
>> wrap it (and it might as well be in a RollbackException). Summary: +1
>>
>> As for whether to wrap the exceptions during non-flush/commit times
>> (e.g., during a find() call), I'm a little less keen on it, but not
>> really opposed. The reason is that the clause "Lifecycle callback
>> methods may throw runtime exceptions" suggests to me (and,
>> apparently, to the CTS authors) that they restricted the exception
>> type to be runtime exceptions because they expect the unmodified
>> exception will be thrown straight up the application. Summary: +0
>>
>>
>>
>>
>> On Feb 1, 2007, at 3:27 PM, Patrick Linskey wrote:
>>
>>> Hi,
>>>
>>> There's a bit of ambiguity in the spec about what should happen
>>> when an
>>> exception is thrown from a callback.
>>>
>>> I propose that we change OpenJPA's behavior to always wrap
>> exceptions
>>> thrown from callbacks in a RollbackException. Additionally,
>> I propose
>>> that if a callback is thrown from a direct flush() call or
>> a find() or
>>> other data load, we should mark the transaction for rollback
>>> instead of
>>> immediately rolling back the transaction.
>>>
>>>
>>> Details:
>>>
>>> Section 3.5 says:
>>>
>>> "Lifecycle callback methods may throw unchecked/runtime
>> exceptions. A
>>> runtime exception thrown by a callback method that executes within a
>>> transaction causes that transaction to be rolled back."
>>>
>>> 3.5.6:
>>>
>>> "Lifecycle callback methods may throw runtime exceptions. A runtime
>>> exception thrown by a callback method that executes within a
>>> transaction
>>> causes that transaction to be rolled back. No further lifecycle
>>> callback
>>> methods will be invoked after a runtime exception is thrown."
>>>
>>> 3.7:
>>>
>>> "The PersistenceException is thrown by the persistence
>> provider when a
>>> problem occurs. [...] All instances of PersistenceException
>> except for
>>> instances of NoResultException and NonUniqueResultException
>> will cause
>>> the current transaction, if one is active, to be marked for
>> rollback."
>>>
>>> ...
>>>
>>> "The RollbackException is thrown by the persistence provider when
>>> EntityTransaction.commit() fails.
>>>
>>>
>>> So.... in my opinion, this means that if a callback fails during
>>> commit(), the exception thrown by the callback clearly should be
>>> wrapped
>>> in a RollbackException. It is less clear to me what should
>> happen if a
>>> callback fails at some other time, such as during a find()
>> call. In my
>>> opinion, we should be wrapping the user-thrown exceptions in
>>> RollbackExceptions all the time.
>>>
>>> Further, I think that 3.7 trumps 3.5.6, so if an exception is thrown
>>> from a callback during a find(), we should be marking the
>> transaction
>>> for rollback, rather than actually rolling it back.
>>>
>>> I've got changes in place that implement the behavior I just
>>> described.
>>> The CTS tests surrounding this issue have been excluded, due to the
>>> ambiguity in the spec.
>>>
>>> Thoughts?
>>>
>>> -Patrick
>>>
>>> -- 
>>> Patrick Linskey
>>> BEA Systems, Inc.
>>>
>>>
>> ______________________________________________________________
>> ________
>>> _
>>> Notice:  This email message, together with any attachments, may
>>> contain
>>> information  of  BEA Systems,  Inc.,  its subsidiaries  and
>>> affiliated
>>> entities,  that may be confidential,  proprietary,  copyrighted
>>> and/or
>>> legally privileged, and is intended solely for the use of the
>>> individual
>>> or entity named in this message. If you are not the intended
>>> recipient,
>>> and have received this message in error, please immediately return
>>> this
>>> by email and then delete it.
>>
>>


Mime
View raw message