diff --git a/Bugsnag/Assets/Bugsnag/Runtime/Client.cs b/Bugsnag/Assets/Bugsnag/Runtime/Client.cs index c751840e6..7c2dcb8cf 100644 --- a/Bugsnag/Assets/Bugsnag/Runtime/Client.cs +++ b/Bugsnag/Assets/Bugsnag/Runtime/Client.cs @@ -465,7 +465,7 @@ private void NotifyOnMainThread(Error[] exceptions, HandledState handledState, F { try { - if (!onErrorCallback.Invoke(@event)) + if (!RunEventCallback(onErrorCallback, @event)) { return; } @@ -481,7 +481,7 @@ private void NotifyOnMainThread(Error[] exceptions, HandledState handledState, F { if (callback != null) { - if (!callback.Invoke(@event)) + if (!RunEventCallback(callback, @event)) { return; } @@ -508,6 +508,24 @@ private void NotifyOnMainThread(Error[] exceptions, HandledState handledState, F } } + private bool RunEventCallback(Func callback, IEvent @event) + { + try + { + var initialUnhandledState = @event.Unhandled; + var callbackResult = callback.Invoke(@event); + if (initialUnhandledState != @event.Unhandled) + { + ((Payload.Event)@event).UnhandledOverridden(); + } + return callbackResult; + } + catch + { + return true; + } + } + private bool ShouldAddProjectPackagesToEvent(Payload.Event theEvent) { diff --git a/Bugsnag/Assets/Bugsnag/Runtime/Delivery.cs b/Bugsnag/Assets/Bugsnag/Runtime/Delivery.cs index 55b8601fd..17216ad9d 100644 --- a/Bugsnag/Assets/Bugsnag/Runtime/Delivery.cs +++ b/Bugsnag/Assets/Bugsnag/Runtime/Delivery.cs @@ -73,7 +73,7 @@ public void Deliver(IPayload payload) { try { - if (!onSendErrorCallback.Invoke(report.Event)) + if (!RunEventCallback(onSendErrorCallback, report.Event)) { return; } @@ -99,6 +99,24 @@ public void Deliver(IPayload payload) } } + private bool RunEventCallback(Func callback, IEvent @event) + { + try + { + var initialUnhandledState = @event.Unhandled; + var callbackResult = callback.Invoke(@event); + if (initialUnhandledState != @event.Unhandled) + { + ((Payload.Event)@event).UnhandledOverridden(); + } + return callbackResult; + } + catch + { + return true; + } + } + // Push to the server and handle the result IEnumerator PushToServer(IPayload payload) { diff --git a/Bugsnag/Assets/Bugsnag/Runtime/Payload/Event.cs b/Bugsnag/Assets/Bugsnag/Runtime/Payload/Event.cs index ec3b24b2b..d4178f56f 100644 --- a/Bugsnag/Assets/Bugsnag/Runtime/Payload/Event.cs +++ b/Bugsnag/Assets/Bugsnag/Runtime/Payload/Event.cs @@ -36,17 +36,9 @@ internal Event(string context, Metadata metadata, AppWithState app, DeviceWithSt breadcrumbsList.Add(crumb); } Breadcrumbs = new ReadOnlyCollection(breadcrumbsList); - _originalUnhandledState = !handledState.Handled; if (session != null) { - if (handledState.Handled) - { - session.Events.IncrementHandledCount(); - } - else - { - session.Events.IncrementUnhandledCount(); - } + session.Events.UpdateEventCount(handledState.Handled, true); Session = session; } _featureFlags = featureFlags; @@ -161,9 +153,6 @@ internal void AddAndroidProjectPackagesToEvent(string[] packages) { _androidProjectPackages = packages; } - - private bool _originalUnhandledState; - private OrderedDictionary _featureFlags; HandledState _handledState; @@ -213,24 +202,28 @@ public bool Unhandled } set { - if(value != _originalUnhandledState && Session != null) - { - if (value) - { - Session.Events.IncrementUnhandledCount(); - Session.Events.DecrementHandledCount(); - } - else - { - Session.Events.IncrementHandledCount(); - Session.Events.DecrementUnhandledCount(); - } - } Add("unhandled", value); } } - + internal void UnhandledOverridden() + { + if (Session != null) + { + var events = Session.Events; + if (Unhandled) + { + events.UpdateUnhandledCount(true); + events.UpdateHandledCount(false); + } + else + { + events.UpdateUnhandledCount(false); + events.UpdateHandledCount(true); + } + } + _handledState.UnhandledOverridden(); + } public string Context { get; set; } 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/Session.cs b/Bugsnag/Assets/Bugsnag/Runtime/Payload/Session.cs index 0035e03dc..4d9219a97 100644 --- a/Bugsnag/Assets/Bugsnag/Runtime/Payload/Session.cs +++ b/Bugsnag/Assets/Bugsnag/Runtime/Payload/Session.cs @@ -74,14 +74,7 @@ internal Session(string providedGuid, DateTimeOffset startedAt, int handled, int internal void AddException(Report report) { - if (report.IsHandled) - { - Events.IncrementHandledCount(); - } - else - { - Events.IncrementUnhandledCount(); - } + Events.UpdateEventCount(report.IsHandled, true); } internal Session Copy() @@ -96,48 +89,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 void IncrementHandledCount() - { - lock (_handledLock) - { - this["handled"]++; - } - } + internal int Handled => this[HANDLED_KEY]; + internal int Unhandled => this[UNHANDLED_KEY]; - internal void DecrementHandledCount() + internal 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 DecrementUnhandledCount() + internal void UpdateHandledCount(bool increment) { - lock (_unhandledLock) - { - this["unhandled"]--; - } + UpdateEventCount(true, increment); } } } diff --git a/features/csharp/csharp_callbacks.feature b/features/csharp/csharp_callbacks.feature index ea013f20f..e97f1c43e 100644 --- a/features/csharp/csharp_callbacks.feature +++ b/features/csharp/csharp_callbacks.feature @@ -5,7 +5,6 @@ Feature: Callbacks Scenario: OnError callbacks in config When I run the game in the "OnErrorInConfig" state - And I wait to receive a session And I wait to receive 2 errors And I sort the errors by the payload field "events.0.exceptions.0.message" @@ -21,10 +20,6 @@ Feature: Callbacks Then the error is valid for the error reporting API sent by the Unity notifier And the exception "message" equals "Error 2" And all possible parameters have been edited in a callback - And the event "session.events.unhandled" equals 2 - And the event "session.events.handled" equals 0 - - Scenario: OnError callbacks added after Start When I run the game in the "OnErrorAfterStart" state @@ -71,3 +66,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 bec8bb184..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.Unhandled = true; @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: diff --git a/features/fixtures/maze_runner/Assets/Scripts/Scenarios/Csharp/Callbacks/OnErrorInConfig.cs b/features/fixtures/maze_runner/Assets/Scripts/Scenarios/Csharp/Callbacks/OnErrorInConfig.cs index 0c23b4a64..b3a5a090d 100644 --- a/features/fixtures/maze_runner/Assets/Scripts/Scenarios/Csharp/Callbacks/OnErrorInConfig.cs +++ b/features/fixtures/maze_runner/Assets/Scripts/Scenarios/Csharp/Callbacks/OnErrorInConfig.cs @@ -11,7 +11,6 @@ public override void PrepareConfig(string apiKey, string host) public override void Run() { - Bugsnag.StartSession(); Bugsnag.Notify(new Exception("Error 1")); throw new Exception("Error 2"); }