exceptionless / Exceptionless.Net

Exceptionless clients for the .NET platform
https://exceptionless.com
Other
551 stars 142 forks source link

Breaking change - System.Diagnostics.ExceptionExtensions now marked as internal #285

Closed Jaans closed 1 year ago

Jaans commented 1 year ago

Hi Guys!

Thanks for the hard work and the latest updates. Really appreciate the quality and ease of use!

Ran into the breaking change from #266 where the System.Diagnostics.ExceptionExtensions has been internalized. I understand the desire to do this and have no issue with it.

What I wanted to get some feedback on is the recommended path for our scenario. Some time ago, we used to have a dependency on Demystifier (@benaadams) but then saw that this was duplicated in Exceptionless and removed our dependency and used the .Demystify() method from the above mentioned internalized extensions class.

We still want to use .Demystify() E.g. for UI/Integration based exception messages which may or may not have a corresponding call to Exceptionless to "log it". It seems that Exceptionless has the demystifier implementation internally added and not as an external dependency on the Demystifier NuGet package - does that mean we have take a dependency on that NuGet package ourselves despite an implementation available within Exceptionless? (We have 463 projects using it so not a trivial decision for us).

Also, just to confirm, does Exceptionless apply the demystification logic implicitly for all exceptions sent (and that we don't have to do it before logging anymore) ?

Thanks again, Jaans.

niemyjski commented 1 year ago

If you want to show a pretty exception message to users than I'd bring in Demystifier to show it, but I'd be hesitant on showing any stack trace to end user as it could expose private information to consumers.

As for Exceptionless logging, I would think it would always be better to pass in the original unchanged exception for processing, that way we can ensure we dedupe it properly based on hash and all that goodness. We do run demystifying on it client side, but we could someday remove it and do it server side.

Jaans commented 1 year ago

OK. Thank you for your feedback. We'll take our own dependency on Demystifier NuGet package then and not invoke it on exceptions sent to Exceptionless.