darrachequesne / spring-data-jpa-datatables

Spring Data JPA extension to work with the great jQuery plugin DataTables (https://datatables.net/)
Apache License 2.0
448 stars 172 forks source link

Log Exceptions or they are silently lost #143

Closed chpasha closed 2 years ago

chpasha commented 2 years ago

Consider following repository

public interface ObjectDataTablesRepository extends DataTablesRepository<Object, Integer>
{
    default DataTablesOutput<BwDbObject> findUnmapped(DataTablesInput input)
    {
        return findAll(input, (root, query, builder) -> {
            root.fetch(Object_.association, JoinType.LEFT); //wrong, should be join
            return builder.isNull(root.get(Object_.objectClass));
        });
    }
}

it adds custom condition and a join fetch on associated entity to the query. The latter causes the "query specified join fetching, but the owner of the fetched association was not present in the select list" exception, which is catched in findAll but then put into output only

 catch (Exception e) {
      output.setError(e.toString());
 }

then execution continues and fails with "Transaction silently rolled back because it has been marked as rollback-only" which is ultimately thrown by spring, so we never get to the output.error and never see any exception in log unless we debug findAll and see the real cause. I'm not sure why the transaction marked for rollback since we catch the exception, but I don't have time anymore to dig further. I think that it wouldn't be a terrible idea to log exceptions to slf4j anyway, not only to error field.

darrachequesne commented 2 years ago

Hi! That's a good idea :+1: , would you have time to open a pull request?

chpasha commented 2 years ago

Yes, perhaps next weekend

chpasha commented 2 years ago

Here you go https://github.com/darrachequesne/spring-data-jpa-datatables/pull/144

darrachequesne commented 2 years ago

This should be fixed by https://github.com/darrachequesne/spring-data-jpa-datatables/commit/d102cfabc3a67b3dd1768e373e21f0855f94a43a, included in release 5.2.0. Thanks for the heads-up :+1: