Circular Error Handler
-
Below is some code on an app I/we just started working on. It's the logging error handler for an application that is supposed to support a semi-huge user base.
I'd love to go into all the WTFs I see but I'll just point out the biggest ones.
- The dev lead wrote this.
- The only logging in the entire application are the errors even though it processes medical information.
- As part of the Catch it calls itself. This potentiall leads to a memory eating runaway condition.namespace MyErrorFreeApp
{
public class ErrHandler
{
public static void WriteError(string errorMessage)
{
try
{
string path = ConfigurationManager.AppSettings["LogPath"].ToString() + "/MyErrorFreeApp-" + DateTime.Today.ToString("dd-MM-yy") + ".txt";
if(!File.Exists(path))
{
File.Create(path).Close();
}
using (StreamWriter w = File.AppendText(path))
{
w.WriteLine("\r\nLog Entry : ");
w.WriteLine("{0}", DateTime.Now.ToString(CultureInfo.InvariantCulture));
string err = "Error in: " + System.Web.HttpContext.Current.Request.Url.ToString() + ". Error Message:" + errorMessage;
w.WriteLine(err);
w.WriteLine("____________________________________________________");
w.Flush();
w.Close();
HttpContext.Current.Session["errMsg"] = errorMessage;
HttpContext.Current.Response.Redirect("~/Error.aspx", false);
}
}
catch (Exception ex)
{
WriteError(ex.Message);
}
}
}
}I've already added logging from the MS Enterprise Library 5.0 to the app.
-
@SteamBoat said:
The only logging in the entire application are the errors even though it processes medical information.
Can you elaborate here? What else would you want to log? Seems like excessive logging could lead to nightmares, privacy-wise.
-
Does this run on Windows? If so there's a whole boatload of more WTFs in it.
-
@boomzilla said:
Can you elaborate here? What else would you want to log? Seems like excessive logging could lead to nightmares, privacy-wise.
Log every critical process! Anything that goes to IPC, anything that enters an important region, anything that handles risk. Think of the data as being passed in a bucket through a bucket brigade. Everyone in that line needs to say "Got it!" when they are handled the bucket. You don't need to shout out which fingers are holding the handle (except maybe if being passed to an arthritic member), and you of course don't want to expose sensitive data, but otherwise I strongly advocate to use sufficient logging so that you can always tell what is happening to your data and where.
If a process hangs, or takes an unusually long time, or incorrectly accepts bad values, etc, you need to know where and why. And if your data is being processed correctly, and everything is fast and shiny, you need to know that too! Even if it requires another process to purge off old logs every so often.
-
@blakeyrat said:
Does this run on Windows? If so there's a whole boatload of more WTFs in it.
It's written in C# using ASP.NET... I'd certainly hope it'd run on Windows.
-
@powerlord said:
@blakeyrat said:
Does this run on Windows? If so there's a whole boatload of more WTFs in it.
It's written in C# using ASP.NET... I'd certainly hope it'd run on Windows.I don't see anything in there that would fail under Mono. And he uses a Unix-style path in a constant. Let me know if I'm the idiot or if you are. I don't like making unwarranted assumptions.
MS Enterprise Library 5.0 is almost certainly Windows-only though, but I've never used it personally.
-
Some thoughts:
- The only logging in the entire application are the errors even though it processes medical information.
On one hand I can understand not wanting to log stuff that might contain PHI/PII because getting logs means scrubbing out that stuff and that fucking sucks. On the other hand you end up with a useless log file that logs shit like "Path cannot be null" and NO context. Kinda like MSBuild errors!
- As part of the Catch it calls itself. This potentiall leads to a memory eating runaway condition.
If you want to prove this to your boss just blast away that LogPath key in the config on a dev server and cause a known error and watch that baby crash!
And a noobie WTF:
I have to fucking insert HTML tags to get a non-shitty formatted post? Jesus it's like fucking 1995 up in this bitch!
-
@MathNerdCNU said:
Weird, even though you added HTML tags, your post remains shitty...Some thoughts:
- The only logging in the entire application are the errors even though it processes medical information.On one hand I can understand not wanting to log stuff that might contain PHI/PII because getting logs means scrubbing out that stuff and that fucking sucks. On the other hand you end up with a useless log file that logs shit like "Path cannot be null" and NO context. Kinda like MSBuild errors!
- As part of the Catch it calls itself. This potentiall leads to a memory eating runaway condition.
If you want to prove this to your boss just blast away that LogPath key in the config on a dev server and cause a known error and watch that baby crash!And a noobie WTF:
I have to fucking insert HTML tags to get a non-shitty formatted post? Jesus it's like fucking 1995 up in this bitch!
-
@C-Octothorpe said:
@MathNerdCNU said:
Weird, even though you added HTML tags, your post remains shitty...Some thoughts:
- The only logging in the entire application are the errors even though it processes medical information.On one hand I can understand not wanting to log stuff that might contain PHI/PII because getting logs means scrubbing out that stuff and that fucking sucks. On the other hand you end up with a useless log file that logs shit like "Path cannot be null" and NO context. Kinda like MSBuild errors!
- As part of the Catch it calls itself. This potentiall leads to a memory eating runaway condition.
If you want to prove this to your boss just blast away that LogPath key in the config on a dev server and cause a known error and watch that baby crash!And a noobie WTF:
I have to fucking insert HTML tags to get a non-shitty formatted post? Jesus it's like fucking 1995 up in this bitch!So what is shitty, my mad HTMLz(maybeVersionFIFVES!) or my content? Trolls gotta know how to improve they games...z...ies.
-
@MathNerdCNU said:
So what is shitty, my mad HTMLz(maybeVersionFIFVES!) or my content? Trolls gotta know how to improve they games...z...ies.
You gotta use a not-Chrome browser. Duh.
-
@blakeyrat said:
@powerlord said:
@blakeyrat said:
Does this run on Windows? If so there's a whole boatload of more WTFs in it.
It's written in C# using ASP.NET... I'd certainly hope it'd run on Windows.I don't see anything in there that would fail under Mono. And he uses a Unix-style path in a constant. Let me know if I'm the idiot or if you are. I don't like making unwarranted assumptions.
MS Enterprise Library 5.0 is almost certainly Windows-only though, but I've never used it personally.
It's a web application written in ASP.NET. The "Unix-style path" was likely a mistake as we only do web apps running on IIS.
-
@MathNerdCNU said:
Some thoughts:
- The only logging in the entire application are the errors even though it processes medical information.On one hand I can understand not wanting to log stuff that might contain PHI/PII because getting logs means scrubbing out that stuff and that fucking sucks. On the other hand you end up with a useless log file that logs shit like "Path cannot be null" and NO context. Kinda like MSBuild errors!
Yea, important and verbose error messages like this:
Log Entry :
09/13/2012 17:40:11
Error in: http://localhost:60460/Login.aspx. Error Message:Object reference not set to an instance of an object.
____________________________________________________
-
@blakeyrat said:
And he uses a Unix-style path in a constant.
Even on Unix/Linux, that path is a bit of a WTF in its own right (as opposed to Windows, where it simply wouldn't work). OK, so we can assume that www-data's home directory is the webroot in this case (if it isn't, that line is even more WTFy, to the extent of "how does anyone even come up with that?"). But there are two common webroot layouts: one is to have the entire directory tree as seen by the user in the webroot (in which case Error.aspx would work if someone typed it into their browser), and the other is to have it managed by the distro and put the data somewhere else (in which case you're just asking to have the distro randomly overwrite your error page at some undetermined point in the future). It also adds an unnecessary dependency on the user who happens to be running the web daemon (and it's not totally unheard of for people to run them under their own user during testing
It's saved from being a RWTF on the basis that there are setups which make it work that aren't completely insane, but they'd require rather unnecessary amounts of customization.
-
@blakeyrat said:
And he uses a Unix-style path in a constant.
That doesn't preclude Windows - it accepts either / or \ as a separator in paths.
-
@dhromed said:
@MathNerdCNU said:
So what is shitty, my mad HTMLz(maybeVersionFIFVES!) or my content? Trolls gotta know how to improve they games...z...ies.
You gotta use a not-Chrome browser. Duh.
Are you a wizard? How did you know?
@SteamBoat said:
@MathNerdCNU said:
Some thoughts:
- The only logging in the entire application are the errors even though it processes medical information.On one hand I can understand not wanting to log stuff that might contain PHI/PII because getting logs means scrubbing out that stuff and that fucking sucks. On the other hand you end up with a useless log file that logs shit like "Path cannot be null" and NO context. Kinda like MSBuild errors!
Yea, important and verbose error messages like this:
Log Entry :
09/13/2012 17:40:11
Error in: http://localhost:60460/Login.aspx. Error Message:Object reference not set to an instance of an object.
____________________________________________________
So, we agree that logging sucks?
-
@PJH said:
@blakeyrat said:
And he uses a Unix-style path in a constant.
That doesn't preclude Windows - it accepts either / or \ as a separator in paths.Yes but that doesn't make the path correct.
PROTIP: "works" isn't the same as "correct"
ANOTHER PROTIP: You could just assume I'm not a fucking retard and knew the path worked, especially since the OP said it did, and not have posted this pedantic dickweedery
-
@MathNerdCNU said:
Are you a wizard? How did you know?
The HTML field only works in IE and Firefox. And we just assume you're not one of those cretins using Safari or Opera.
-
@blakeyrat said:
PROTIP: "works" isn't the same as "correct"
Huh. And here I thought we were talking about WTF code, which after all usually comes from the "works... somehow... most of the time" end of the spectrum. It would seem that "correctness" is a concept that is about as familiar to WTF programmers as "honesty" is to politicians.
-
@blakeyrat said:
And we just assume you're not one of those cretins using Safari or Opera.
My ability to discern cretins is based on good statistics, sir!
