-10
catch(Exception ex)

//code being sent to database table to log is -

ex.ToString()

The parameters are not being sent.

private int GetAge (int ID)
{

}

private string GetName (int ID)
{

}

How do i write in a generic way so that all the methods can have parameters concatenated with exception(ex.ToString)? Is there any generic approach(for winforms)?

Sharpeye500
  • 8,775
  • 25
  • 95
  • 143
  • 6
    What parameters aren't being sent to what? – Steven Doggart Feb 11 '13 at 17:30
  • I want to catch the parameters the methods are consuming(for ex:ID), i can go and write inside each method to concatenate parameter to ex.ToString(), but it will be too many methods, is there any generic way? – Sharpeye500 Feb 11 '13 at 17:32
  • You want to log which ones are "missing", or you want to log the name/value pairs of all of the method parameters? – Steven Doggart Feb 11 '13 at 17:33
  • Can you please show sample of code where you want "catch the parameters the methods are consuming"? – Alexei Levenkov Feb 11 '13 at 17:33
  • you have your answer right in front of you.. with that code there alone there is nothing there but a method signature but common sense would be to `ex.ToString() + " " + ID.ToString(); but can't tell what it is you want because you have probably confused others as well as yourself – MethodMan Feb 11 '13 at 17:33
  • It would be helpful if you showed an example of what you'd like the error message to say. – Steven Doggart Feb 11 '13 at 17:34
  • You want the parameter names (i.e. 'ID') or the parameter values (i.e. ID=12345)? Not really sure what it is you are trying to do here. – DougEC Feb 11 '13 at 17:35
  • If the method fails(for ex:GetName, then when it hits the exception, i want to catch ID with ex.ToString, similarly when other methods fail, i would like to catch the values that are being sent as input parameters, is there any generic approach for this? I have an old project where parameters are not being sent to the exception log, which makes difficult to correct some of the production errors. – Sharpeye500 Feb 11 '13 at 17:37
  • 1
    In general, you should not be catching exceptions that you can't appropriately handle. Additionally you really don't want to concatenate an exception to parameters. You should allow exceptions to bubble up and fix the underlying cause. – Ryan Gates Feb 11 '13 at 17:38
  • Agreed, some of the errors don't occur in development and occur only in production, the only way i can check that is looking at what data being passed at. Then use that to fix it. – Sharpeye500 Feb 11 '13 at 17:39
  • 1
    -1. Please use "Edit" button and add sample with method that does `throw new Exception()`, try/catch and have comment where do you want to log something. – Alexei Levenkov Feb 11 '13 at 17:41

2 Answers2

4

This is what's being logged:

ex.ToString()

presumably because that's all you're logging. Something like this, I assume, is the logging statement?:

catch (Exception ex)
{
    Logger.Log(ex.ToString());
}

The way to add more information to what's being logged is to pass it to the logger. Something like this:

catch (Exception ex)
{
    Logger.Log(string.Format("Failed to get a Name for the ID: {0} - Exception was: {1}", ID, ex.ToString()));
}

This is assuming you're logging only strings. You can perhaps expand this to include more strongly-typed data perhaps with custom exceptions being passed directly to the logger itself. It's hard to say without knowing more about the capabilities of the logger. Maybe a custom exception with a generic type parameter which internally adds the parameter to its string representation? Again, it's hard to say in this one simple example without more information.

As for making the whole thing generic, that's really up to your own designs. In the two method signatures you provide there is only one parameter. So a single generic property on a custom exception would work for that. What about methods with multiple parameters? What about methods with context above and beyond its own parameters? (Instance properties on the method's class, for example.)

Each exception is potentially vastly different. Each code block is its own piece of logic in the domain. And providing meaningful context with errors (which is an excellent thing to do, by the way) is often very subjective to those differences in the logic for those code blocks. Sometimes it's strongly typed information, sometimes it's human-readable context about the situation, etc.

Working with legacy code, a workable long-term approach is to identify the errors which require additional information and modify the error-handling code to provide that information. You can often catch a lot of them in one pass to production, but there will be stragglers. In time that number would decrease. It's not the most desirable approach from management's perspective because it often requires two production releases to fix one error (one release to add more logging, another to actually fix the error after the necessary runtime information is provided). But, well, that's the technical debt incurred from such novel gems of bad code as this:

catch (Exception ex)
{
    Logger.Log(ex.ToString());
}
David
  • 208,112
  • 36
  • 198
  • 279
0

@David's approach is fine if your Logger is exposed at the same level as GetAge or GetName. However if you're logger is at a higher level (e.g. a Top Level Exception handler) and you want to add context to an exception that is at a lower level you might want to wrap exceptions instead if you want to add context to an error.

e.g.

private int GetAge (int ID)
{
   try 
   {
        GetAgeFromFile(ID)


   }
   catch {System.IO.Exception e }
   {
        throw new CommunctionError(
        string.Format("Cannot access age file ID:{0}.",ID) ,
        e);
   }

}

Note that the exception filter limited this to very specific errors. Except for a top level exception handlers you should always limit your catches to things you can reasonable recover from.

Conrad Frix
  • 51,984
  • 12
  • 96
  • 155