Skip to main content
added 234 characters in body
Source Link
Nkosi
  • 3.3k
  • 17
  • 25

Given the variable name, the assumption is that the timestamp was meant to be UTC, which would mean that it should be using DateTime.UtcNow

The SQL error logging (though commented out) can be encapsulated into its own function/concern

public class UnhandledExceptionLogger : ExceptionLogger {
    private static readonly Logger logger = LogManager.GetCurrentClassLogger();

    public override void Log(ExceptionLoggerContext context) {
        DateTime timestamp = DateTime.Now;UtcNow;
        string strLogText = FormatException(context.Exception);            
        //SQL
        SqlLog(strLogText, context.Request, timestamp);
        //NLOG
        NLog(logger, strLogText, timestamp);
    }

    private void NLog(Logger logger, string message, DateTime timestamp) {
        var sb = new StringBuilder();
        sb.AppendLine(message);
        sb.AppendLine(timestamp);
        logger.Error(sb.ToString());
    }

    private void SqlLog(string message, HttpRequestMessage request, DateTime timestamp) {
        var requestedURi = (string)request.RequestUri.AbsoluteUri;
        var requestMethod = request.Method.ToString();
        SqlErrorLogging sqlErrorLogging = new SqlErrorLogging();
        ApiError apiError = new ApiError() {
            Message = message,
            RequestUri = requestedURi,
            RequestMethod = requestMethod,
            TimeUtc = timestamp
        };
        //sqlErrorLogging.InsertErrorLog(apiError);
    }

    private string FormatException(Exception ex, int depth = 0) {
        //... omitted for brevity
    }
}

The SQL error logging (though commented out) can be encapsulated into its own function/concern

public class UnhandledExceptionLogger : ExceptionLogger {
    private static readonly Logger logger = LogManager.GetCurrentClassLogger();

    public override void Log(ExceptionLoggerContext context) {
        DateTime timestamp = DateTime.Now;
        string strLogText = FormatException(context.Exception);            
        //SQL
        SqlLog(strLogText, context.Request, timestamp);
        //NLOG
        NLog(logger, strLogText, timestamp);
    }

    private void NLog(Logger logger, string message, DateTime timestamp) {
        var sb = new StringBuilder();
        sb.AppendLine(message);
        sb.AppendLine(timestamp);
        logger.Error(sb.ToString());
    }

    private void SqlLog(string message, HttpRequestMessage request, DateTime timestamp) {
        var requestedURi = (string)request.RequestUri.AbsoluteUri;
        var requestMethod = request.Method.ToString();
        SqlErrorLogging sqlErrorLogging = new SqlErrorLogging();
        ApiError apiError = new ApiError() {
            Message = message,
            RequestUri = requestedURi,
            RequestMethod = requestMethod,
            TimeUtc = timestamp
        };
        //sqlErrorLogging.InsertErrorLog(apiError);
    }

    private string FormatException(Exception ex, int depth = 0) {
        //... omitted for brevity
    }
}

Given the variable name, the assumption is that the timestamp was meant to be UTC, which would mean that it should be using DateTime.UtcNow

The SQL error logging (though commented out) can be encapsulated into its own function/concern

public class UnhandledExceptionLogger : ExceptionLogger {
    private static readonly Logger logger = LogManager.GetCurrentClassLogger();

    public override void Log(ExceptionLoggerContext context) {
        DateTime timestamp = DateTime.UtcNow;
        string strLogText = FormatException(context.Exception);            
        //SQL
        SqlLog(strLogText, context.Request, timestamp);
        //NLOG
        NLog(logger, strLogText, timestamp);
    }

    private void NLog(Logger logger, string message, DateTime timestamp) {
        var sb = new StringBuilder();
        sb.AppendLine(message);
        sb.AppendLine(timestamp);
        logger.Error(sb.ToString());
    }

    private void SqlLog(string message, HttpRequestMessage request, DateTime timestamp) {
        var requestedURi = (string)request.RequestUri.AbsoluteUri;
        var requestMethod = request.Method.ToString();
        SqlErrorLogging sqlErrorLogging = new SqlErrorLogging();
        ApiError apiError = new ApiError() {
            Message = message,
            RequestUri = requestedURi,
            RequestMethod = requestMethod,
            TimeUtc = timestamp
        };
        //sqlErrorLogging.InsertErrorLog(apiError);
    }

    private string FormatException(Exception ex, int depth = 0) {
        //... omitted for brevity
    }
}
added 2 characters in body
Source Link
Nkosi
  • 3.3k
  • 17
  • 25

The variable ex is not used anywhere else, so it can be in-lined.

string strLogText = FormatException(context.Exception);

The multiple calls to DateTime.Now will give different time stamps.

YouIt was already assign itassigned here

This can be abstracted out into its own service/concern if so desired

The variable ex is not used anywhere else, so it can be in-lined.

string strLogText = FormatException(context.Exception);
public class UnhandledExceptionLogger : ExceptionLogger {
    private static readonly Logger logger = LogManager.GetCurrentClassLogger();

    public override void Log(ExceptionLoggerContext context) {
        DateTime timeUtctimestamp = DateTime.Now;
        string strLogText = FormatException(context.Exception);            
        //SQL
        SqlLog(strLogText, context.Request, timestamp);
        //NLOG
        NLog(logger, strLogText, timestamp);
    }

    private void NLog(Logger logger, string message, DateTime timestamp) {
        var sb = new StringBuilder();
        sb.AppendLine(message);
        sb.AppendLine(timestamp);
        logger.Error(sb.ToString());
    }

    private void SqlLog(string message, HttpRequestMessage request, DateTime timestamp) {
        var requestedURi = (string)request.RequestUri.AbsoluteUri;
        var requestMethod = request.Method.ToString();
        SqlErrorLogging sqlErrorLogging = new SqlErrorLogging();
        ApiError apiError = new ApiError() {
            Message = message,
            RequestUri = requestedURi,
            RequestMethod = requestMethod,
            TimeUtc = timestamp
        };
        //sqlErrorLogging.InsertErrorLog(apiError);
    }

    private string FormatException(Exception ex, int depth = 0) {
        //... omitted for brevity
    }
}

The multiple calls to DateTime.Now will give different time stamps.

You already assign it here

This can be abstracted out into its own service/concern if so desired

The variable ex is not used anywhere else, so it can be in-lined.

string strLogText = FormatException(context.Exception);
public class UnhandledExceptionLogger : ExceptionLogger {
    private static readonly Logger logger = LogManager.GetCurrentClassLogger();

    public override void Log(ExceptionLoggerContext context) {
        DateTime timeUtc = DateTime.Now;
        string strLogText = FormatException(context.Exception);            
        //SQL
        SqlLog(strLogText, context.Request, timestamp);
        //NLOG
        NLog(logger, strLogText, timestamp);
    }

    private void NLog(Logger logger, string message, DateTime timestamp) {
        var sb = new StringBuilder();
        sb.AppendLine(message);
        sb.AppendLine(timestamp);
        logger.Error(sb.ToString());
    }

    private void SqlLog(string message, HttpRequestMessage request, DateTime timestamp) {
        var requestedURi = (string)request.RequestUri.AbsoluteUri;
        var requestMethod = request.Method.ToString();
        SqlErrorLogging sqlErrorLogging = new SqlErrorLogging();
        ApiError apiError = new ApiError() {
            Message = message,
            RequestUri = requestedURi,
            RequestMethod = requestMethod,
            TimeUtc = timestamp
        };
        //sqlErrorLogging.InsertErrorLog(apiError);
    }

    private string FormatException(Exception ex, int depth = 0) {
        //... omitted for brevity
    }
}

The variable ex is not used anywhere else, so it can be in-lined.

string strLogText = FormatException(context.Exception);

The multiple calls to DateTime.Now will give different time stamps.

It was already assigned here

This can be abstracted out into its own service/concern if so desired

public class UnhandledExceptionLogger : ExceptionLogger {
    private static readonly Logger logger = LogManager.GetCurrentClassLogger();

    public override void Log(ExceptionLoggerContext context) {
        DateTime timestamp = DateTime.Now;
        string strLogText = FormatException(context.Exception);            
        //SQL
        SqlLog(strLogText, context.Request, timestamp);
        //NLOG
        NLog(logger, strLogText, timestamp);
    }

    private void NLog(Logger logger, string message, DateTime timestamp) {
        var sb = new StringBuilder();
        sb.AppendLine(message);
        sb.AppendLine(timestamp);
        logger.Error(sb.ToString());
    }

    private void SqlLog(string message, HttpRequestMessage request, DateTime timestamp) {
        var requestedURi = (string)request.RequestUri.AbsoluteUri;
        var requestMethod = request.Method.ToString();
        SqlErrorLogging sqlErrorLogging = new SqlErrorLogging();
        ApiError apiError = new ApiError() {
            Message = message,
            RequestUri = requestedURi,
            RequestMethod = requestMethod,
            TimeUtc = timestamp
        };
        //sqlErrorLogging.InsertErrorLog(apiError);
    }

    private string FormatException(Exception ex, int depth = 0) {
        //... omitted for brevity
    }
}
Source Link
Nkosi
  • 3.3k
  • 17
  • 25

The multiple calls to DateTime.Now will give different time stamps.

Hold on to the timestamp in one variable early in the function and reuse that.

You already assign it here

 var timeUtc = DateTime.Now;

so use that

//...

ApiError apiError = new ApiError()
{
    Message = strLogText,
    RequestUri = requestedURi,
    RequestMethod = requestMethod,
    TimeUtc = timeUtc
};

//...

The SQL error logging (though commented out) can be encapsulated into its own function/concern

private void LogSQL(string message, HttpRequestMessage request, DateTime timestamp) {
    var requestedURi = (string)request.RequestUri.AbsoluteUri;
    var requestMethod = request.Method.ToString();
    SqlErrorLogging sqlErrorLogging = new SqlErrorLogging();
    ApiError apiError = new ApiError() {
        Message = message,
        RequestUri = requestedURi,
        RequestMethod = requestMethod,
        TimeUtc = timestamp
    };
    //sqlErrorLogging.InsertErrorLog(apiError);
}

This can be abstracted out into its own service/concern if so desired

The variable ex is not used anywhere else, so it can be in-lined.

string strLogText = FormatException(context.Exception);

The above and a few minor changes results in a refactoring of

public class UnhandledExceptionLogger : ExceptionLogger {
    private static readonly Logger logger = LogManager.GetCurrentClassLogger();

    public override void Log(ExceptionLoggerContext context) {
        DateTime timeUtc = DateTime.Now;
        string strLogText = FormatException(context.Exception);            
        //SQL
        SqlLog(strLogText, context.Request, timestamp);
        //NLOG
        NLog(logger, strLogText, timestamp);
    }

    private void NLog(Logger logger, string message, DateTime timestamp) {
        var sb = new StringBuilder();
        sb.AppendLine(message);
        sb.AppendLine(timestamp);
        logger.Error(sb.ToString());
    }

    private void SqlLog(string message, HttpRequestMessage request, DateTime timestamp) {
        var requestedURi = (string)request.RequestUri.AbsoluteUri;
        var requestMethod = request.Method.ToString();
        SqlErrorLogging sqlErrorLogging = new SqlErrorLogging();
        ApiError apiError = new ApiError() {
            Message = message,
            RequestUri = requestedURi,
            RequestMethod = requestMethod,
            TimeUtc = timestamp
        };
        //sqlErrorLogging.InsertErrorLog(apiError);
    }

    private string FormatException(Exception ex, int depth = 0) {
        //... omitted for brevity
    }
}