PaperMC
Add "cancel on error" option for event listeners
Adds the option to cancel an event if an error is thrown inside its handler useful for mission critical listeners, for example, preventing players from breaking blocks in an area or preventing players from modifying certain items inside an anvil.
Of course, ideally, your plugin shouldn't be throwing errors in listeners, but mistakes can happen, this feature adds a way for plugin authors to have a fail-safe for important listeners, for example:
Before:
@EventHandler(ignoreCancelled = true)
public void onBlockPlace(BlockPlaceEvent event) {
try {
if (isSpecialItem(event.getItemInHand())) {
event.setCancelled(true);
} else if (isProtectedArea(event.getBlock().getLocation())) {
event.getPlayer().sendMessage(getLocalized("cant.use.here")); // Maybe at some point this will fail
event.setCancelled(true);
}
} catch (Throwable throwable) { // fail safe
event.setCancelled(true);
}
}
After:
@EventHandler(ignoreCancelled = true, cancelOnError = true)
public void onBlockPlace(BlockPlaceEvent event) {
if (isSpecialItem(event.getItemInHand())) {
event.setCancelled(true);
} else if (isProtectedArea(event.getBlock().getLocation())) {
event.getPlayer().sendMessage(getLocalized("cant.use.here")); // Maybe at some point this will fail
event.setCancelled(true);
}
}
Also, I didn't bother adding it for TimedEventListener since those are all behind if (false) statements and timings is about to be removed.
Somewhat meh on the concept, not the biggest fan nor hater. If people are coding such critical stuff, they should be able to just wrap in try-catch. Like, I just don't see how often this would actually be used in the wild? Over just try-catch, which also gives you proper control of any potential exceptions.
Yeah, I think people that want this should just be more explicit about it and just use try-catch. That's always going to be more flexible than a boolean on the annotation. Like what if I want it to cancel on exceptions unless some condition is met. Using code is more flexible. So I'm against this.
If you're going to add an annotation that requires manually adding it, whose only purpose is to effectively be try{/*code*/}catch(Exception ignored){event.setCancelled(true);}, why aren't you just try-catching? The only way this becomes a more-useful feature would be if it were a breaking change to enable it by default. (And I also don't like that idea, to be clear.)
