Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Log Exceptions or they are silently lost #143

Closed
chpasha opened this issue May 9, 2022 · 4 comments
Closed

Log Exceptions or they are silently lost #143

chpasha opened this issue May 9, 2022 · 4 comments
Milestone

Comments

@chpasha
Copy link

chpasha commented May 9, 2022

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
Copy link
Owner

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

@chpasha
Copy link
Author

chpasha commented May 11, 2022

Yes, perhaps next weekend

@chpasha
Copy link
Author

chpasha commented May 14, 2022

Here you go
#144

darrachequesne pushed a commit that referenced this issue May 19, 2022
@darrachequesne
Copy link
Owner

This should be fixed by d102cfa, included in release 5.2.0. Thanks for the heads-up 👍

@darrachequesne darrachequesne added this to the 5.2.0 milestone May 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants