From 65da9b7be14de67ac94291bac2a426414b8446d6 Mon Sep 17 00:00:00 2001 From: Karl Stenerud Date: Mon, 18 Nov 2024 15:17:08 +0100 Subject: [PATCH 01/10] Send native mapped information in stack traces from system exceptions --- .../Assets/Bugsnag/Runtime/INativeClient.cs | 8 +- Bugsnag/Assets/Bugsnag/Runtime/LoadedImage.cs | 4 +- .../Runtime/Native/Android/NativeClient.cs | 4 +- .../Runtime/Native/Cocoa/LoadedImages.cs | 88 ++++++++++---- .../Runtime/Native/Cocoa/NativeClient.cs | 114 +++++++++++++++++- .../Runtime/Native/Fallback/NativeClient.cs | 4 +- .../Runtime/Native/Windows/NativeClient.cs | 4 +- .../Bugsnag/Runtime/Payload/ErrorBuilder.cs | 10 +- .../Bugsnag/Runtime/Payload/StackTraceLine.cs | 66 +++++++++- 9 files changed, 254 insertions(+), 48 deletions(-) diff --git a/Bugsnag/Assets/Bugsnag/Runtime/INativeClient.cs b/Bugsnag/Assets/Bugsnag/Runtime/INativeClient.cs index a65685db9..2a97a2225 100644 --- a/Bugsnag/Assets/Bugsnag/Runtime/INativeClient.cs +++ b/Bugsnag/Assets/Bugsnag/Runtime/INativeClient.cs @@ -103,13 +103,7 @@ interface INativeClient : IFeatureFlagStore IDictionary GetNativeMetadata(); - /// - /// Find the native loaded image that corresponds to a native instruction address - /// supplied by il2cpp_native_stack_trace(). - /// - /// The address to find the corresponding image of - /// The corresponding image, or null - LoadedImage FindImageAtAddress(UInt64 address); + StackTraceLine[] ToStackFrames(System.Exception exception); bool ShouldAttemptDelivery(); diff --git a/Bugsnag/Assets/Bugsnag/Runtime/LoadedImage.cs b/Bugsnag/Assets/Bugsnag/Runtime/LoadedImage.cs index 4941e9c90..f31bbc99e 100644 --- a/Bugsnag/Assets/Bugsnag/Runtime/LoadedImage.cs +++ b/Bugsnag/Assets/Bugsnag/Runtime/LoadedImage.cs @@ -4,17 +4,19 @@ namespace BugsnagUnity { class LoadedImage { - public LoadedImage(UInt64 loadAddress, UInt64 size, string fileName, string uuid) + public LoadedImage(UInt64 loadAddress, UInt64 size, string fileName, string uuid, bool isMainImage) { LoadAddress = loadAddress; Size = size; FileName = fileName; Uuid = uuid; + IsMainImage = isMainImage; } public readonly UInt64 LoadAddress; public readonly UInt64 Size; public readonly string FileName; public readonly string Uuid; + public readonly bool IsMainImage; } } diff --git a/Bugsnag/Assets/Bugsnag/Runtime/Native/Android/NativeClient.cs b/Bugsnag/Assets/Bugsnag/Runtime/Native/Android/NativeClient.cs index 184d01e73..dad0096f2 100644 --- a/Bugsnag/Assets/Bugsnag/Runtime/Native/Android/NativeClient.cs +++ b/Bugsnag/Assets/Bugsnag/Runtime/Native/Android/NativeClient.cs @@ -164,9 +164,9 @@ public void RegisterForOnSessionCallbacks() NativeInterface.RegisterForOnSessionCallbacks(); } - public LoadedImage FindImageAtAddress(UInt64 address) + public StackTraceLine[] ToStackFrames(System.Exception exception) { - return null; + return new StackTraceLine[0]; } } diff --git a/Bugsnag/Assets/Bugsnag/Runtime/Native/Cocoa/LoadedImages.cs b/Bugsnag/Assets/Bugsnag/Runtime/Native/Cocoa/LoadedImages.cs index df29474e2..7df4f984d 100644 --- a/Bugsnag/Assets/Bugsnag/Runtime/Native/Cocoa/LoadedImages.cs +++ b/Bugsnag/Assets/Bugsnag/Runtime/Native/Cocoa/LoadedImages.cs @@ -5,40 +5,77 @@ namespace BugsnagUnity { + /// + /// Caches the list of loaded images as reported by the native runtime, and provides searching by address. + /// class LoadedImages { /// /// Refresh the list of loaded images to match what the native side currently says. /// + /// The file name of the main image file /// - /// Note: You MUST call this at least once before using an instance of this class! + /// Note: If anything goes wrong during the refresh, the currently cached state won't change. /// - public void Refresh() + #nullable enable + public void Refresh(String? mainImageFileName) { UInt64 loadedNativeImagesAt = NativeCode.bugsnag_lastChangedLoadedImages(); - if (loadedNativeImagesAt == lastLoadedNativeImagesAt) + if (loadedNativeImagesAt == LastLoadedNativeImagesAt) + { + // Only refresh if something changed. + return; + } + + var imageCount = NativeCode.bugsnag_getLoadedImageCount(); + if (imageCount == 0) { return; } - lastLoadedNativeImagesAt = loadedNativeImagesAt; // Ask for the current count * 2 in case new images get added between calls - var nativeImages = new NativeLoadedImage[NativeCode.bugsnag_getLoadedImageCount() * 2]; + var nativeImages = new NativeLoadedImage[imageCount * 2]; var count = NativeCode.bugsnag_getLoadedImages(nativeImages, (UInt64)nativeImages.LongLength); - var images = new LoadedImage[count]; - for (UInt64 i = 0; i < count; i++) + try + { + if (count == 0) + { + return; + } + + UInt64 mainLoadAddress = 0; + var images = new LoadedImage[count]; + for (UInt64 i = 0; i < count; i++) + { + var nativeImage = nativeImages[i]; + var uuid = new byte[16]; + Marshal.Copy(nativeImage.UuidBytes, uuid, 0, 16); + var fileName = Marshal.PtrToStringAnsi(nativeImage.FileName); + var isMainImage = fileName == mainImageFileName; + + var image = new LoadedImage(nativeImage.LoadAddress, + nativeImage.Size, + fileName, + new Guid(uuid).ToString(), + isMainImage); + if (isMainImage) + { + mainLoadAddress = image.LoadAddress; + } + images[i] = image; + } + + // Update cache + Images = images; + MainImageLoadAddress = mainLoadAddress; + LowestImageLoadAddress = images[0].LoadAddress; + LastLoadedNativeImagesAt = loadedNativeImagesAt; + } + finally { - var nativeImage = nativeImages[i]; - var uuid = new byte[16]; - Marshal.Copy(nativeImage.UuidBytes, uuid, 0, 16); - images[i] = new LoadedImage(nativeImage.LoadAddress, - nativeImage.Size, - Marshal.PtrToStringAnsi(nativeImage.FileName), - new Guid(uuid).ToString()); + // bugsnag_getLoadedImages() locks a mutex, so we must call bugsnag_unlockLoadedImages() + NativeCode.bugsnag_unlockLoadedImages(); } - Images = images; - // bugsnag_getLoadedImages() locks a mutex, so we must call bugsnag_unlockLoadedImages() - NativeCode.bugsnag_unlockLoadedImages(); } /// @@ -47,8 +84,14 @@ public void Refresh() /// /// The address to find the corresponding image of /// The corresponding image, or null - public LoadedImage FindImageAtAddress(UInt64 address) + #nullable enable + public LoadedImage? FindImageAtAddress(UInt64 address) { + if (address < LowestImageLoadAddress) + { + address += MainImageLoadAddress; + } + int idx = Array.BinarySearch(Images, address, new AddressToImageComparator()); if (idx < 0) { @@ -57,11 +100,10 @@ public LoadedImage FindImageAtAddress(UInt64 address) return Images[idx]; } - /// - /// The currently loaded images, as of the last call to Refresh(). - /// - public LoadedImage[] Images; - private UInt64 lastLoadedNativeImagesAt = 0; + private LoadedImage[] Images = new LoadedImage[0]; + private UInt64 MainImageLoadAddress = 0; + private UInt64 LowestImageLoadAddress = 0; + private UInt64 LastLoadedNativeImagesAt = 0; private class AddressToImageComparator : IComparer { diff --git a/Bugsnag/Assets/Bugsnag/Runtime/Native/Cocoa/NativeClient.cs b/Bugsnag/Assets/Bugsnag/Runtime/Native/Cocoa/NativeClient.cs index e72201f8a..1faee71c7 100644 --- a/Bugsnag/Assets/Bugsnag/Runtime/Native/Cocoa/NativeClient.cs +++ b/Bugsnag/Assets/Bugsnag/Runtime/Native/Cocoa/NativeClient.cs @@ -3,6 +3,7 @@ using System; using System.Collections; using System.Collections.Generic; +using System.Diagnostics; using System.IO; using System.Runtime.InteropServices; using System.Text; @@ -20,7 +21,7 @@ class NativeClient : INativeClient private static NativeClient _instance; private bool _registeredForSessionCallbacks; - private LoadedImages loadedImages; + private LoadedImages loadedImages = new LoadedImages(); public NativeClient(Configuration configuration) { @@ -488,12 +489,117 @@ public void RegisterForOnSessionCallbacks() NativeCode.bugsnag_registerForSessionCallbacksAfterStart(HandleSessionCallbacks); } - public LoadedImage FindImageAtAddress(UInt64 address) + [DllImport("__Internal")] + private static extern IntPtr il2cpp_gchandle_get_target(int gchandle); + + [DllImport("__Internal")] + private static extern void il2cpp_free(IntPtr ptr); + + [DllImport("__Internal")] + private static extern void il2cpp_native_stack_trace(IntPtr exc, out IntPtr addresses, out int numFrames, out IntPtr imageUUID, out IntPtr imageName); + + #nullable enable + private static string? ExtractString(IntPtr pString) + { + return (pString == IntPtr.Zero) ? null : Marshal.PtrToStringAnsi(pString); + } + + private StackTraceLine[] ToStackFrames(System.Exception exception, IntPtr[] nativeAddresses) { - loadedImages.Refresh(); - return loadedImages.FindImageAtAddress(address); + var systemTrace = new StackTrace(exception, true); + var stackFrames = new StackTraceLine[systemTrace.FrameCount]; + for (int i = 0; i < systemTrace.FrameCount; i++) + { + var method = systemTrace.GetFrame(i).GetMethod(); // Only method has useful info + var address = (UInt64)nativeAddresses[i].ToInt64(); + var image = loadedImages.FindImageAtAddress(address); + + var trace = new StackTraceLine(); + trace.FrameAddress = address.ToString(); + trace.Method = method.ToString(); + if (image != null) + { + if (address < image.LoadAddress) + { + // It's a relative address + trace.FrameAddress = (address + image.LoadAddress).ToString(); + } + trace.MachoFile = image.FileName; + trace.MachoLoadAddress = image.LoadAddress.ToString(); + trace.MachoUuid = image.Uuid; + trace.InProject = image.IsMainImage; + } + stackFrames[i] = trace; + } + return stackFrames; } + public StackTraceLine[] ToStackFrames(System.Exception exception) + { + var notFound = new StackTraceLine[0]; + + if (exception == null) + { + return notFound; + } + +#if ENABLE_IL2CPP + var hException = GCHandle.Alloc(exception); + var pNativeAddresses = IntPtr.Zero; + var pImageUuid = IntPtr.Zero; + var pImageName = IntPtr.Zero; + try + { + if (hException == null) + { + return notFound; + } + + var pException = il2cpp_gchandle_get_target(GCHandle.ToIntPtr(hException).ToInt32()); + if (pException == IntPtr.Zero) + { + return notFound; + } + + var frameCount = 0; + string? mainImageFileName = null; + + il2cpp_native_stack_trace(pException, out pNativeAddresses, out frameCount, out pImageUuid, out pImageName); + if (pNativeAddresses == IntPtr.Zero) + { + return notFound; + } + + mainImageFileName = ExtractString(pImageName); + var nativeAddresses = new IntPtr[frameCount]; + Marshal.Copy(pNativeAddresses, nativeAddresses, 0, frameCount); + + loadedImages.Refresh(mainImageFileName); + return ToStackFrames(exception, nativeAddresses); + } + finally + { + if (pImageUuid != IntPtr.Zero) + { + il2cpp_free(pImageUuid); + } + if (pImageName != IntPtr.Zero) + { + il2cpp_free(pImageName); + } + if (pNativeAddresses != IntPtr.Zero) + { + il2cpp_free(pNativeAddresses); + } + if (hException != null) + { + hException.Free(); + } + } +#else + return notFound; +#endif + } } } #endif \ No newline at end of file diff --git a/Bugsnag/Assets/Bugsnag/Runtime/Native/Fallback/NativeClient.cs b/Bugsnag/Assets/Bugsnag/Runtime/Native/Fallback/NativeClient.cs index d42f34ad3..5b5b3a8e3 100644 --- a/Bugsnag/Assets/Bugsnag/Runtime/Native/Fallback/NativeClient.cs +++ b/Bugsnag/Assets/Bugsnag/Runtime/Native/Fallback/NativeClient.cs @@ -171,9 +171,9 @@ public void RegisterForOnSessionCallbacks() // Not Used on this platform } - public LoadedImage FindImageAtAddress(UInt64 address) + public StackTraceLine[] ToStackFrames(System.Exception exception) { - return null; + return new StackTraceLine[0]; } } } diff --git a/Bugsnag/Assets/Bugsnag/Runtime/Native/Windows/NativeClient.cs b/Bugsnag/Assets/Bugsnag/Runtime/Native/Windows/NativeClient.cs index bc977518a..facb5a75d 100644 --- a/Bugsnag/Assets/Bugsnag/Runtime/Native/Windows/NativeClient.cs +++ b/Bugsnag/Assets/Bugsnag/Runtime/Native/Windows/NativeClient.cs @@ -218,9 +218,9 @@ public void RegisterForOnSessionCallbacks() // Not Used on this platform } - public LoadedImage FindImageAtAddress(UInt64 address) + public StackTraceLine[] ToStackFrames(System.Exception exception) { - return null; + return new StackTraceLine[0]; } } } diff --git a/Bugsnag/Assets/Bugsnag/Runtime/Payload/ErrorBuilder.cs b/Bugsnag/Assets/Bugsnag/Runtime/Payload/ErrorBuilder.cs index db16d05a2..7df602d03 100644 --- a/Bugsnag/Assets/Bugsnag/Runtime/Payload/ErrorBuilder.cs +++ b/Bugsnag/Assets/Bugsnag/Runtime/Payload/ErrorBuilder.cs @@ -93,13 +93,19 @@ internal Error FromSystemException(System.Exception exception, string stackTrace return new Error(errorClass, exception.Message, lines); } + internal Error FromSystemException(System.Exception exception, System.Diagnostics.StackFrame[] alternativeStackTrace) { + var frames = NativeClient.ToStackFrames(exception); var errorClass = exception.GetType().Name; - // JVM exceptions in the main thread are handled by unity and require extra formatting - if (errorClass == ANDROID_JAVA_EXCEPTION_CLASS) + if (frames.Length > 0) + { + return new Error(errorClass, exception.Message, frames); + } + else if (errorClass == ANDROID_JAVA_EXCEPTION_CLASS) { + // JVM exceptions in the main thread are handled by unity and require extra formatting var androidErrorData = ProcessAndroidError(exception.Message); var androidErrorClass = androidErrorData[0]; var androidErrorMessage = androidErrorData[1]; diff --git a/Bugsnag/Assets/Bugsnag/Runtime/Payload/StackTraceLine.cs b/Bugsnag/Assets/Bugsnag/Runtime/Payload/StackTraceLine.cs index c40ad6ff5..07a6f3a1c 100644 --- a/Bugsnag/Assets/Bugsnag/Runtime/Payload/StackTraceLine.cs +++ b/Bugsnag/Assets/Bugsnag/Runtime/Payload/StackTraceLine.cs @@ -140,6 +140,10 @@ internal StackTraceLine(Dictionary data) } } + internal StackTraceLine() + { + } + public string File { get @@ -176,14 +180,66 @@ public string Method } } - public string FrameAddress { get; set; } + public string FrameAddress { + get + { + return this.Get("frameAddress") as string; + } + set + { + this.AddToPayload("frameAddress", value); + } + } + + public string MachoLoadAddress { + get + { + return this.Get("machoLoadAddress") as string; + } + set + { + this.AddToPayload("machoLoadAddress", value); + } + } + + public string MachoFile { + get + { + return this.Get("machoFile") as string; + } + set + { + this.AddToPayload("machoFile", value); + } + } + + public string MachoUuid { + get + { + return this.Get("machoUUID") as string; + } + set + { + this.AddToPayload("machoUUID", value); + } + } + + public bool? InProject + { + get + { + return this.Get("inProject") as bool?; + } + set + { + this.AddToPayload("inProject", value); + } + } + + public bool? IsLr { get; set; } public bool? IsPc { get; set; } - public string MachoFile { get; set; } - public string MachoLoadAddress { get; set; } - public string MachoUuid { get; set; } public string MachoVmAddress { get; set; } public string SymbolAddress { get; set; } - public bool? InProject { get; set; } } } From 4b6e859502705378b5bda7c557e191a8db8edfa9 Mon Sep 17 00:00:00 2001 From: Karl Stenerud Date: Tue, 19 Nov 2024 10:11:14 +0100 Subject: [PATCH 02/10] Fetch method name from the string-based stack trace so that we get the full dotted path --- .../Assets/Bugsnag/Runtime/Native/Cocoa/NativeClient.cs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Bugsnag/Assets/Bugsnag/Runtime/Native/Cocoa/NativeClient.cs b/Bugsnag/Assets/Bugsnag/Runtime/Native/Cocoa/NativeClient.cs index 1faee71c7..cf4a4366a 100644 --- a/Bugsnag/Assets/Bugsnag/Runtime/Native/Cocoa/NativeClient.cs +++ b/Bugsnag/Assets/Bugsnag/Runtime/Native/Cocoa/NativeClient.cs @@ -506,11 +506,12 @@ public void RegisterForOnSessionCallbacks() private StackTraceLine[] ToStackFrames(System.Exception exception, IntPtr[] nativeAddresses) { - var systemTrace = new StackTrace(exception, true); - var stackFrames = new StackTraceLine[systemTrace.FrameCount]; - for (int i = 0; i < systemTrace.FrameCount; i++) + var unityTrace = new PayloadStackTrace(exception.StackTrace).StackTraceLines; + var length = nativeAddresses.Length < unityTrace.Length ? nativeAddresses.Length : unityTrace.Length; + var stackFrames = new StackTraceLine[length]; + for (int i = 0; i < length; i++) { - var method = systemTrace.GetFrame(i).GetMethod(); // Only method has useful info + var method = unityTrace[i].Method; var address = (UInt64)nativeAddresses[i].ToInt64(); var image = loadedImages.FindImageAtAddress(address); From 08dae142b422e4c7ba72f70a2cbeb1501e8da21d Mon Sep 17 00:00:00 2001 From: Karl Stenerud Date: Tue, 19 Nov 2024 12:45:54 +0100 Subject: [PATCH 03/10] Add e2e test for ios frames --- features/csharp/csharp_events.feature | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/features/csharp/csharp_events.feature b/features/csharp/csharp_events.feature index fb5a8f17d..b9dfc0af8 100644 --- a/features/csharp/csharp_events.feature +++ b/features/csharp/csharp_events.feature @@ -30,6 +30,24 @@ Feature: csharp events And expected device metadata is included in the event And expected app metadata is included in the event + @ios_only + Scenario: Uncaught Exception ios smoke test + When I run the game in the "UncaughtExceptionSmokeTest" state + And I wait to receive an error + Then the error is valid for the error reporting API sent by the Unity notifier + And the exception "errorClass" equals "Exception" + And the exception "message" equals "UncaughtExceptionSmokeTest" + And the event "unhandled" is false + And custom metadata is included in the event + And expected device metadata is included in the event + And expected app metadata is included in the event + And the error payload field "events.0.exceptions.0.stacktrace.0.frameAddress" matches the regex "\d+" + And the error payload field "events.0.exceptions.0.stacktrace.0.method" equals "UncaughtExceptionSmokeTest.Run()" + And the error payload field "events.0.exceptions.0.stacktrace.0.machoFile" matches the regex ".*/UnityFramework.framework/UnityFramework" + And the error payload field "events.0.exceptions.0.stacktrace.0.machoLoadAddress" matches the regex "\d+" + And the error payload field "events.0.exceptions.0.stacktrace.0.machoUUID" matches the regex "[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}" + And the error payload field "events.0.exceptions.0.stacktrace.0.inProject" is true + Scenario: Debug Log Exception smoke test When I run the game in the "DebugLogExceptionSmokeTest" state And I wait to receive an error From dbd06bd8df7af8fcbe1785ec4801c255c865a649 Mon Sep 17 00:00:00 2001 From: Karl Stenerud Date: Fri, 6 Dec 2024 09:56:34 +0100 Subject: [PATCH 04/10] Disable native trace capture on older Unity --- .../Runtime/Native/Cocoa/NativeClient.cs | 23 +++++++++++-------- features/csharp/csharp_events.feature | 1 + features/support/env.rb | 9 ++++++++ 3 files changed, 23 insertions(+), 10 deletions(-) diff --git a/Bugsnag/Assets/Bugsnag/Runtime/Native/Cocoa/NativeClient.cs b/Bugsnag/Assets/Bugsnag/Runtime/Native/Cocoa/NativeClient.cs index cf4a4366a..5f8f5031d 100644 --- a/Bugsnag/Assets/Bugsnag/Runtime/Native/Cocoa/NativeClient.cs +++ b/Bugsnag/Assets/Bugsnag/Runtime/Native/Cocoa/NativeClient.cs @@ -489,15 +489,6 @@ public void RegisterForOnSessionCallbacks() NativeCode.bugsnag_registerForSessionCallbacksAfterStart(HandleSessionCallbacks); } - [DllImport("__Internal")] - private static extern IntPtr il2cpp_gchandle_get_target(int gchandle); - - [DllImport("__Internal")] - private static extern void il2cpp_free(IntPtr ptr); - - [DllImport("__Internal")] - private static extern void il2cpp_native_stack_trace(IntPtr exc, out IntPtr addresses, out int numFrames, out IntPtr imageUUID, out IntPtr imageName); - #nullable enable private static string? ExtractString(IntPtr pString) { @@ -535,6 +526,17 @@ private StackTraceLine[] ToStackFrames(System.Exception exception, IntPtr[] nati return stackFrames; } +#if ENABLE_IL2CPP && UNITY_2021_3_OR_NEWER + [DllImport("__Internal")] + private static extern IntPtr il2cpp_gchandle_get_target(int gchandle); + + [DllImport("__Internal")] + private static extern void il2cpp_free(IntPtr ptr); + + [DllImport("__Internal")] + private static extern void il2cpp_native_stack_trace(IntPtr exc, out IntPtr addresses, out int numFrames, out IntPtr imageUUID, out IntPtr imageName); +#endif + public StackTraceLine[] ToStackFrames(System.Exception exception) { var notFound = new StackTraceLine[0]; @@ -544,7 +546,7 @@ public StackTraceLine[] ToStackFrames(System.Exception exception) return notFound; } -#if ENABLE_IL2CPP +#if ENABLE_IL2CPP && UNITY_2021_3_OR_NEWER var hException = GCHandle.Alloc(exception); var pNativeAddresses = IntPtr.Zero; var pImageUuid = IntPtr.Zero; @@ -603,4 +605,5 @@ public StackTraceLine[] ToStackFrames(System.Exception exception) } } } + #endif \ No newline at end of file diff --git a/features/csharp/csharp_events.feature b/features/csharp/csharp_events.feature index b9dfc0af8..598eeed9f 100644 --- a/features/csharp/csharp_events.feature +++ b/features/csharp/csharp_events.feature @@ -31,6 +31,7 @@ Feature: csharp events And expected app metadata is included in the event @ios_only + @skip_before_unity_2021 Scenario: Uncaught Exception ios smoke test When I run the game in the "UncaughtExceptionSmokeTest" state And I wait to receive an error diff --git a/features/support/env.rb b/features/support/env.rb index f3d75ae48..4a0769e64 100644 --- a/features/support/env.rb +++ b/features/support/env.rb @@ -9,6 +9,15 @@ end end +Before('@skip_before_unity_2021') do |_scenario| + if ENV['UNITY_VERSION'] + unity_version = ENV['UNITY_VERSION'][0..3].to_i + if unity_version < 2021 + skip_this_scenario('Skipping scenario on Unity < 2021') + end + end +end + Before('@skip_webgl') do |_scenario| skip_this_scenario('Skipping scenario') unless Maze.config.browser.nil? From 2c13735ca06888bb8bc5ef05197888a0d4e2d2f6 Mon Sep 17 00:00:00 2001 From: Richard Elms Date: Tue, 7 Jan 2025 08:10:37 +0100 Subject: [PATCH 05/10] allow for the notifier to be started on RuntimeInitializeLoadType.SubsystemRegistration (#862) * enable earlier start * before scene load * changelog --- .../Assets/Bugsnag/Runtime/BugsnagAutoInit.cs | 2 +- Bugsnag/Assets/Bugsnag/Runtime/Client.cs | 9 +- Bugsnag/Assets/Bugsnag/Runtime/Delivery.cs | 4 +- .../Runtime/MainThreadDispatchBehaviour.cs | 106 +++++++----------- CHANGELOG.md | 6 + 5 files changed, 50 insertions(+), 77 deletions(-) diff --git a/Bugsnag/Assets/Bugsnag/Runtime/BugsnagAutoInit.cs b/Bugsnag/Assets/Bugsnag/Runtime/BugsnagAutoInit.cs index 26c076da4..754c5c9dc 100644 --- a/Bugsnag/Assets/Bugsnag/Runtime/BugsnagAutoInit.cs +++ b/Bugsnag/Assets/Bugsnag/Runtime/BugsnagAutoInit.cs @@ -4,7 +4,7 @@ namespace BugsnagUnity public class BugsnagAutoInit { - [RuntimeInitializeOnLoadMethod(RuntimeInitializeLoadType.BeforeSceneLoad)] + [RuntimeInitializeOnLoadMethod(RuntimeInitializeLoadType.SubsystemRegistration)] static void OnBeforeSceneLoadRuntimeMethod() { var settings = Resources.Load("Bugsnag/BugsnagSettingsObject"); diff --git a/Bugsnag/Assets/Bugsnag/Runtime/Client.cs b/Bugsnag/Assets/Bugsnag/Runtime/Client.cs index c751840e6..d84486303 100644 --- a/Bugsnag/Assets/Bugsnag/Runtime/Client.cs +++ b/Bugsnag/Assets/Bugsnag/Runtime/Client.cs @@ -116,7 +116,6 @@ private void SetupAdvancedExceptionInterceptor() public Client(INativeClient nativeClient) { - InitMainthreadDispatcher(); NativeClient = nativeClient; _errorBuilder = new ErrorBuilder(nativeClient); CacheManager = new CacheManager(Configuration); @@ -144,11 +143,6 @@ public Client(INativeClient nativeClient) InitLogHandlers(); } - private void InitMainthreadDispatcher() - { - MainThreadDispatchBehaviour.Instance(); - } - private bool IsUnity2019OrHigher() { var version = Application.unityVersion; @@ -384,8 +378,7 @@ private void Notify(Error[] exceptions, HandledState handledState, Func { NotifyOnMainThread(exceptions, handledState, callback,logType, correlation); }); + MainThreadDispatchBehaviour.Enqueue(() => { NotifyOnMainThread(exceptions, handledState, callback,logType, correlation); }); } catch { diff --git a/Bugsnag/Assets/Bugsnag/Runtime/Delivery.cs b/Bugsnag/Assets/Bugsnag/Runtime/Delivery.cs index 55b8601fd..644c038b2 100644 --- a/Bugsnag/Assets/Bugsnag/Runtime/Delivery.cs +++ b/Bugsnag/Assets/Bugsnag/Runtime/Delivery.cs @@ -91,7 +91,7 @@ public void Deliver(IPayload payload) } try { - MainThreadDispatchBehaviour.Instance().Enqueue(PushToServer(payload)); + MainThreadDispatchBehaviour.Enqueue(PushToServer(payload)); } catch { @@ -508,7 +508,7 @@ public void StartDeliveringCachedPayloads() try { _finishedCacheDeliveries.Clear(); - MainThreadDispatchBehaviour.Instance().Enqueue(DeliverCachedPayloads()); + MainThreadDispatchBehaviour.Enqueue(DeliverCachedPayloads()); } catch { diff --git a/Bugsnag/Assets/Bugsnag/Runtime/MainThreadDispatchBehaviour.cs b/Bugsnag/Assets/Bugsnag/Runtime/MainThreadDispatchBehaviour.cs index 8366f333a..54183fb9a 100644 --- a/Bugsnag/Assets/Bugsnag/Runtime/MainThreadDispatchBehaviour.cs +++ b/Bugsnag/Assets/Bugsnag/Runtime/MainThreadDispatchBehaviour.cs @@ -1,49 +1,50 @@ -/* -Copyright 2015 Pim de Witte All Rights Reserved. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -using System; +using System; using System.Collections.Generic; using System.Collections; using UnityEngine; +using UnityEngine.LowLevel; -/// Author: Pim de Witte (pimdewitte.com) and contributors -/// -/// A thread-safe class which holds a queue with actions to execute on the next Update() method. It can be used to make calls to the main thread for -/// things such as UI Manipulation in Unity. It was developed for use in combination with the Firebase Unity plugin, which uses separate threads for event handling -/// namespace BugsnagUnity { - public class MainThreadDispatchBehaviour : MonoBehaviour + public class BugsnagCoroutineRunner : MonoBehaviour { + private static BugsnagCoroutineRunner _instance; - private static MainThreadDispatchBehaviour _instance; - - private static readonly Queue _executionQueue = new Queue(); + public static BugsnagCoroutineRunner Instance + { + get + { + if (_instance == null) + { + var runnerObject = new GameObject("BugsnagCoroutineRunner"); + _instance = runnerObject.AddComponent(); + DontDestroyOnLoad(runnerObject); + } + return _instance; + } + } + } + public class MainThreadDispatchBehaviour + { + private static readonly Queue _executionQueue = new Queue(); - public static MainThreadDispatchBehaviour Instance() + [RuntimeInitializeOnLoadMethod(RuntimeInitializeLoadType.BeforeSceneLoad)] + private static void InitializeLoop() { - if (_instance == null) + var playerLoop = PlayerLoop.GetCurrentPlayerLoop(); + var newSystem = new PlayerLoopSystem { - _instance = new GameObject("Bugsnag main thread dispatcher").AddComponent(); - } - return _instance; + updateDelegate = OnUpdate + }; + + var systems = new List(playerLoop.subSystemList); + systems.Insert(0, newSystem); + playerLoop.subSystemList = systems.ToArray(); + PlayerLoop.SetPlayerLoop(playerLoop); } - public void Update() + private static void OnUpdate() { lock (_executionQueue) { @@ -54,50 +55,23 @@ public void Update() } } - /// - /// Locks the queue and adds the IEnumerator to the queue - /// - /// IEnumerator function that will be executed from the main thread. - public void Enqueue(IEnumerator action) + public static void Enqueue(IEnumerator action) { lock (_executionQueue) { _executionQueue.Enqueue(() => { - StartCoroutine(action); + BugsnagCoroutineRunner.Instance.StartCoroutine(action); }); } } - /// - /// Locks the queue and adds the Action to the queue - /// - /// function that will be executed from the main thread. - public void Enqueue(Action action) + public static void Enqueue(Action action) { - Enqueue(ActionWrapper(action)); - } - IEnumerator ActionWrapper(Action a) - { - a(); - yield return null; - } - - public void EnqueueWithDelayCoroutine(Action action, float delay) - { - StartCoroutine(DelayAction(action, delay)); - } - - private IEnumerator DelayAction(Action action, float delay) - { - yield return new WaitForSeconds(delay); - action.Invoke(); - } - - private void Awake() - { - DontDestroyOnLoad(gameObject); + lock (_executionQueue) + { + _executionQueue.Enqueue(action); + } } - } } diff --git a/CHANGELOG.md b/CHANGELOG.md index 75968f69e..e62bb7e0c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## TBD + +### Enhancements + +- Allow the notifier to be started much earlier in the Unity lifecycle. [#862](https://github.com/bugsnag/bugsnag-unity/pull/862) + ## 8.3.1 (2024-12-09) ### Bug Fixes From 7171507fd744b75d8009c410800c625e779a3fff Mon Sep 17 00:00:00 2001 From: Richard Elms Date: Wed, 8 Jan 2025 11:50:50 +0100 Subject: [PATCH 06/10] enable minification support (#867) * enable minification support * only minify on release fixture * stop stripping crashy plugin * line fixes --- bugsnag-android-unity/build.gradle | 1 + bugsnag-android-unity/proguard-rules.pro | 1 + features/android/android_jvm_errors.feature | 4 ++-- .../maze_runner/ProjectSettings/ProjectSettings.asset | 3 +++ .../fixtures/maze_runner/nativeplugin/android/build.gradle | 1 + .../maze_runner/nativeplugin/android/proguard-rules.pro | 1 + 6 files changed, 9 insertions(+), 2 deletions(-) create mode 100644 bugsnag-android-unity/proguard-rules.pro create mode 100644 features/fixtures/maze_runner/nativeplugin/android/proguard-rules.pro diff --git a/bugsnag-android-unity/build.gradle b/bugsnag-android-unity/build.gradle index bcebd874b..66e3d805d 100644 --- a/bugsnag-android-unity/build.gradle +++ b/bugsnag-android-unity/build.gradle @@ -29,6 +29,7 @@ android { } } ndkVersion = "21.4.7075529" + consumerProguardFiles 'proguard-rules.pro' } externalNativeBuild { diff --git a/bugsnag-android-unity/proguard-rules.pro b/bugsnag-android-unity/proguard-rules.pro new file mode 100644 index 000000000..4ebeecf27 --- /dev/null +++ b/bugsnag-android-unity/proguard-rules.pro @@ -0,0 +1 @@ +-keep class com.bugsnag.android.** {*;} \ No newline at end of file diff --git a/features/android/android_jvm_errors.feature b/features/android/android_jvm_errors.feature index 53bd8517e..ab367f75c 100644 --- a/features/android/android_jvm_errors.feature +++ b/features/android/android_jvm_errors.feature @@ -30,7 +30,7 @@ Feature: Android JVM Exceptions And the error payload field "events.0.exceptions.0.stacktrace" is a non-empty array And the event "exceptions.0.stacktrace.0.method" equals "com.example.bugsnagcrashplugin.CrashHelper.triggerJvmException()" And the exception "stacktrace.0.file" equals "CrashHelper.java" - And the event "exceptions.0.stacktrace.0.lineNumber" equals 13 + And the event "exceptions.0.stacktrace.0.lineNumber" equals 20 And the error payload field "events.0.threads" is null Scenario: Android JVM Background Thread Smoke Test @@ -62,7 +62,7 @@ Feature: Android JVM Exceptions # Stacktrace validation And the error payload field "events.0.exceptions.0.stacktrace" is a non-empty array And the exception "stacktrace.0.file" equals "CrashHelper.java" - And the event "exceptions.0.stacktrace.0.lineNumber" equals 19 + And the event "exceptions.0.stacktrace.0.lineNumber" equals 5 And the error payload field "events.0.threads" is not null And the error payload field "events.0.usage.config" is not null diff --git a/features/fixtures/maze_runner/ProjectSettings/ProjectSettings.asset b/features/fixtures/maze_runner/ProjectSettings/ProjectSettings.asset index 13a273ae4..b16b9dd89 100644 --- a/features/fixtures/maze_runner/ProjectSettings/ProjectSettings.asset +++ b/features/fixtures/maze_runner/ProjectSettings/ProjectSettings.asset @@ -269,6 +269,9 @@ PlayerSettings: banner: {fileID: 0} androidGamepadSupportLevel: 0 resolutionDialogBanner: {fileID: 0} + AndroidMinifyWithR8: 1 + AndroidMinifyRelease: 1 + AndroidMinifyDebug: 0 m_BuildTargetIcons: [] m_BuildTargetPlatformIcons: - m_BuildTarget: Android diff --git a/features/fixtures/maze_runner/nativeplugin/android/build.gradle b/features/fixtures/maze_runner/nativeplugin/android/build.gradle index 6c5ab09b8..9c245dd0f 100644 --- a/features/fixtures/maze_runner/nativeplugin/android/build.gradle +++ b/features/fixtures/maze_runner/nativeplugin/android/build.gradle @@ -22,6 +22,7 @@ android { minSdkVersion 14 targetSdkVersion 30 ndkVersion = "21.4.7075529" + consumerProguardFiles 'proguard-rules.pro' } externalNativeBuild.cmake.path "CMakeLists.txt" } diff --git a/features/fixtures/maze_runner/nativeplugin/android/proguard-rules.pro b/features/fixtures/maze_runner/nativeplugin/android/proguard-rules.pro new file mode 100644 index 000000000..d8ca2a940 --- /dev/null +++ b/features/fixtures/maze_runner/nativeplugin/android/proguard-rules.pro @@ -0,0 +1 @@ +-keep class com.example.bugsnagcrashplugin.** { *; } \ No newline at end of file From 03961c58b340812a172d70b38c02d3755a92314b Mon Sep 17 00:00:00 2001 From: Richard Elms Date: Thu, 9 Jan 2025 11:42:06 +0100 Subject: [PATCH 07/10] fix for incorrect session handled/unhandled event counts (#865) --- Bugsnag/Assets/Bugsnag/Runtime/Client.cs | 9 ++- Bugsnag/Assets/Bugsnag/Runtime/Delivery.cs | 20 ++++-- .../Assets/Bugsnag/Runtime/Payload/Event.cs | 61 +++++++++++-------- .../Bugsnag/Runtime/Payload/HandledState.cs | 7 +++ .../Assets/Bugsnag/Runtime/Payload/Report.cs | 2 +- .../Assets/Bugsnag/Runtime/Payload/Session.cs | 38 ++++++------ CHANGELOG.md | 4 ++ features/csharp/csharp_callbacks.feature | 33 +++++++++- features/csharp/csharp_events.feature | 2 + .../maze_runner/Assets/Scenes/MainScene.unity | 22 ++++++- .../maze_runner/Assets/Scripts/Scenario.cs | 1 - .../Csharp/Callbacks/EditUnhandled.cs | 56 +++++++++++++++++ .../Csharp/Callbacks/EditUnhandled.cs.meta | 11 ++++ 13 files changed, 212 insertions(+), 54 deletions(-) create mode 100644 features/fixtures/maze_runner/Assets/Scripts/Scenarios/Csharp/Callbacks/EditUnhandled.cs create mode 100644 features/fixtures/maze_runner/Assets/Scripts/Scenarios/Csharp/Callbacks/EditUnhandled.cs.meta diff --git a/Bugsnag/Assets/Bugsnag/Runtime/Client.cs b/Bugsnag/Assets/Bugsnag/Runtime/Client.cs index d84486303..c5f8e3f3d 100644 --- a/Bugsnag/Assets/Bugsnag/Runtime/Client.cs +++ b/Bugsnag/Assets/Bugsnag/Runtime/Client.cs @@ -452,6 +452,9 @@ private void NotifyOnMainThread(Error[] exceptions, HandledState handledState, F @event.AddAndroidProjectPackagesToEvent(Configuration.ProjectPackages); } + // save handled state before callbacks so we can check later if it was overidden + var initialUnhandledState = @event.Unhandled; + lock (CallbackLock) { foreach (var onErrorCallback in Configuration.GetOnErrorCallbacks()) @@ -485,6 +488,11 @@ private void NotifyOnMainThread(Error[] exceptions, HandledState handledState, F // If the callback causes an exception, ignore it and execute the next one } + if (initialUnhandledState != @event.Unhandled) + { + @event.UnhandledOverridden(); + } + var report = new Report(Configuration, @event); if (!report.Ignored) { @@ -501,7 +509,6 @@ private void NotifyOnMainThread(Error[] exceptions, HandledState handledState, F } } - private bool ShouldAddProjectPackagesToEvent(Payload.Event theEvent) { return Application.platform.Equals(RuntimePlatform.Android) diff --git a/Bugsnag/Assets/Bugsnag/Runtime/Delivery.cs b/Bugsnag/Assets/Bugsnag/Runtime/Delivery.cs index 644c038b2..735970768 100644 --- a/Bugsnag/Assets/Bugsnag/Runtime/Delivery.cs +++ b/Bugsnag/Assets/Bugsnag/Runtime/Delivery.cs @@ -67,6 +67,8 @@ public void Deliver(IPayload payload) var report = (Report)payload; if (_configuration.GetOnSendErrorCallbacks().Count > 0) { + // save handled state before callbacks so we can check later if it was overidden + var initialUnhandledState = report.Event.Unhandled; lock (_callbackLock) { foreach (var onSendErrorCallback in _configuration.GetOnSendErrorCallbacks()) @@ -84,6 +86,10 @@ public void Deliver(IPayload payload) } } } + if (initialUnhandledState != report.Event.Unhandled) + { + report.Event.UnhandledOverridden(); + } } report.Event.RedactMetadata(_configuration); //pipeline expects and array of events even though we only ever report 1 @@ -110,7 +116,8 @@ IEnumerator PushToServer(IPayload payload) else { var networkCheckDone = false; - new Thread(() => { + new Thread(() => + { shouldDeliver = _client.NativeClient.ShouldAttemptDelivery(); networkCheckDone = true; }).Start(); @@ -158,7 +165,8 @@ IEnumerator PushToServer(IPayload payload) else { var bodyReady = false; - new Thread(() => { + new Thread(() => + { try { body = PrepareEventBody(payload); @@ -210,7 +218,7 @@ IEnumerator PushToServer(IPayload payload) _payloadManager.RemovePayload(payload); StartDeliveringCachedPayloads(); } - else if ( code == 0 || code == 408 || code == 429 || code >= 500) + else if (code == 0 || code == 408 || code == 429 || code >= 500) { // sending failed with no network or retryable error, cache payload to disk _payloadManager.SendPayloadFailed(payload); @@ -250,7 +258,7 @@ private byte[] PrepareEventBody(IPayload payload) } } } - + return serialisedPayload; } @@ -277,7 +285,7 @@ private byte[] PrepareEventBodySimple(IPayload payload) } } } - + return serialisedPayload; } @@ -587,7 +595,7 @@ private IEnumerator ProcessCachedItems(Type t) } Deliver(report); } - + yield return new WaitUntil(() => CachedPayloadProcessed(id)); } } diff --git a/Bugsnag/Assets/Bugsnag/Runtime/Payload/Event.cs b/Bugsnag/Assets/Bugsnag/Runtime/Payload/Event.cs index ccad20d28..622f11aae 100644 --- a/Bugsnag/Assets/Bugsnag/Runtime/Payload/Event.cs +++ b/Bugsnag/Assets/Bugsnag/Runtime/Payload/Event.cs @@ -36,16 +36,15 @@ internal Event(string context, Metadata metadata, AppWithState app, DeviceWithSt breadcrumbsList.Add(crumb); } Breadcrumbs = new ReadOnlyCollection(breadcrumbsList); - if (session != null) { - if (handledState.Handled) + if(handledState.Handled) { - session.Events.IncrementHandledCount(); + session.Events.UpdateHandledCount(true); } else { - session.Events.IncrementUnhandledCount(); + session.Events.UpdateUnhandledCount(true); } Session = session; } @@ -53,7 +52,7 @@ internal Event(string context, Metadata metadata, AppWithState app, DeviceWithSt } internal Event(Dictionary serialisedPayload) - { + { ApiKey = serialisedPayload["apiKey"].ToString(); var eventObject = (Dictionary)serialisedPayload["event"]; @@ -66,7 +65,7 @@ internal Event(Dictionary serialisedPayload) { Add("unhandled", false); } - + if (eventObject["severity"] != null) { Add("severity", eventObject["severity"]); @@ -161,7 +160,6 @@ internal void AddAndroidProjectPackagesToEvent(string[] packages) { _androidProjectPackages = packages; } - private OrderedDictionary _featureFlags; HandledState _handledState; @@ -172,13 +170,13 @@ internal void AddAndroidProjectPackagesToEvent(string[] packages) private Metadata _metadata { get; } - public void AddMetadata(string section, string key, object value) => _metadata.AddMetadata(section,key,value); + public void AddMetadata(string section, string key, object value) => _metadata.AddMetadata(section, key, value); public void AddMetadata(string section, IDictionary metadata) => _metadata.AddMetadata(section, metadata); public IDictionary GetMetadata(string section) => _metadata.GetMetadata(section); - public object GetMetadata(string section, string key) => _metadata.GetMetadata(section,key); + public object GetMetadata(string section, string key) => _metadata.GetMetadata(section, key); public void ClearMetadata(string section) => _metadata.ClearMetadata(section); @@ -198,29 +196,40 @@ internal void AddAndroidProjectPackagesToEvent(string[] packages) internal LogType? LogType { get; } - public bool Unhandled { - get { - var currentValue = Get("unhandled"); - if (currentValue == null) + public bool Unhandled + { + get + { + var currentValue = Get("unhandled"); + if (currentValue == null) + { + return false; + } + return (bool)currentValue; + } + set { - return false; + Add("unhandled", value); } - return (bool)currentValue; - } - set => Add("unhandled",value); } - internal bool IsHandled + internal void UnhandledOverridden() { - get + if (Session != null) { - if (Get("unhandled") is bool unhandled) + var events = Session.Events; + if (Unhandled) { - return !unhandled; + events.UpdateUnhandledCount(true); + events.UpdateHandledCount(false); + } + else + { + events.UpdateUnhandledCount(false); + events.UpdateHandledCount(true); } - - return false; } + _handledState.UnhandledOverridden(); } public string Context { get; set; } @@ -238,7 +247,7 @@ internal bool IsHandled public List Errors { get; } public string GroupingHash { get; set; } - + public Severity Severity { set => HandledState = HandledState.ForCallbackSpecifiedSeverity(value, _handledState); @@ -253,7 +262,7 @@ public Severity Severity public void SetUser(string id, string email, string name) { - _user = new User(id, email, name ); + _user = new User(id, email, name); } internal bool IsAndroidJavaError() @@ -317,7 +326,7 @@ internal Dictionary GetEventPayload() Add("groupingHash", GroupingHash); Add("payloadVersion", PayloadVersion); Add("exceptions", _errors); - if(Correlation != null) + if (Correlation != null) { Add("correlation", Correlation.Payload); } diff --git a/Bugsnag/Assets/Bugsnag/Runtime/Payload/HandledState.cs b/Bugsnag/Assets/Bugsnag/Runtime/Payload/HandledState.cs index 818ac2ee9..9b7aa52bf 100644 --- a/Bugsnag/Assets/Bugsnag/Runtime/Payload/HandledState.cs +++ b/Bugsnag/Assets/Bugsnag/Runtime/Payload/HandledState.cs @@ -9,6 +9,12 @@ namespace BugsnagUnity.Payload /// class HandledState : Dictionary { + + public void UnhandledOverridden() + { + var severityReason = this["severityReason"] as SeverityReason; + severityReason["unhandledOverridden"] = true; + } /// /// Creates a HandledState object for an error report payload where the exception was not handled by the application /// and caught by a global error handler. @@ -145,6 +151,7 @@ internal static SeverityReason ForCallbackSpecifiedSeverity() { this.AddToPayload("type", type); this.AddToPayload("attributes", attributes); + this.AddToPayload("unhandledOverridden",false); } } } diff --git a/Bugsnag/Assets/Bugsnag/Runtime/Payload/Report.cs b/Bugsnag/Assets/Bugsnag/Runtime/Payload/Report.cs index afe638783..68d5e9203 100644 --- a/Bugsnag/Assets/Bugsnag/Runtime/Payload/Report.cs +++ b/Bugsnag/Assets/Bugsnag/Runtime/Payload/Report.cs @@ -64,7 +64,7 @@ internal void ApplyEventsArray() internal Session Session => Event.Session; - internal bool IsHandled => Event.IsHandled; + internal bool IsHandled => !Event.Unhandled; internal string Context => Event.Context; diff --git a/Bugsnag/Assets/Bugsnag/Runtime/Payload/Session.cs b/Bugsnag/Assets/Bugsnag/Runtime/Payload/Session.cs index 4c3317cc9..5a7c44963 100644 --- a/Bugsnag/Assets/Bugsnag/Runtime/Payload/Session.cs +++ b/Bugsnag/Assets/Bugsnag/Runtime/Payload/Session.cs @@ -74,13 +74,13 @@ internal Session(string providedGuid, DateTimeOffset startedAt, int handled, int internal void AddException(Report report) { - if (report.IsHandled) + if(report.IsHandled) { - Events.IncrementHandledCount(); + Events.UpdateHandledCount(true); } else { - Events.IncrementUnhandledCount(); + Events.UpdateUnhandledCount(true); } } @@ -96,32 +96,36 @@ internal Session Copy() internal class SessionEvents : Dictionary { - private readonly object _handledLock = new object(); - private readonly object _unhandledLock = new object(); + private readonly object _lock = new object(); + + private const string HANDLED_KEY = "handled"; + private const string UNHANDLED_KEY = "unhandled"; internal SessionEvents(int handled, int unhandled) { - this.AddToPayload("handled", handled); - this.AddToPayload("unhandled", unhandled); + this.AddToPayload(HANDLED_KEY, handled); + this.AddToPayload(UNHANDLED_KEY, unhandled); } - internal int Handled => this["handled"]; - internal int Unhandled => this["unhandled"]; + internal int Handled => this[HANDLED_KEY]; + internal int Unhandled => this[UNHANDLED_KEY]; - internal void IncrementHandledCount() + private void UpdateEventCount(bool handled, bool increment) { - lock (_handledLock) + lock (_lock) { - this["handled"]++; + this[handled ? HANDLED_KEY : UNHANDLED_KEY] += increment ? 1 : -1; } } - internal void IncrementUnhandledCount() + internal void UpdateUnhandledCount(bool increment) { - lock (_unhandledLock) - { - this["unhandled"]++; - } + UpdateEventCount(false, increment); + } + + internal void UpdateHandledCount(bool increment) + { + UpdateEventCount(true, increment); } } } diff --git a/CHANGELOG.md b/CHANGELOG.md index e62bb7e0c..540578ac2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ - Allow the notifier to be started much earlier in the Unity lifecycle. [#862](https://github.com/bugsnag/bugsnag-unity/pull/862) +### Bug Fixes + +- Fixed an issue where session handled/unhandled event counts were not updated if the handled status of the event was changed in a callback. [#865](https://github.com/bugsnag/bugsnag-unity/pull/865) + ## 8.3.1 (2024-12-09) ### Bug Fixes diff --git a/features/csharp/csharp_callbacks.feature b/features/csharp/csharp_callbacks.feature index f584d19c9..800e29e86 100644 --- a/features/csharp/csharp_callbacks.feature +++ b/features/csharp/csharp_callbacks.feature @@ -7,10 +7,10 @@ Feature: Callbacks When I run the game in the "OnErrorInConfig" state And I wait to receive 2 errors And I sort the errors by the payload field "events.0.exceptions.0.message" - Then the error is valid for the error reporting API sent by the Unity notifier And the exception "message" equals "Error 1" And all possible parameters have been edited in a callback + And I discard the oldest error Then the error is valid for the error reporting API sent by the Unity notifier @@ -62,3 +62,34 @@ Feature: Callbacks And I wait to receive 1 session And all possible parameters have been edited in a session callback + Scenario: Edit handled state in callbacks + When I run the game in the "EditUnhandled" state + And I wait to receive 4 errors + + # Error 1 + And the exception "message" equals "Control" + And the event "unhandled" is false + And the event "severityReason.unhandledOverridden" is false + And the event "session.events.unhandled" equals 0 + And the event "session.events.handled" equals 1 + And I discard the oldest error + + And the exception "message" equals "HandledInNotifyCallback" + And the event "unhandled" is true + And the event "severityReason.unhandledOverridden" is true + And the event "session.events.unhandled" equals 1 + And the event "session.events.handled" equals 1 + And I discard the oldest error + + And the exception "message" equals "HandledInOnSendCallback" + And the event "unhandled" is true + And the event "severityReason.unhandledOverridden" is true + And the event "session.events.unhandled" equals 2 + And the event "session.events.handled" equals 1 + And I discard the oldest error + + And the exception "message" equals "UnhandledInOnErrorCallback" + And the event "unhandled" is false + And the event "severityReason.unhandledOverridden" is true + And the event "session.events.unhandled" equals 2 + And the event "session.events.handled" equals 2 diff --git a/features/csharp/csharp_events.feature b/features/csharp/csharp_events.feature index 598eeed9f..81ac5c30b 100644 --- a/features/csharp/csharp_events.feature +++ b/features/csharp/csharp_events.feature @@ -15,6 +15,8 @@ Feature: csharp events | NotifySmokeTest.Run() | Main+d__8.MoveNext() | And expected device metadata is included in the event And expected app metadata is included in the event + And the error payload field "events.0.severityReason.unhandledOverridden" is false + Scenario: Uncaught Exception smoke test When I run the game in the "UncaughtExceptionSmokeTest" state diff --git a/features/fixtures/maze_runner/Assets/Scenes/MainScene.unity b/features/fixtures/maze_runner/Assets/Scenes/MainScene.unity index c18310998..92726f1bb 100644 --- a/features/fixtures/maze_runner/Assets/Scenes/MainScene.unity +++ b/features/fixtures/maze_runner/Assets/Scenes/MainScene.unity @@ -38,7 +38,6 @@ RenderSettings: m_ReflectionIntensity: 1 m_CustomReflection: {fileID: 0} m_Sun: {fileID: 0} - m_IndirectSpecularColor: {r: 0.37311932, g: 0.38073996, b: 0.3587271, a: 1} m_UseRadianceAmbientProbe: 0 --- !u!157 &3 LightmapSettings: @@ -1146,6 +1145,7 @@ GameObject: - component: {fileID: 1160627406} - component: {fileID: 1160627409} - component: {fileID: 1160627407} + - component: {fileID: 1160627410} m_Layer: 0 m_Name: Callbacks m_TagString: Untagged @@ -1264,6 +1264,18 @@ MonoBehaviour: Main.CUSTOM2 () (at Assets/Scripts/Main.cs:123)' +--- !u!114 &1160627410 +MonoBehaviour: + m_ObjectHideFlags: 0 + m_CorrespondingSourceObject: {fileID: 0} + m_PrefabInstance: {fileID: 0} + m_PrefabAsset: {fileID: 0} + m_GameObject: {fileID: 1160627402} + m_Enabled: 1 + m_EditorHideFlags: 0 + m_Script: {fileID: 11500000, guid: df293b62197504f21a7d582167548183, type: 3} + m_Name: + m_EditorClassIdentifier: --- !u!1 &1338701864 GameObject: m_ObjectHideFlags: 0 @@ -1394,6 +1406,10 @@ MonoBehaviour: m_Script: {fileID: 11500000, guid: 3cbe76e3ceae047ee85769786e2840dd, type: 3} m_Name: m_EditorClassIdentifier: + CustomStacktrace: 'Main.CUSTOM1 () (at Assets/Scripts/Main.cs:123) + + Main.CUSTOM2 + () (at Assets/Scripts/Main.cs:123)' --- !u!114 &1338701872 MonoBehaviour: m_ObjectHideFlags: 0 @@ -1406,6 +1422,10 @@ MonoBehaviour: m_Script: {fileID: 11500000, guid: 36160400d18884e48983505794649646, type: 3} m_Name: m_EditorClassIdentifier: + CustomStacktrace: 'Main.CUSTOM1 () (at Assets/Scripts/Main.cs:123) + + Main.CUSTOM2 + () (at Assets/Scripts/Main.cs:123)' --- !u!1 &1388241496 GameObject: m_ObjectHideFlags: 0 diff --git a/features/fixtures/maze_runner/Assets/Scripts/Scenario.cs b/features/fixtures/maze_runner/Assets/Scripts/Scenario.cs index dd7686691..21d0f7fb8 100644 --- a/features/fixtures/maze_runner/Assets/Scripts/Scenario.cs +++ b/features/fixtures/maze_runner/Assets/Scripts/Scenario.cs @@ -157,7 +157,6 @@ public bool SimpleEventCallback(IEvent @event) @event.AddMetadata("test1", new Dictionary { { "test", "test" } }); @event.AddMetadata("test2", new Dictionary { { "test", "test" } }); @event.ClearMetadata("test2"); - @event.AddFeatureFlag("fromCallback", "a"); return true; diff --git a/features/fixtures/maze_runner/Assets/Scripts/Scenarios/Csharp/Callbacks/EditUnhandled.cs b/features/fixtures/maze_runner/Assets/Scripts/Scenarios/Csharp/Callbacks/EditUnhandled.cs new file mode 100644 index 000000000..19218d6a4 --- /dev/null +++ b/features/fixtures/maze_runner/Assets/Scripts/Scenarios/Csharp/Callbacks/EditUnhandled.cs @@ -0,0 +1,56 @@ +using System; +using System.Collections; +using BugsnagUnity; +using UnityEngine; + +public class EditUnhandled : Scenario +{ + public override void PrepareConfig(string apiKey, string host) + { + base.PrepareConfig(apiKey, host); + Configuration.AddOnError(SimpleEventCallback); + Configuration.ReportExceptionLogsAsHandled = false; + Configuration.AddOnError(OnErrorCallback); + Configuration.AddOnSendError(OnSendCallback); + } + + public override void Run() + { + Bugsnag.StartSession(); + StartCoroutine(DoTest()); + } + + private IEnumerator DoTest() + { + Bugsnag.Notify(new Exception("Control")); + yield return new WaitForSeconds(1); + Bugsnag.Notify(new Exception("HandledInNotifyCallback"), (report) => + { + report.Unhandled = true; + return true; + }); + yield return new WaitForSeconds(1); + Bugsnag.Notify(new Exception("HandledInOnSendCallback")); + yield return new WaitForSeconds(1); + throw new Exception("UnhandledInOnErrorCallback"); + } + + private bool OnErrorCallback(IEvent @event) + { + if (@event.Errors[0].ErrorMessage == "UnhandledInOnErrorCallback") + { + @event.Unhandled = false; + return true; + } + return true; + } + private bool OnSendCallback(IEvent @event) + { + if (@event.Errors[0].ErrorMessage == "HandledInOnSendCallback") + { + @event.Unhandled = true; + return true; + } + return true; + } +} diff --git a/features/fixtures/maze_runner/Assets/Scripts/Scenarios/Csharp/Callbacks/EditUnhandled.cs.meta b/features/fixtures/maze_runner/Assets/Scripts/Scenarios/Csharp/Callbacks/EditUnhandled.cs.meta new file mode 100644 index 000000000..bc7e13375 --- /dev/null +++ b/features/fixtures/maze_runner/Assets/Scripts/Scenarios/Csharp/Callbacks/EditUnhandled.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: df293b62197504f21a7d582167548183 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: From 71350dd0fe3672bdcb0ea7261e51476c47cfce57 Mon Sep 17 00:00:00 2001 From: Richard Elms Date: Thu, 9 Jan 2025 16:35:56 +0100 Subject: [PATCH 08/10] Android and iOS CI flake fixes (#868) * some CI fixes [full ci] * missing env var [full ci] * simpler check [full ci] * skip flakey tests until later [full ci] * typo [full ci] * disable native frames until we fix unity 6 [full ci] --- .buildkite/unity.2020.yml | 2 + .../Runtime/Native/Cocoa/NativeClient.cs | 123 +++++++++--------- features/android/android_jvm_errors.feature | 12 +- features/csharp/csharp_events.feature | 15 ++- features/steps/unity_steps.rb | 5 + features/support/env.rb | 6 +- 6 files changed, 88 insertions(+), 75 deletions(-) diff --git a/.buildkite/unity.2020.yml b/.buildkite/unity.2020.yml index f67f532c6..d69060fc8 100644 --- a/.buildkite/unity.2020.yml +++ b/.buildkite/unity.2020.yml @@ -183,6 +183,8 @@ steps: depends_on: "build-ios-fixture-2020" agents: queue: opensource + env: + UNITY_VERSION: *2020 plugins: artifacts#v1.9.0: download: diff --git a/Bugsnag/Assets/Bugsnag/Runtime/Native/Cocoa/NativeClient.cs b/Bugsnag/Assets/Bugsnag/Runtime/Native/Cocoa/NativeClient.cs index 5f8f5031d..ffb0ef97d 100644 --- a/Bugsnag/Assets/Bugsnag/Runtime/Native/Cocoa/NativeClient.cs +++ b/Bugsnag/Assets/Bugsnag/Runtime/Native/Cocoa/NativeClient.cs @@ -540,68 +540,69 @@ private StackTraceLine[] ToStackFrames(System.Exception exception, IntPtr[] nati public StackTraceLine[] ToStackFrames(System.Exception exception) { var notFound = new StackTraceLine[0]; - - if (exception == null) - { - return notFound; - } - -#if ENABLE_IL2CPP && UNITY_2021_3_OR_NEWER - var hException = GCHandle.Alloc(exception); - var pNativeAddresses = IntPtr.Zero; - var pImageUuid = IntPtr.Zero; - var pImageName = IntPtr.Zero; - try - { - if (hException == null) - { - return notFound; - } - - var pException = il2cpp_gchandle_get_target(GCHandle.ToIntPtr(hException).ToInt32()); - if (pException == IntPtr.Zero) - { - return notFound; - } - - var frameCount = 0; - string? mainImageFileName = null; - - il2cpp_native_stack_trace(pException, out pNativeAddresses, out frameCount, out pImageUuid, out pImageName); - if (pNativeAddresses == IntPtr.Zero) - { - return notFound; - } - - mainImageFileName = ExtractString(pImageName); - var nativeAddresses = new IntPtr[frameCount]; - Marshal.Copy(pNativeAddresses, nativeAddresses, 0, frameCount); - - loadedImages.Refresh(mainImageFileName); - return ToStackFrames(exception, nativeAddresses); - } - finally - { - if (pImageUuid != IntPtr.Zero) - { - il2cpp_free(pImageUuid); - } - if (pImageName != IntPtr.Zero) - { - il2cpp_free(pImageName); - } - if (pNativeAddresses != IntPtr.Zero) - { - il2cpp_free(pNativeAddresses); - } - if (hException != null) - { - hException.Free(); - } - } -#else return notFound; -#endif +// Disabled until we can get this working with IL2CPP and Unity 6. raised in PLAT-13394 +// if (exception == null) +// { +// return notFound; +// } + +// #if ENABLE_IL2CPP && UNITY_2021_3_OR_NEWER +// var hException = GCHandle.Alloc(exception); +// var pNativeAddresses = IntPtr.Zero; +// var pImageUuid = IntPtr.Zero; +// var pImageName = IntPtr.Zero; +// try +// { +// if (hException == null) +// { +// return notFound; +// } + +// var pException = il2cpp_gchandle_get_target(GCHandle.ToIntPtr(hException).ToInt32()); +// if (pException == IntPtr.Zero) +// { +// return notFound; +// } + +// var frameCount = 0; +// string? mainImageFileName = null; + +// il2cpp_native_stack_trace(pException, out pNativeAddresses, out frameCount, out pImageUuid, out pImageName); +// if (pNativeAddresses == IntPtr.Zero) +// { +// return notFound; +// } + +// mainImageFileName = ExtractString(pImageName); +// var nativeAddresses = new IntPtr[frameCount]; +// Marshal.Copy(pNativeAddresses, nativeAddresses, 0, frameCount); + +// loadedImages.Refresh(mainImageFileName); +// return ToStackFrames(exception, nativeAddresses); +// } +// finally +// { +// if (pImageUuid != IntPtr.Zero) +// { +// il2cpp_free(pImageUuid); +// } +// if (pImageName != IntPtr.Zero) +// { +// il2cpp_free(pImageName); +// } +// if (pNativeAddresses != IntPtr.Zero) +// { +// il2cpp_free(pNativeAddresses); +// } +// if (hException != null) +// { +// hException.Free(); +// } +// } +// #else +// return notFound; +// #endif } } } diff --git a/features/android/android_jvm_errors.feature b/features/android/android_jvm_errors.feature index ab367f75c..34b990ae2 100644 --- a/features/android/android_jvm_errors.feature +++ b/features/android/android_jvm_errors.feature @@ -29,8 +29,10 @@ Feature: Android JVM Exceptions # Stacktrace validation And the error payload field "events.0.exceptions.0.stacktrace" is a non-empty array And the event "exceptions.0.stacktrace.0.method" equals "com.example.bugsnagcrashplugin.CrashHelper.triggerJvmException()" - And the exception "stacktrace.0.file" equals "CrashHelper.java" - And the event "exceptions.0.stacktrace.0.lineNumber" equals 20 + And the exception "stacktrace.0.file" equals one of: + | CrashHelper.java | + | SourceFile | + And the error payload field "events.0.exceptions.0.stacktrace.0.lineNumber" is a number And the error payload field "events.0.threads" is null Scenario: Android JVM Background Thread Smoke Test @@ -61,8 +63,10 @@ Feature: Android JVM Exceptions # Stacktrace validation And the error payload field "events.0.exceptions.0.stacktrace" is a non-empty array - And the exception "stacktrace.0.file" equals "CrashHelper.java" - And the event "exceptions.0.stacktrace.0.lineNumber" equals 5 + And the exception "stacktrace.0.file" equals one of: + | CrashHelper.java | + | SourceFile | + And the error payload field "events.0.exceptions.0.stacktrace.0.lineNumber" is a number And the error payload field "events.0.threads" is not null And the error payload field "events.0.usage.config" is not null diff --git a/features/csharp/csharp_events.feature b/features/csharp/csharp_events.feature index 81ac5c30b..d59918e0d 100644 --- a/features/csharp/csharp_events.feature +++ b/features/csharp/csharp_events.feature @@ -33,8 +33,8 @@ Feature: csharp events And expected app metadata is included in the event @ios_only - @skip_before_unity_2021 - Scenario: Uncaught Exception ios smoke test + @skip_unity_2020 + Scenario: Uncaught Exception ios smoke test with more frame information When I run the game in the "UncaughtExceptionSmokeTest" state And I wait to receive an error Then the error is valid for the error reporting API sent by the Unity notifier @@ -44,12 +44,13 @@ Feature: csharp events And custom metadata is included in the event And expected device metadata is included in the event And expected app metadata is included in the event - And the error payload field "events.0.exceptions.0.stacktrace.0.frameAddress" matches the regex "\d+" + # some skipped steps pending: PLAT-13392 + #And the error payload field "events.0.exceptions.0.stacktrace.0.frameAddress" matches the regex "\d+" And the error payload field "events.0.exceptions.0.stacktrace.0.method" equals "UncaughtExceptionSmokeTest.Run()" - And the error payload field "events.0.exceptions.0.stacktrace.0.machoFile" matches the regex ".*/UnityFramework.framework/UnityFramework" - And the error payload field "events.0.exceptions.0.stacktrace.0.machoLoadAddress" matches the regex "\d+" - And the error payload field "events.0.exceptions.0.stacktrace.0.machoUUID" matches the regex "[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}" - And the error payload field "events.0.exceptions.0.stacktrace.0.inProject" is true + #And the error payload field "events.0.exceptions.0.stacktrace.0.machoFile" matches the regex ".*/UnityFramework.framework/UnityFramework" + #And the error payload field "events.0.exceptions.0.stacktrace.0.machoLoadAddress" matches the regex "\d+" + #And the error payload field "events.0.exceptions.0.stacktrace.0.machoUUID" matches the regex "[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}" + #And the error payload field "events.0.exceptions.0.stacktrace.0.inProject" is true Scenario: Debug Log Exception smoke test When I run the game in the "DebugLogExceptionSmokeTest" state diff --git a/features/steps/unity_steps.rb b/features/steps/unity_steps.rb index c8d8b87ba..45b7bb6d5 100644 --- a/features/steps/unity_steps.rb +++ b/features/steps/unity_steps.rb @@ -362,3 +362,8 @@ def switch_run_on_target # Check if the integer event value is one of the expected values Maze.check.true(expected_values.include?(event_value), "Expected one of #{expected_values} but got #{event_value}") end + +Then("the exception {string} equals one of:") do |keypath, possible_values| + value = Maze::Helper.read_key_path(Maze::Server.errors.current[:body], "events.0.exceptions.0.#{keypath}") + Maze.check.include(possible_values.raw.flatten, value) +end diff --git a/features/support/env.rb b/features/support/env.rb index 4a0769e64..ded754708 100644 --- a/features/support/env.rb +++ b/features/support/env.rb @@ -9,11 +9,11 @@ end end -Before('@skip_before_unity_2021') do |_scenario| +Before('@skip_unity_2020') do |_scenario| if ENV['UNITY_VERSION'] unity_version = ENV['UNITY_VERSION'][0..3].to_i - if unity_version < 2021 - skip_this_scenario('Skipping scenario on Unity < 2021') + if unity_version == 2020 + skip_this_scenario('Skipping scenario on Unity 2020') end end end From 86f4bc42b8c560443fd1c0d8f6f840cd1bd207f0 Mon Sep 17 00:00:00 2001 From: richard elms Date: Thu, 9 Jan 2025 16:39:10 +0100 Subject: [PATCH 09/10] Release v8.4.0 --- Bugsnag/Assets/Bugsnag/Runtime/AssemblyInfo.cs | 2 +- CHANGELOG.md | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Bugsnag/Assets/Bugsnag/Runtime/AssemblyInfo.cs b/Bugsnag/Assets/Bugsnag/Runtime/AssemblyInfo.cs index a52f50240..e427edf37 100644 --- a/Bugsnag/Assets/Bugsnag/Runtime/AssemblyInfo.cs +++ b/Bugsnag/Assets/Bugsnag/Runtime/AssemblyInfo.cs @@ -1,2 +1,2 @@ using System.Reflection; -[assembly: AssemblyVersion("8.3.1.0")] \ No newline at end of file +[assembly: AssemblyVersion("8.4.0.0")] \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index 540578ac2..7de0f5f58 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # Changelog -## TBD +## 8.4.0 (2024-12-10) ### Enhancements @@ -8,7 +8,7 @@ ### Bug Fixes -- Fixed an issue where session handled/unhandled event counts were not updated if the handled status of the event was changed in a callback. [#865](https://github.com/bugsnag/bugsnag-unity/pull/865) +- Fixed an issue where session handled/unhandled event counts were not updated if the handled status of the event was changed in a callback and send unhandledOveridden in the error payload [#865](https://github.com/bugsnag/bugsnag-unity/pull/865) ## 8.3.1 (2024-12-09) From d3b317e95d4609aca6a5cee39e54361cdb95e10a Mon Sep 17 00:00:00 2001 From: richard elms Date: Fri, 10 Jan 2025 09:43:39 +0100 Subject: [PATCH 10/10] date fix --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7de0f5f58..22f40d26e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # Changelog -## 8.4.0 (2024-12-10) +## 8.4.0 (2025-12-10) ### Enhancements