Try-catch for delegate is unmaintainable
May. 3rd, 2019 02:36 pm![[personal profile]](https://www.dreamwidth.org/img/silk/identity/user.png)
Couple or years ago we tried to reuse exception handling code and created this method:
The idea was that if we know that a method may crash then we would just pass that method as a "tryAction" parameter.
Then ExecuteEmailCrashSuppressWebException() will swallow web exceptions, and will notify us (developers) about all other exceptions.
We wanted to avoid repeating try-catch boilerplate code by reusing ExecuteEmailCrashSuppressWebException().
That attempt [to reuse try-catch] failed miserably. Every tryAction method needed its own custom exception handling:
- In some cases we needed to swallow WebException, but in other cases we wanted to log it.
- In some cases we wanted to write to one "{tryAction}.log" file, but in other cases we wanted to write to differently named log file, or not to write to log file at all.
- Log message content was quite different for different tryAction methods.
Eventually we deleted ExecuteEmailCrashSuppressWebException() and wrapped every individual method that needed custom exception handling - by its own try-catch block of code.
My conclusion is:
Try-catch block should wrap only direct method calls.
Almost never try-catch should wrap Action/delegate invocation (such as "tryAction()").
The reason why wrapping delegate invocation with try-catch does not work is that "catch" implementation is very custom for every individual method.
Merging all these custom implementation into a single "catch" block produces unmaintainable mess.
public static void ExecuteEmailCrashSuppressWebException(Action tryAction, ActionreportAction) { var tl = new TimeLog(); try { tryAction(); } catch (WebException) { return; } catch (Exception ex) { string displayName = tryAction.GetDisplayName(); string message = CookMessage(displayName, reportAction, tl, ex); string subject = $"Win crash: {displayName}()"; Log.ToFile(message, displayName + ".log"); EmailToDevelopers.EmailTextIfSubjectWasNotSentRecently(subject, message); } }
The idea was that if we know that a method may crash then we would just pass that method as a "tryAction" parameter.
Then ExecuteEmailCrashSuppressWebException() will swallow web exceptions, and will notify us (developers) about all other exceptions.
We wanted to avoid repeating try-catch boilerplate code by reusing ExecuteEmailCrashSuppressWebException().
That attempt [to reuse try-catch] failed miserably. Every tryAction method needed its own custom exception handling:
- In some cases we needed to swallow WebException, but in other cases we wanted to log it.
- In some cases we wanted to write to one "{tryAction}.log" file, but in other cases we wanted to write to differently named log file, or not to write to log file at all.
- Log message content was quite different for different tryAction methods.
Eventually we deleted ExecuteEmailCrashSuppressWebException() and wrapped every individual method that needed custom exception handling - by its own try-catch block of code.
My conclusion is:
Try-catch block should wrap only direct method calls.
Almost never try-catch should wrap Action/delegate invocation (such as "tryAction()").
The reason why wrapping delegate invocation with try-catch does not work is that "catch" implementation is very custom for every individual method.
Merging all these custom implementation into a single "catch" block produces unmaintainable mess.
no subject
Date: 2019-05-03 07:16 pm (UTC)no subject
Date: 2019-05-03 07:26 pm (UTC)