review changes

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<IEvent,bool> 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)
         {

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<IEvent,bool> 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)
         {

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<IBreadcrumb>(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; }
 

Bugsnag/Assets/Bugsnag/Runtime/Payload/HandledState.cs

@@ -9,6 +9,12 @@ namespace BugsnagUnity.Payload
     /// </summary>
     class HandledState : Dictionary<string, object>
     {
+
+        public void UnhandledOverridden()
+        {
+            var severityReason = this["severityReason"] as SeverityReason;
+            severityReason["unhandledOverridden"] = true;
+        }
         /// <summary>
         /// 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);
             }
         }
     }

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<string, int>
     {
-        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);
         }
     }
 }

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

features/csharp/csharp_events.feature

@@ -15,6 +15,8 @@ Feature: csharp events
       | NotifySmokeTest.Run()                                                       | Main+<RunNextMazeCommand>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

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

features/fixtures/maze_runner/Assets/Scripts/Scenario.cs

@@ -157,7 +157,6 @@ public bool SimpleEventCallback(IEvent @event)
         @event.AddMetadata("test1", new Dictionary<string, object> { { "test", "test" } });
         @event.AddMetadata("test2", new Dictionary<string, object> { { "test", "test" } });
         @event.ClearMetadata("test2");
-        @event.Unhandled = true;
         @event.AddFeatureFlag("fromCallback", "a");
 
         return true;