RE: Exceptions thrown from callbacks
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?
-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:[EMAIL PROTECTED]
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:[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
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:[EMAIL PROTECTED] 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:[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
Re: Exceptions thrown from callbacks
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. I can go either way on it, but I think Craig's reasoning is sound. +1 ___ 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.
Re: Exceptions thrown from callbacks
Ah, the perils of spec-writing... On Feb 1, 2007, at 3:55 PM, Dain Sundstrom wrote: On Feb 1, 2007, at 3:27 PM, Patrick Linskey wrote: 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. One other thing to check is if the tx is marked rollback only, should we not call anymore lifecycle events or is that No Further lifecycle callback clause supposed to apply to the single entity that failed. More decisions. Is it 1. After a runtime exception is thrown (during any em method call) the tx is marked for rollback and no callbacks are called on any entity for any method 2. After a runtime exception is thrown from a user-defined callback, the tx is marked for rollback and no callbacks are called on any entity for any method 3. After a runtime exception is thrown from a user-defined callback, the tx is marked for rollback and no callbacks are called on the entity that stimulated the exception 4. After a runtime exception is thrown from a user-defined callback, the tx is marked for rollback and no callbacks are called on any entity during the execution of the current entity manager method 5. The provider decides what to do 6. Something else Craig Russell DB PMC [EMAIL PROTECTED] http://db.apache.org/jdo smime.p7s Description: S/MIME cryptographic signature
RE: Exceptions thrown from callbacks
It seems like we've reached consensus around the following behavior at least: 1. if a callback throws an exception during commit(), wrap in RollbackException and roll back the transaction. 2. if a callback throws an exception during some non-commit() operation, do not wrap the exception, and mark the transaction for rollback. It turns out that this is exactly the current behavior. I just checked in some test cases to exercise this, since the CTS has an outage here now. -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 Sent: Thursday, February 01, 2007 3:27 PM To: open-jpa-dev@incubator.apache.org Subject: Exceptions thrown from callbacks 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.
RE: Exceptions thrown from callbacks
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
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
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:[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
Exceptions thrown from callbacks
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.
Re: Exceptions thrown from callbacks
On Feb 1, 2007, at 3:27 PM, Patrick Linskey wrote: 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. One other thing to check is if the tx is marked rollback only, should we not call anymore lifecycle events or is that NoFurther lifecycle callback clause supposed to apply to the single entity that failed. 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. That is standard EJB behavior and what I would expect from the EJB spec committee. -dain
RE: Exceptions thrown from callbacks
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. One other thing to check is if the tx is marked rollback only, should we not call anymore lifecycle events or is that NoFurther lifecycle callback clause supposed to apply to the single entity that failed. You mean: if someone calls setRollbackOnly() and then calls flush(), what should we do about the @PreUpdate callback? I don't have a strong opinion either way. My read of the spec is that we should execute the callback; callbacks should only be aborted if the exception is thrown during a time when the callbacks would have otherwise been invoked. 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. That is standard EJB behavior and what I would expect from the EJB spec committee. Agreed. -Patrick ___ 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.
