ref(session): Change session update logic to follow the spec (#477)

Author: kamilogorek

Diff for: sentry-core/src/client.rs

@@ -158,14 +158,6 @@ impl Client {
         mut event: Event<'static>,
         scope: Option<&Scope>,
     ) -> Option<Event<'static>> {
-        if let Some(scope) = scope {
-            scope.update_session_from_event(&event);
-        }
-
-        if !self.sample_should_send() {
-            return None;
-        }
-
         // event_id and sdk_info are set before the processors run so that the
         // processors can poke around in that data.
         if event.event_id.is_nil() {
@@ -211,10 +203,20 @@ impl Client {
         if let Some(ref func) = self.options.before_send {
             sentry_debug!("invoking before_send callback");
             let id = event.event_id;
-            func(event).or_else(move || {
+            if let Some(processed_event) = func(event) {
+                event = processed_event;
+            } else {
                 sentry_debug!("before_send dropped event {:?}", id);
-                None
-            })
+                return None;
+            }
+        }
+
+        if let Some(scope) = scope {
+            scope.update_session_from_event(&event);
+        }
+
+        if !self.sample_should_send() {
+            None
         } else {
             Some(event)
         }

Diff for: sentry-core/src/session.rs

@@ -439,8 +439,9 @@ mod tests {
                     sentry::start_session();
                 }
 
-                // this error will be discarded because of the event processor,
-                // but the session will still be updated accordingly.
+                // This error will be discarded because of the event processor,
+                // and session will not be updated.
+                // Only events dropped due to sampling should update the session.
                 let err = "NaN".parse::<usize>().unwrap_err();
                 sentry::capture_error(&err);
             },
@@ -469,11 +470,10 @@ mod tests {
 
             assert_eq!(aggregates[0].distinct_id, None);
             assert_eq!(aggregates[0].exited, 50);
-            assert_eq!(aggregates[1].errored, 1);
 
+            assert_eq!(aggregates[1].errored, 0);
             assert_eq!(aggregates[1].distinct_id, Some("foo-bar".into()));
-            assert_eq!(aggregates[1].exited, 49);
-            assert_eq!(aggregates[1].errored, 1);
+            assert_eq!(aggregates[1].exited, 50);
         } else {
             panic!("expected session");
         }