Reading SQL query from a resource, with error logging I've created a utility class that expects a logger to be passed at a method level, since I prefer legitimate errors to be written out for troubleshooting later on.  Another developer hates passing the logger to the method, because he does not care.  


Cannot use optional attribute, since ILogger is not a constant.
Cannot use params because it would be a singleton, not a collection.


So I implemented it in the following manner:

public static class EmbeddedResourceUtility
{
     public static string GetSqlQuery(string namespaceAndFileNameWithExtension, ILogger logger)
     {
          try
          {
               using(var stream = typeof(EmbeddedResourceUtility).GetTypeInfo().Assembly.GetManifestResourceStream(namespaceAndFileNameWithExtension))
                   using(var reader = new StreamReader(stream, Encoding.UTF8))
                        return reader.ReadToEnd();
          }

          catch(Exception exception)
          {
               logger.Error($"Failed to read query file {namespaceAndFileNameWithExtension}. {Environment.NewLine}{exception.HResult}: {exception.Message}");
               throw new Exception(exception.Message);
          }
     }
}


Would be incorrect to expect someone to pass an ILogger even if they do not intend to use it?  Or would there be a better way to handle this?
The most unforgivable aspect of this code is that it rethrows a degraded exception.  The original type of the exception (likely an IOException) and the stack trace information are lost when you throw new Exception(…).

Two acceptable approaches are:


Log the failure (if a non-null logger was passed).  Then re-throw the original exception.

 public static string GetSqlQuery(string namespaceAndFileNameWithExtension, ILogger logger)
 {
      try
      {
           using(var stream = typeof(EmbeddedResourceUtility).GetTypeInfo().Assembly.GetManifestResourceStream(namespaceAndFileNameWithExtension))
           using(var reader = new StreamReader(stream, Encoding.UTF8))
           {
                return reader.ReadToEnd();
           }
      }
      catch (Exception exception)
      {
           logger?.Error($"Failed to read query file {namespaceAndFileNameWithExtension}.{Environment.NewLine}{exception.HResult}: {exception.Message}");
           throw;
      }
 }

Throw a custom exception, wrapping the original exception.

 public static string GetSqlQuery(string namespaceAndFileNameWithExtension)
 {
      try
      {
           using(var stream = typeof(EmbeddedResourceUtility).GetTypeInfo().Assembly.GetManifestResourceStream(namespaceAndFileNameWithExtension))
           using(var reader = new StreamReader(stream, Encoding.UTF8))
           {
               return reader.ReadToEnd();
           }
      }
      catch (Exception exception)
      {
           throw new EmbeddedResourceException($"Failed to read query file {namespaceAndFileNameWithExtension}", exception);
      }
 }


In this case, requiring an ILogger just isn't worth it.  The caller has all of the information in the EmbeddedResourceException, and can log it if it wants.


I recommend the second approach, because it respects the principle of separation of concerns.