raygun4net icon indicating copy to clipboard operation
raygun4net copied to clipboard

Inconsistency in added tags like "UnhandledException"

Open fschwiet opened this issue 6 years ago • 1 comments

It's nice that the tag "UnhandledException" gets added sometimes.. but it would be nice if every report that was initiated by Raygun had a tag indicating how the exception was caught..

For example, on on Android we have the UnhandledException tag, but not on Mac. I would hope both platforms would have such a tag:

  • https://github.com/MindscapeHQ/raygun4net/blob/637422bc0c28a06f3dda26737f118a0c632a019a/Mindscape.Raygun4Net.Xamarin.Android/RaygunClient.cs#L455
  • https://github.com/MindscapeHQ/raygun4net/blob/637422bc0c28a06f3dda26737f118a0c632a019a/Mindscape.Raygun4Net.Xamarin.Mac.Unified/RaygunClient.cs#L221

A second example is that in Android we have the same tag used for separate handlers. I would hope both exception sources could have their own name:

  • https://github.com/MindscapeHQ/raygun4net/blob/637422bc0c28a06f3dda26737f118a0c632a019a/Mindscape.Raygun4Net.Xamarin.Android/RaygunClient.cs#L446
  • https://github.com/MindscapeHQ/raygun4net/blob/637422bc0c28a06f3dda26737f118a0c632a019a/Mindscape.Raygun4Net.Xamarin.Android/RaygunClient.cs#L455

This seems like a straightforward thing to improve, heck maybe even I could do it. I wanted to get feedback though to find out if the project owners think it is a good idea. It seems adding tags should be safe, there might be more risk in changing tags (I would like to change https://github.com/MindscapeHQ/raygun4net/blob/637422bc0c28a06f3dda26737f118a0c632a019a/Mindscape.Raygun4Net.Xamarin.Android/RaygunClient.cs#L455 to use "UnhandledExceptionRaiser" instead of "UnhandledException").

fschwiet avatar Sep 19 '18 18:09 fschwiet

Hi fscwiet,

Thanks for starting a discussion about this. The tag "UnhandledException" is used by the Raygun service when calculating metrics that distinguish between exceptions that have caused an application crash, vs exceptions that do not crash the application. The thinking here is that exceptions that reach the unhandled exceptions hooks generally mean the application is going to crash, whereas exceptions caught in a try/catch don't.

As such, we wouldn't want these tags to be changed anywhere.

The missing tag in the Mac provider would be good to fix up though. I expect we won't get around to this for a while, so if you need this sooner than later, feel free to add a PR to speed this up.

As for distinguishing the different unhandled exception hooks such as in the Android provider, it shouldn't hurt to add an additional tag of "UnhandledExceptionRaiser", so you can add that too if you like.

-Jason Fauchelle

QuantumNightmare avatar Oct 10 '18 22:10 QuantumNightmare