http://ayende.comAyende @ RahienCopyright (C) Ayende Rahien 2004 - 2021 (c) 202660Ayende, have you tried using the latest version of CommonLogging? You will get a much nicer syntax instead of if(log.IsDebugEnabled) you can simply write log.Warn(logm => logm(" Task {0} has failed and was deleted without completing any work", taskAsJson)); which will be evalutated only if warning is active. As a bonus, commonlogging will abstract the used logging framework and you can switch easily between logging implementations (system.trace (who wants that anyways?), log4net, nlog, simpleconsolelogger...). See Erich Eichinger's blogpost about the new features: [eeichinger.blogspot.com/.../...r-net-released.html](http://eeichinger.blogspot.com/2009/05/common-logging-20-for-net-released.html) http://ayende.com/4425/getting-code-ready-for-production#comment24http://ayende.com/4425/getting-code-ready-for-production#comment24Fri, 12 Mar 2010 08:49:41 GMTMichael, Ah! Good, I was wondering when someone will catch that. http://ayende.com/4425/getting-code-ready-for-production#comment23http://ayende.com/4425/getting-code-ready-for-production#comment23Mon, 08 Mar 2010 06:56:33 GMTWhat Steve said. Seems like you are still doing allocations after the exception is thrown (one of the things you tried to avoid for production code) http://ayende.com/4425/getting-code-ready-for-production#comment22http://ayende.com/4425/getting-code-ready-for-production#comment22Mon, 08 Mar 2010 05:57:11 GMTOne difference I would make here is to have two catches for SystemException and Exception; for the first, catch and re-throw as it caters for all of the OutOfMemoryException etc, the second you could swallow I suppose but it would be nicer to know what exceptions you could receive and handle them explicitly - though it does make the code more fragile. http://ayende.com/4425/getting-code-ready-for-production#comment21http://ayende.com/4425/getting-code-ready-for-production#comment21Sun, 07 Mar 2010 20:47:35 GMTAre you sure you can handle OutOfMemoryExceptions this way? http://ayende.com/4425/getting-code-ready-for-production#comment20http://ayende.com/4425/getting-code-ready-for-production#comment20Sun, 07 Mar 2010 16:53:15 GMTFrans, If I am writing code that I know how to work with, sure, you should do it right the first time around. But this codebase represent me doing design & exploration along the way, taking the burden of production worthiness (along the other related taxes) would have made it too hard. It was more efficient to do it in a two stage approach. http://ayende.com/4425/getting-code-ready-for-production#comment19http://ayende.com/4425/getting-code-ready-for-production#comment19Sun, 07 Mar 2010 10:39:40 GMTIsn't this 'getting ready for production' really a waste of time? I mean: what if you try to write the code the first time already as if it's going to be for production. that would save you time, and you also don't run the risk of not having time to go over the code to make it ready for production. The bigger code base become, the more time it takes to write them, the more time it will take (and thus the lower the chance you'll do it) to make it production ready. http://ayende.com/4425/getting-code-ready-for-production#comment18http://ayende.com/4425/getting-code-ready-for-production#comment18Sun, 07 Mar 2010 10:35:17 GMTZiv, Error tracking is handled else where for this scenario, and too many errors would disable a certain set of tasks http://ayende.com/4425/getting-code-ready-for-production#comment17http://ayende.com/4425/getting-code-ready-for-production#comment17Sun, 07 Mar 2010 09:29:14 GMTRory, That isn't the reason for that. The reason is that I want to avoid the perf hit of doing string formating if loging isn't enabled. http://ayende.com/4425/getting-code-ready-for-production#comment16http://ayende.com/4425/getting-code-ready-for-production#comment16Sun, 07 Mar 2010 09:27:58 GMT@Rory the check is done to avoid formatting a string if logging is disabled. If the exception is an abnormal situation, optimizing its handler seems unnecessary. And the logging API could handle that, for example NLog av oids formatting a string if the message won't get logged. http://ayende.com/4425/getting-code-ready-for-production#comment15http://ayende.com/4425/getting-code-ready-for-production#comment15Sun, 07 Mar 2010 07:48:15 GMTAre all kinds of errors expected? Does this include ImCorruptPleaseDontCallMeAgainOrIllFormatYourHardDrive? What about whoever submitted the job? If it doesn't care whether the job succeeds or not, you may as well ignore every second job -- performance would be way up. (Of course, this is all tongue-in-cheek.) http://ayende.com/4425/getting-code-ready-for-production#comment14http://ayende.com/4425/getting-code-ready-for-production#comment14Sun, 07 Mar 2010 07:18:49 GMTAh, didn't see that. It overflowed into the sidebar. http://ayende.com/4425/getting-code-ready-for-production#comment13http://ayende.com/4425/getting-code-ready-for-production#comment13Sun, 07 Mar 2010 04:29:42 GMTMy 20c worth, the bit that screams ugly to me is the logging check around warning being enabled. The logging implementation should handle the outcome of whether warning level logging is enabled or not. To have this implemented by calling code seems like incorrect allocation of responsibility and duplicated logic. Calling code should not be responsible for understanding the how/what/why of the logging implementation as it does here. If this is a restriction of the logging implementation when I would use a different logger. http://ayende.com/4425/getting-code-ready-for-production#comment12http://ayende.com/4425/getting-code-ready-for-production#comment12Sun, 07 Mar 2010 02:25:01 GMTSteve, That is why I include the exception in. http://ayende.com/4425/getting-code-ready-for-production#comment11http://ayende.com/4425/getting-code-ready-for-production#comment11Sat, 06 Mar 2010 23:37:01 GMTLooks pretty standard. I'm asuming that: log.DebugFormat("Executing {0}", taskAsJson); is conditional for Debug builds? It might be worth while on your inner catch for the Execute to include details in the Warn about what exception was raised. (Why the task couldn't be executed.) http://ayende.com/4425/getting-code-ready-for-production#comment10http://ayende.com/4425/getting-code-ready-for-production#comment10Sat, 06 Mar 2010 22:49:25 GMT> How is that you are so ignorant of .NET yet still choose to criticize? I don't need to know the .net apis to see that these logging calls are ducking ugly. > there is no built-in logging API in .NET. Well that answers my first question with a negative (though I'm not sure it's a good thing) http://ayende.com/4425/getting-code-ready-for-production#comment9http://ayende.com/4425/getting-code-ready-for-production#comment9Sat, 06 Mar 2010 22:38:52 GMTZiv, There isn't any calling code, this is handled by a indexing background thread. Rethrowing will kill the entire application, and having errors on indexing is expected http://ayende.com/4425/getting-code-ready-for-production#comment8http://ayende.com/4425/getting-code-ready-for-production#comment8Sat, 06 Mar 2010 21:57:20 GMT"There is nothing meaningful that can be done": I disagree. Either rethrow, and let the calling code decide what it wants to do with the exception, or do Environment.FailFast. Anything else (in particular if you're writing libraries, not the "top-level" apps) masks bugs and is usually very difficult to debug. http://ayende.com/4425/getting-code-ready-for-production#comment7http://ayende.com/4425/getting-code-ready-for-production#comment7Sat, 06 Mar 2010 21:50:02 GMT@masklinn How is that you are so ignorant of .NET yet still choose to criticize? Since you really don't know what you're talking about here's a little FYI: there is no built-in logging API in .NET. http://ayende.com/4425/getting-code-ready-for-production#comment6http://ayende.com/4425/getting-code-ready-for-production#comment6Sat, 06 Mar 2010 21:16:47 GMTGoodness gracious, is this .Net's built-in logging API? It looks downright dreadful, why in science's name do you have different methods depending on whether you want to do formatting by hand or not, and why do you end up using 3 different kinds of formatting? http://ayende.com/4425/getting-code-ready-for-production#comment5http://ayende.com/4425/getting-code-ready-for-production#comment5Sat, 06 Mar 2010 20:32:22 GMTYep, There is nothing meaningful that can be done. http://ayende.com/4425/getting-code-ready-for-production#comment4http://ayende.com/4425/getting-code-ready-for-production#comment4Sat, 06 Mar 2010 18:38:34 GMTcatch (Exception) w/o rethrow in production code? http://ayende.com/4425/getting-code-ready-for-production#comment3http://ayende.com/4425/getting-code-ready-for-production#comment3Sat, 06 Mar 2010 18:30:55 GMTAndrey, The assumption is that WARN is enabled for production. The server keeps track of the last 50 errors and can show them on request. http://ayende.com/4425/getting-code-ready-for-production#comment2http://ayende.com/4425/getting-code-ready-for-production#comment2Sat, 06 Mar 2010 11:09:51 GMTSo if the warnings are not enabled there is no way to tell that task has thrown an exception? Is it generally ok for task to throw an exception? http://ayende.com/4425/getting-code-ready-for-production#comment1http://ayende.com/4425/getting-code-ready-for-production#comment1Sat, 06 Mar 2010 10:51:02 GMT