google / google/guice

Google guice persist bug report for error handling for txn rollback when used with hibernate 4.3

Open
#1,013 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
12.7k
Forks
1.7k
Avg merge
11m
Merged PRs (30d)
2

Description

[JpaLocalTxnInterceptor.java](https://github.com/google/guice/blob/master/extensions/persist/src/com/google/inject/persist/jpa/JpaLocalTxnInterceptor.java#L69-L79)

```
try {
result = methodInvocation.proceed();

} catch (Exception e) {
//commit transaction only if rollback didnt occur
if (rollbackIfNecessary(transactional, e, txn)) {
txn.commit();
}

//propagate whatever exception is thrown anyway
throw e;
```

It call the `method` and if that method throws a error, in try block it `rollbackIfNecessary` and then re-throw the error.

Now if we look at the code of [TransactionImpl v4.3](https://github.com/hibernate/hibernate-orm/blob/4.3/hibernate-entitymanager/src/main/java/org/hibernate/jpa/internal/TransactionImpl.java#L72-L75)

```
public void commit() {
if ( tx == null || !tx.isActive() ) {
throw new IllegalStateException( "Transaction not active" );
}
if ( rollbackOnly ) {
tx.rollback();
throw new RollbackException( "Transaction marked as rollbackOnly" );
}
```

the `commit` method will throw a `RunTimeException` if transaction is marked for rollback and if that happens **google persist will not rethrow the actual exception instead it will throw the runtimeException**.

_# So should `txn.commit();` in `if (rollbackIfNecessary(transactional, e, txn)) {` be put inside the `try catch`block ?_

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.