Discussion: should all callbacks be wrapped so that exceptions are caught

[stefvanschie]

Discussion: should all callbacks be wrapped so that exceptions are caught

[stefvanschie]

Let's keep this open for a little longer - if no one uses the current methods, they can go; if people are using them (and want to continue using those instead of the new callX versions) we can just keep them.

[Trigary]

One counter-argument is that if plugins decide to throw exceptions in these callbacks, they should expect stuff to break. My response is that as long as the intermediate state these interrupts leave the IF library in are non-permanent, non-fatal (eg. they don't matter the next the click event is called), it's fine. But if that's not the case or if code ever gets added that might get badly hit by these uncaught exceptions problems arise. Catching them is a future-proof, guaranteed bug-free solution with little overhead (both code and runtime performance-wise) and you wouldn't have to worry/think about exceptions from callbacks at all in that case.

[stefvanschie]

I'm assuming you then want the listed methods to be transformed into some kind of callX method that automatically takes care of the exception handling? I'm fine with the changes to keep IF in a 'safe' state even if the user makes a Consumer throw an Exception, but I'm not certain if it's necessary to replace the current methods with these new ones or just keep both of them and internally always use the callX version. After all, if the user calls their own exception throwing Consumer retrieved from the getX methods as you listed, that is, at that point, their problem. It depends on if these current methods are actually used by someone I suppose; if no one uses them, no point in keeping them.

[Trigary]

I'm assuming you then want the listed methods to be transformed into some kind of callX method that automatically takes care of the exception handling?

Yes.

I'm not certain if it's necessary to replace the current methods with these new ones or just keep both of them and internally always use the callX version.

By replacing those getters, safety in the sense of exception handling would be enforced, there would be no way to make mistakes. Granted, this change helps the developers working on or looking at IF's code more than those who are just using it.

After all, if the user calls their own exception throwing Consumer retrieved from the getX methods as you listed, that is, at that point, their problem.

Good counterargument, but as you also said:

It depends on if these current methods are actually used by someone I suppose; if no one uses them, no point in keeping them.

Exactly. I personally don't think it would affect anyone at all, but that's just a wild guess.

[stefvanschie]

I've gone through the dependents of this framework and from what I can find, none of them seem to be making use of the getX methods, so replacing them with their equivalent callX method should be fine.