dennisgorelik: 2020-06-13 in my home office (Default)
Dennis Gorelik ([personal profile] dennisgorelik) wrote2019-05-03 02:36 pm

Try-catch for delegate is unmaintainable

Couple or years ago we tried to reuse exception handling code and created this method:
public static void ExecuteEmailCrashSuppressWebException(Action tryAction, Action reportAction)
{
	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.
juan_gandhi: (Default)

[personal profile] juan_gandhi 2019-05-03 06:47 pm (UTC)(link)
So, a good step in the direction of discovering a monadic nature of computing. Good.

And sure, never ignore exceptions.
juan_gandhi: (Default)

[personal profile] juan_gandhi 2019-05-04 12:35 am (UTC)(link)
500 is 500. Make conclusion out of this fact. But take some action in any case.
juan_gandhi: (Default)

[personal profile] juan_gandhi 2019-05-04 01:31 am (UTC)(link)
I'd stop pinging every second, and increase the delay. Also, if the thing is not available e.g. for a couple of hours, notify someone.
juan_gandhi: (Default)

[personal profile] juan_gandhi 2019-05-04 02:10 am (UTC)(link)
Oh, I see.
xoxlobandera: (Default)

[personal profile] xoxlobandera 2019-05-03 07:16 pm (UTC)(link)
Если так хотелось сделать "универсальную" обертку, то кто мешал передавать ей параметром-делегатом требуемую функциональность для блока catch?