Re: Exceptions thrown from callbacks

[Craig L Russell]

On Feb 5, 2007, at 7:14 PM, Patrick Linskey wrote:

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.

spec team agreed at one point. I think that this one just slipped

that was excluded, for just this reason presumably.

I think that rolling back directly is pretty evil, and as I recall, the through the cracks. In any event, this is covered in the area of the CTS

Craig

If you get the exception in a commit, then it's appropriate to roll back immediately. If you get the exception anywhere else, I'm strongly opposed to an immediate rollback. Mark the transaction for rollback is the appropriate action.

--
Patrick Linskey
BEA Systems, Inc.


by email and then delete it.

______________________________________________________________________ _ 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

-----Original Message-----
From: Marc Prud'hommeaux [mailto:[EMAIL PROTECTED] 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:[EMAIL PROTECTED] 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
Architect, Sun Java Enterprise System http://java.sun.com/products/jdo
408 276-5638 mailto:[EMAIL PROTECTED]
P.S. A good JDO? O, Gasp!

smime.p7s
Description: S/MIME cryptographic signature