Potentially swallowed exceptions due to use of CompletableFuture

[A248]

Someone was asking me about Anvil and I saw in the code that exceptions in CompletableFutures are sometimes swallowed. Sadly CompletableFuture's design makes it very easy to accidentally swallow exceptions, so it is an honest mistake that many have made, including myself.

However, the extent of this problem in the AnvilPowered codebase is generally high. I post this issue here on behalf of not only Anvil but also Catalyst, for the same problem exists in both repositories. I've only found one case of it in Anvil, but many in Catalyst, though my search was limited. If it is all pleasing to you for organizational purposes, do feel free to open an issue on the Catalyst repository, and close this one.

Here is an example of what I mean:

https://github.com/AnvilPowered/Anvil/blob/3a6b899ae980d29816d69b3fe4d214e5a27910ab/anvil-common/src/main/java/org/anvilpowered/anvil/common/messaging/CommonRedisService.java#L80-L81

The problem here is that we have a CompletableFuture which is not awaited, not returned, and has no dependent actions. Therefore, nothing will uncover an exception arising inside the async execution.

As the following sample code shows, the exception will be swallowed and the code after the exception will not run:

		java.util.concurrent.CompletableFuture.runAsync(() -> {
			System.out.println("Pre throw");
			throw new RuntimeException();
		}).thenRun(() -> {
			System.out.println("Post throw"); // Will never run
		});

I do urge that you heed the possibility of hidden exceptions in CompletableFuture-utilizing code to the same extent that you recognize the issue of swallowed exceptions in imperative code. It will hamper maintainability by making debugging difficult.

Fixing this problem will take some time; it will require the addition of exceptionally blocks everywhere, as has been done in other cases. Here is an example: https://github.com/PaperMC/Velocity/pull/376

Addendum

Additionally, I need to apologize for a comment made by another user to one of your previous issues, "bruh". The comment was made to my displeasure, but on account of my own incitation, regarding a discussion in which I did not correctly convey my opinion regarding a matter. The comment is entirely unwarranted, and in no way did I suggest to the user making the comment, that they should do so. Nevertheless I take some measure of responsibility for it (thought not full responsibility, since it was not my comment) having myself discussed the content of that same issue with a tone implying some note of antagonism. I hope you can overlook this lack of foresight on my part that I did not imagine another user would exhibit such lack of respect to the Anvil developers with a comment like "bruh".

[alexstaeding]

I agree with your point that these callback-style futures are extremely prone to exception swallowing.

This is one of the reasons I have been planning for a while to switch entirely to Kotlin and coroutines but this is quite a large change that needs proper attention and time investment. As soon as time permits, it will be fixed.

Yes, this codebase is in dire need of maintenance but unfortunately, other commitments have taken precedence.

[alexstaeding]

I imagine this is fixed now by #64. There shouldn't be any java futures anymore - and all coroutines should be joined to some main thread now.