NickCraver / StackExchange.Exceptional

Error handler used for the Stack Exchange network
https://nickcraver.com/StackExchange.Exceptional/
Apache License 2.0
862 stars 170 forks source link

Logging InnerException? #52

Closed AkosLukacs closed 6 years ago

AkosLukacs commented 10 years ago

Hi!

The InnerException is not logged, so information there is lost. Is this a design choice, or you just didn't had to look at InnerExceptions, and didn't implement this?

stijnherreman commented 10 years ago

Similarly, exceptions can refer to EntityValidationErrors which is not included (even though the cause can often be determined by looking at form values).

Nick, any idea if/how this could be added? I wouldn't mind giving it a try.

radenkozec commented 9 years ago

It seams that you have a bool AppendFullStackTrace which is by default false in ErrorStore.LogExceptionWithoutContext

CptRobby commented 9 years ago

@radenkozec I don't think that calling LogException with appendFullStackTrace set to true would give you the details of the InnerException(s). It appears that it merely gets a list of the function calls on the current thread's execution stack from where the LogException function is called. I don't really know how useful that would be since the code that threw the exception is more than likely no longer in the execution stack at that point.

I have actually added my own code that logs the InnerException details into the CustomData dictionary. It's really quite simple. You can see it at CptRobby/StackExchange.Exceptional@67f13660c3d473ffad7702bbea1fa9c2816b3542 ;)

radenkozec commented 9 years ago

@CptRobby Strange. I see in code that Detail is actually calling exception.ToString(). Exception.ToString() will give you all innerExceptions. More info here http://stackoverflow.com/questions/2176707/exception-message-vs-exception-tostring

So Actually you can just look at Detail property.

CptRobby commented 9 years ago

@radenkozec Wow. Good call. I guess I just wasn't noticing that before. I guess I'll just be removing that change then. Thanks :)

CptRobby commented 9 years ago

@stijnherreman Don't know if you're still interested in this or not, but I was having the same issue and I found an easy solution. Without editing StackExchange.Exceptional code, you need to create a function that you would use for calling the LogException function. The following is a LogException function that I added to my base Controller class:

    public StackExchange.Exceptional.Error LogException(Exception ex, HttpContext context = null)
    {
        //First check for EntityValidationErrors...
        var dbEx = ex as System.Data.Entity.Validation.DbEntityValidationException;
        if (dbEx != null)
        {
            var sb = new System.Text.StringBuilder();
            foreach (var errEntry in dbEx.EntityValidationErrors)
            {
                sb.AppendFormat("[{1}] {0} failed validation", errEntry.Entry.Entity.GetType(), errEntry.Entry.State);
                foreach (var err in errEntry.ValidationErrors)
                {
                    sb.AppendFormat("- {0} : {1}", err.PropertyName, err.ErrorMessage);
                }
            }
            dbEx.Data.Add("EntityValidationErrors", sb.ToString());
        }
        return StackExchange.Exceptional.ErrorStore.LogException(ex, context ?? HttpContext.ApplicationInstance.Context);
    }

Additionally you would have to edit your web.config file to set a matching regex pattern to the dataIncludePattern attribute of the Exceptional tag. So it would look something like this:

<Exceptional applicationName="My Website" dataIncludePattern=".*">

/.*/ would match on anything, which is my preference since I would just like to see everything anyways.

That's all there is to it. The EntityValidationErrors will now show up under the Custom section when viewing the details (and in the emails if you have those configured).

Enjoy! :)

NickCraver commented 9 years ago

Thanks for sharing @CptRobby, unfortunately I can't put that in the library because of the dependency it would need...but I do want to do something similar for AggregateExceptions as so many things move to async/parallel. I plan on doing this in 2.0, my biggest enemy at the moment is a changing CoreCLR landscape and as aways: time.

CptRobby commented 9 years ago

@NickCraver I wasn't suggesting that you should include it in the library. It is very application specific after all. I was just providing a suggestion for one way to deal with that issue for others to see. ;)

NickCraver commented 6 years ago

Closing out to tidy - anyone who wants to do this can certainly do so with a bit of code added (see above) or using a few new of the events in the save pipeline (though be sure not to recurse into an overflow there). Here's an example of such an approach:

Exceptional.Settings.OnAfterLog += (_, args) => args.Error?.Exception?.InnerException?.LogNoContext();