mongodb/mongo-azure — pull request #8 by optimiz3 (2012-11-18 09:20:35+00:00)
Title: Fix AZURE-78 and AZURE-79 + small items found.

# High-level changes
* Fixed AZURE-79 Tracing: Trace/Diagnostic helpers shouldn't call String.Format or ToString()
* Parameters now get passed through to TraceListeners
* Implemented simple method enter/exit trace helper to standardize tracing
* Implemented simple exception trace helper methods
* Fixed AZURE-78 Security: Eliminate catch(Exception) without rethrow
* This is a relatively evil class of bugs that lead to undefined state when potentially serious errors get swallowed/masked (NullReferenceException; InvalidOperationException; ArgumentException; SEHException; memory corruption due to bit rot, etc.)
* Refactored config so that it all lives under Settings rather than being split between MongoDBRole, Utilities, and Settings
* Fixed a relatively evil class of bugs where invalid configuration settings would silently revert back to default configuration settings without any indication that the behavior was altered/undefined.
* Improved the RoleEntryPoint process model to not spend cycles spin-polling for status and to immediately terminate when MongoD exits or OnStop is called depending on the value of RecycleOnExit (as opposed to waiting for the next poll interval).
* Fixed a NullReferenceException crash in the Manager web application.

## Individual Changes:
### Tools/*BlobBackup
* *BlobBackup.csproj
* Removed unused references (normally added by default to projects)
* Program.cs
* Replaced catch (Exception) w/ Uri.TryCreate

### Src/*Backup/BackupManager
* BackupManager.cs
* If a StorageClientException that occurred during ListBlobs were incorrectly being swallowed causing success and an empty set to be returned instead of an error.
* *Backup.csproj
* Removed unused references (normally added by default to projects)

### Src/*Common
* ConnectionUtilities.cs
* Replaced catch (Exception) w/ Roles.TryGetValue
* Replaced ReplicaSetEnvironmentException (custom config exception) w/ standard ConfigurationErrorsException (eliminated a single use class-local class that wasn’t caught by any other code and wasn’t provide extra context)
* *Common.csproj
* Removed unused references (normally added by default to projects)

### Src/*InstanceMaintainer
* *InstanceMaintainer.csproj
* Removed unused references (normally added by default to projects)
* WinHostsUpdater.cs
* Simplified log expression (message, stack, and inner exception are logged by default by exception.ToString()); standard formatting is also easier to parse, and trace listeners get access to the actual exception for mining and serialization purposes.

### Src/*Manager
* Controllers/SnapshotController.cs
* Clicking on the backup link when no primary was available would result in a crash handler page (NullReferenceException) because ServerStatus.Primary would be null. As mitigation, it will now show an error instead. Down the road, a potentially better UX might be to allow backups from fresh secondaries, or disable the link if no available nodes exist with some sort of visual queue.
* Models/ReplicaSetStatus.cs
* Replaced catch(Exception) w/ correct exception handlers
* *Manager.csproj
* Removed unused references (normally added by default to projects)

### Src/*MongoDBRole
* DatabaseHelper.cs
* IsReplicaSetInitialized: Removed a blank catch all; did some testing and the only exceptions that could legally be handled here could be MongoConnectionException and MongoCommandException. However, this should never happen as the only code that calls IsReplicaSetInitialized calls EnsureMongoDIsListening before, which prevents both cases. As such, any exceptions here actually are unhandled and should cause a crash because they are undefined.
* EnsureMongodIsListening: Replaced catch(Exception) w/ correct exception handlers; as well as a note that retry back-off logic is needed at some point (retry and backoff are yin and yang, one without the other = DOS waiting to happen).
* DiagnosticsHelper.cs
* Refactored this class to address AZURE-79;
* Trace parameters now get passed through to the trace listener instead of being converted to strings.
* Replaced Trace with a TraceSource that includes the RoleInstance name rather to eliminate the need to prepend it to every message (forcing premature string conversion).
* Helpers have been added to standardize exception logging.
* Added the TraceMethodDisposable class to enable automatic function wide using scopes for logging Begin/End sequences; note this also covers abnormal function exists such as exceptions.
* Added the TraceMethod helper which automatically resolves the caller function name.
* *MongoDBRole.csproj
* Removed unused references (normally added by default to projects)
* MongoDBRole.cs
* Run: Eliminated the timer spin wait; the thread now never needs to wake up and immediately returns if MongoD terminates. If RecycleOnExit is specified, then Run will wait for OnStop to be called. This eliminates unnecessary cycles due to the thread waking up, and enables shutdown to complete faster (instead of potentially delaying 15 seconds).
* OnStart/OnStop: Calling base.OnStart/OnStop is not advised by MSFT as these functions do nothing other than immediately return.
* RoleEnvironmentChanged: Simplified code by moving parsing/value extraction to Settings.cs
* Settings.cs
* Maintainability: In several places different nomenclature was being used; standardized variable names to match their actual configuration value and to represent that they represented either the setting name or the setting value to make the code more self-documenting; ex. RecycleOnExit vs Recycle; LocalCacheDir vs LocalDataDir.
* Split the static constructor up into individual configuration setting assessors methods; this eliminated duplicate similar code in MongoDBRole.cs:RoleEnvironmentChange by enabling RoleEnvironmentChange to call these individual setting assessors directly. This change was motivated after examining how the multiple catch(Exception) blocks could be factored out.
* Fixed a relatively serious bug around incorrect configuration settings; in the case where an admin misconfigures a setting into something that cannot be parsed, the node will revert to default settings rather than throwing an error. These types of silent failures would leave the node incorrectly configured, leading to all sorts of unexpected scenarios that can go undetected. Now if there is an unparsable setting, a standard ConfigurationErrorsException will be thrown for the invalid setting and value.
* Utilities.cs
* Because configuration logic now lives in one place, several methods were eliminated
* Switched to the standard exception formatters