Add inner exception text as a property
emumanu opened this issue · 4 comments
Hello,
first of all. Thank you for Serilog.Exceptions.
I want to ask you for an improvement:
most of the time, you should take a look at the message of the most inner exception to understand why there was a problem. It would be great if you can add an option to generate a property (f.e. InnerMostExceptionMessage") with that message.
It should take you very little time to add and I think that makes debugging a problem a lot easier.
BR
That's a good idea. However, this does make each log message much bigger, so would be a feature that we'd probably turn off by default. We'd accept a PR for this feature.
Good!
As soon as I find some free time I'll try to submit a PR.
I can see how in some rare scenarios it might be a little bit helpful. However, in general, I consider this idea "feature creep" in the context of Serilog.Exceptions
. I think it will add complexity for very little to no gain.
Some rationale:
A) There is a discussion of how to select inner-most exception in case of few candidates.
B) Same question for cyclic object graphs.
C)
C) It still does not solve the problem. I recall that in one project I actually had to deal with exceptions two-level deep(parsing of text) but displaying the message on the top-level would not help because I needed other properties on inner exception as well(for example column, line, offending token and so on). Moreover, even our homepage mentions an example for which this feature would not help
"Message": "Validation failed for one or more entities. See 'EntityValidationErrors' property for more details.",
. In that case you still need other properties.
To sum up, I think we should stick to destructuring exception's object graph and avoid the temptation to add such features.
Regarding A, I think we could just call exception.GetBaseException()
.
Regarding C, I agree that having the full exception information is way more useful.
Simplicity is also a feature. So we do need to be careful when adding features. Lets put this on hold and see if we get more requests for this feature.