-
1. Re: Question about HHH-4721
hernanbolido Mar 9, 2010 5:02 PM (in response to hernanbolido)When hibernate executes the transaction synchronization mechanism, any exception throwed by any implementation of the Synchronization interface (like AuditSync) is discarded by hibernate. So the exception does not propagates.
I don´t know if hibernate synchronization mechanism works like this and maybe envers has to use another mechanism... Or it´s a bug on hibernate and in that case envers is doing the right thing.
Here is AudisSync.beforeCompletion() catch and finally blocks
........
} catch (RuntimeException e) {
// Rolling back the transaction in case of any exceptions
//noinspection finally
try {
if (session.getTransaction().isActive()) {
session.getTransaction().rollback();
}
} finally {
//noinspection ThrowFromFinallyBlock
throw e;
}
}And here is JDBCTransaction method (where the exception throwed is discard):
private void notifyLocalSynchsBeforeTransactionCompletion() {
if (synchronizations!=null) {
for ( int i=0; i<synchronizations.size(); i++ ) {
Synchronization sync = (Synchronization) synchronizations.get(i);
try {
sync.beforeCompletion();
}
catch (Throwable t) {
log.error("exception calling user Synchronization", t);
}
}
}
}What do you think?
-
2. Re: Question about HHH-4721
adamw Mar 10, 2010 5:38 AM (in response to hernanbolido)I agree that maybe it would be nicer if the exception propagated (although I'm not sure on why such a design decision was made and if it was made with consciousness). However:
1. in case of any exception, the transaction is rolledback in AuditSync, so no data will be written
2. the exception is logged, so it doesn't disappear entirely
So it's not really critical, is it?
Adam
-
3. Re: Question about HHH-4721
hernanbolido Mar 10, 2010 6:37 AM (in response to adamw)Hi Adam! Thanks for your time.
The problem I can see is that the application using hibernate and envers doesn´t realized about the rollbacked transaction (because there is no exception to catch). So, you can´t tell the user that the operation failed...
Thanks. Hernán.
-
4. Re: Question about HHH-4721
adamw Mar 10, 2010 7:05 AM (in response to hernanbolido)But isn't an exception thrown when the transaction is commited? (Which can't happen, as it's rolledback, so commit should throw an exception.)
Adam
-
5. Re: Question about HHH-4721
hernanbolido Mar 10, 2010 9:07 AM (in response to adamw)Hi Adam!
That´'s the point... The trqnsaction commit executed after AuditSync doesn't throw any exception, so the execution follows the normal (transaction commit ok) behavior.
Here is a patch with a simple test case for this. The revision table is dropped to force a sql error inside AuditSync.beforeCompletion().
I hope it helps.
Thanks! Hernán.
-
6. Re: Question about HHH-4721
adamw Mar 11, 2010 6:11 AM (in response to hernanbolido)Ah right, there is in fact a test for this: ExceptionListener, but it only checks that the data is not commited.
But the user isn't notified, you're right.
I'll ask the other Hibernate guys why is this so and get back to you.
Adam
-
7. Re: Question about HHH-4721
hernanbolido Mar 11, 2010 7:01 AM (in response to adamw)Great!
Maybe a new exception could be added to the catch block in JDBCTransaction.notifyLocalSynchsBeforeTransactionCompletion() method indicating that this exception must be throwed. Then, AuditSync doesn´t need to execute the rollback and let the up layer do that. It´s an idea... but i don´t know if there are other issues to care about. Hibernate team can clarify this.
Transaction and Synchronization interfaces from jta api don´t make any assumptions or put restrictions about what to do in case of errors inside synchornizers, so, it´s a implementation desicion.
Thanks for the feedback. I would like to help here if you need.
Hernán.
-
8. Re: Question about HHH-4721
hernanbolido Mar 18, 2010 10:37 AM (in response to hernanbolido)Hi!
Adam, did you get any feedback from hibernate team in relation to this issue?
Thanks. Hernán.
-
9. Re: Question about HHH-4721
adamw Mar 24, 2010 4:55 PM (in response to hernanbolido)Sorry, I'm out doing trainings these weeks, so I didn't have much time, only the evenings
I posted a question on the mailing list, I'll let you know if there are any answers, or you can of course watch yourself.
Adam
-
10. Re: Question about HHH-4721
hernanbolido Mar 24, 2010 7:28 PM (in response to adamw)Hi Adam! First of all, thanks for your time!
I made that question because I can´t found anything related to this issue in the mailing list. Maybe I'm watching in the wrong place...
Thanks again. Hernán.
-
11. Re: Question about HHH-4721
adamw Mar 25, 2010 4:58 PM (in response to hernanbolido)It's the hibernate-dev mailing list.
Adam
-
12. Re: Question about HHH-4721
hernanbolido Mar 25, 2010 6:34 PM (in response to adamw)Hello!
I think I'm looking in the correct place: http://lists.jboss.org/pipermail/hibernate-dev/
But I found one mail with you as the author (february and march) and it´s about the web site migration...
-
13. Re: Question about HHH-4721
adamw Mar 26, 2010 10:52 AM (in response to hernanbolido)Hmm, weird, maybe it takes some time for the messages to appear on the list. Anyway, I didn't receive any responses yet. I'll ask on IRC next week, as I'll be finished with the trainings then .
Adam
-
14. Re: Question about HHH-4721
hernanbolido Apr 6, 2010 6:30 PM (in response to adamw)Hi Adam!
I saw that the probem was fixed using BeforeTransactionCompletionProcess instead of Synchronization!
That's great!
Do you think this solution could be pushed to envers-hibernate3.3 branch?
By the way, HHH-4721 must be closed too, isn't it?
http://opensource.atlassian.com/projects/hibernate/browse/HHH-4721
Many thanks for this fix!
Hernán.