justinmclean opened a new issue, #10177:
URL: https://github.com/apache/gravitino/issues/10177

   ### What would you like to be improved?
   
   SessionUtils.doWithCommit(...) catches only Exception. If the mapper 
callback throws a non-Exception Throwable (for example AssertionError), the 
catch block is skipped, so SqlSessions.rollbackAndCloseSqlSession() is not 
called. This can leave transaction/session state unclosed.
   
   ### How should we improve?
   
   Broaden failure handling so cleanup runs for all throwables from the 
callback path. In practice, catch Throwable, call 
SqlSessions.rollbackAndCloseSqlSession(), and rethrow. Apply the same pattern 
consistently.
   
   Here's a unit test to help:
   ```
   public class TestSessionUtils {
   
     @Test
     public void testDoWithCommitShouldRollbackOnAssertionError() {
       Object mapper = new Object();
   
       try (MockedStatic<SqlSessions> mockedSqlSessions = 
mockStatic(SqlSessions.class)) {
         mockedSqlSessions.when(() -> 
SqlSessions.getMapper(Object.class)).thenReturn(mapper);
   
         assertThrows(
             AssertionError.class,
             () ->
                 SessionUtils.doWithCommit(
                     Object.class,
                     ignored -> {
                       throw new AssertionError("boom");
                     }));
   
         mockedSqlSessions.verify(SqlSessions::rollbackAndCloseSqlSession);
         mockedSqlSessions.verify(() -> SqlSessions.commitAndCloseSqlSession(), 
never());
       }
     }
   }
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to