From 7062d8d8e7008d0297ff9c82206f3b333d9408fe Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Kamil=20Og=C3=B3rek?= <kamil.ogorek@gmail.com>
Date: Thu, 23 Jun 2022 18:30:37 +0200
Subject: [PATCH] ref(session): Change session update logic to follow the spec
 (#477)

---
 sentry-core/src/client.rs  | 24 +++++++++++++-----------
 sentry-core/src/session.rs | 10 +++++-----
 2 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/sentry-core/src/client.rs b/sentry-core/src/client.rs
index a04851b7..50bbea8b 100644
--- a/sentry-core/src/client.rs
+++ b/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 --git a/sentry-core/src/session.rs b/sentry-core/src/session.rs
index 288550a4..a163c4cf 100644
--- a/sentry-core/src/session.rs
+++ b/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");
         }