openjpa-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Patrick Linskey" <plins...@bea.com>
Subject RE: Exceptions thrown from callbacks
Date Tue, 06 Feb 2007 20:44:08 GMT
> We should ask that the spec be clarified so applications can be more  
> portable.

I've done this, and have been told that this issue will be included in
the JPA 2.0 process.

-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: Craig.Russell@Sun.COM [mailto:Craig.Russell@Sun.COM] 
> Sent: Tuesday, February 06, 2007 12:12 PM
> To: open-jpa-dev@incubator.apache.org
> Subject: Re: Exceptions thrown from callbacks
> 
> 
> On Feb 6, 2007, at 11:27 AM, Patrick Linskey wrote:
> 
> > Out-of-band, Abe pointed out to me that the text about when
> > RollbackExceptions are thrown is pretty clear. 3.7 says:
> >
> > "The RollbackException is thrown by the persistence provider when
> > EntityTransaction.commit fails."
> >
> > So, it would seem that in the case where an EM.find() or some other
> > non-commit() call triggers a callback that throws an exception, we
> > should not wrap the exception in a RollbackException. Should we just
> > throw the raw exception, or should we wrap in some other
> > PersistenceException?
> 
> I think if a user throws an exception in a callback outside of a  
> commit operation, we should simply rethrow it to the user after we  
> clean up our internal state. Presumably, the specific runtime  
> exception has meaning to the user's code, and we don't add 
> much value  
> in wrapping the exception.
> 
> I might think differently if the spec allowed/required us to 
> continue  
> after catching the first exception thrown from a callback, but it  
> explicitly says that the first exception aborts the operation. From  
> 3.5.6,  "No further lifecycle callback methods will be invoked after  
> a runtime exception is thrown."
> 
>  From that same paragraph, "A runtime exception thrown by a callback  
> method that executes within a transaction causes that transaction to  
> be rolled back." But I think it's ok to set RollbackOnly to satisfy  
> this requirement. And as a matter of design (separation of concerns)  
> this really is best practice.
> 
> We should ask that the spec be clarified so applications can be more  
> portable.
> 
> Craig
> >
> > -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: Patrick Linskey [mailto:plinskey@bea.com]
> >> Sent: Monday, February 05, 2007 7:14 PM
> >> To: open-jpa-dev@incubator.apache.org
> >> Subject: RE: Exceptions thrown from callbacks
> >>
> >>>> 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 :)
> >>
> >> Agreed, and we may need to change our behavior later once the
> >> spec team
> >> clears this up. I'll be lobbying for the spec team to move to the
> >> always-wrapped model, though.
> >>
> >>>> 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.
> >>
> >> I think that rolling back directly is pretty evil, and as I
> >> recall, the
> >> spec team agreed at one point. I think that this one just slipped
> >> through the cracks. In any event, this is covered in the area
> >> of the CTS
> >> that was excluded, for just this reason presumably.
> >>
> >> -- 
> >> 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:49 PM
> >>> To: open-jpa-dev@incubator.apache.org
> >>> Subject: Re: Exceptions thrown from callbacks
> >>>
> >>>
> >>>
> >>>> 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.
> >>>>>
> >>>>>
> >>>
> >>>
> >>
> 
> Craig Russell
> DB PMC
> clr@apache.org http://db.apache.org/jdo
> 
> 
> 

Mime
View raw message