IHub.ResumeSession(): don't start a new session if pause wasn't calle…

…d or if there is no active session (#1089) * IHub.ResumeSession(): don't start a new session if pause wasn't called or if there is no active session * Changelog * rearrange * asd
getsentry · Jun 25, 2021 · 241ec58 · 241ec58
1 parent 9a07c9a
commit 241ec58
Show file tree

Hide file tree

Showing 5 changed files with 87 additions and 4 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased
+
+### Fixes
+
+- `IHub.ResumeSession()`: don't start a new session if pause wasn't called or if there is no active session ([#1089](https://github.com/getsentry/sentry-dotnet/pull/1089))
+
 ## 3.6.0
 
 ### Features

diff --git a/src/Sentry/GlobalSessionManager.cs b/src/Sentry/GlobalSessionManager.cs
@@ -23,6 +23,8 @@ internal class GlobalSessionManager : ISessionManager
         // Internal for testing
         internal Session? CurrentSession => _currentSession;
 
+        public bool IsSessionActive => _currentSession is not null;
+
         public GlobalSessionManager(SentryOptions options, ISystemClock clock)
         {
             _options = options;

diff --git a/src/Sentry/ISessionManager.cs b/src/Sentry/ISessionManager.cs
@@ -4,6 +4,8 @@ namespace Sentry
 {
     internal interface ISessionManager
     {
+        bool IsSessionActive { get; }
+
         SessionUpdate? StartSession();
 
         SessionUpdate? EndSession(DateTimeOffset timestamp, SessionEndStatus status);

diff --git a/src/Sentry/Internal/Hub.cs b/src/Sentry/Internal/Hub.cs
@@ -22,7 +22,7 @@ internal class Hub : IHub, IDisposable
         private readonly IDisposable _rootScope;
         private readonly Enricher _enricher;
 
-        private DateTimeOffset _sessionPauseTimestamp;
+        private DateTimeOffset? _sessionPauseTimestamp;
 
         // Internal for testability
         internal ConditionalWeakTable<Exception, ISpan> ExceptionToSpanMap { get; } = new();
@@ -198,15 +198,29 @@ public void PauseSession()
         {
             lock (_sessionPauseLock)
             {
-                _sessionPauseTimestamp = _clock.GetUtcNow();
+                // Only pause if there's anything to pause.
+                // This might race if a session is started at the same time,
+                // but that's fine.
+                if (_sessionManager.IsSessionActive)
+                {
+                    _sessionPauseTimestamp = _clock.GetUtcNow();
+                }
             }
         }
 
         public void ResumeSession()
         {
             lock (_sessionPauseLock)
             {
-                var pauseDuration = (_clock.GetUtcNow() - _sessionPauseTimestamp).Duration();
+                // Ensure a session has been paused before
+                if (_sessionPauseTimestamp is not { } sessionPauseTimestamp)
+                {
+                    return;
+                }
+
+                // If the pause duration exceeded tracking interval, start a new session
+                // (otherwise do nothing)
+                var pauseDuration = (_clock.GetUtcNow() - sessionPauseTimestamp).Duration();
                 if (pauseDuration >= _options.AutoSessionTrackingInterval)
                 {
                     _options.DiagnosticLogger?.LogDebug(
@@ -215,9 +229,12 @@ public void ResumeSession()
                         pauseDuration
                     );
 
-                    EndSession(_sessionPauseTimestamp, SessionEndStatus.Exited);
+                    EndSession(sessionPauseTimestamp, SessionEndStatus.Exited);
                     StartSession();
                 }
+
+                // Reset the pause timestamp since the session is now resumed
+                _sessionPauseTimestamp = null;
             }
         }
 

diff --git a/test/Sentry.Tests/HubTests.cs b/test/Sentry.Tests/HubTests.cs
@@ -876,5 +876,61 @@ public void ResumeSession_BeyondAutoTrackingInterval_EndsPreviousSessionAndStart
             client.Received().CaptureSession(Arg.Is<SessionUpdate>(s => s.EndStatus == SessionEndStatus.Exited));
             client.Received().CaptureSession(Arg.Is<SessionUpdate>(s => s.IsInitial));
         }
+
+        [Fact]
+        public void ResumeSession_NoActiveSession_DoesNothing()
+        {
+            // Arrange
+            var client = Substitute.For<ISentryClient>();
+            var clock = Substitute.For<ISystemClock>();
+
+            var options = new SentryOptions
+            {
+                Dsn = DsnSamples.ValidDsnWithSecret,
+                AutoSessionTrackingInterval = TimeSpan.FromMilliseconds(10)
+            };
+
+            var hub = new Hub(client, clock, new GlobalSessionManager(options, clock), options);
+
+            clock.GetUtcNow().Returns(DateTimeOffset.Now);
+
+            hub.PauseSession();
+
+            clock.GetUtcNow().Returns(DateTimeOffset.Now + TimeSpan.FromDays(1));
+
+            // Act
+            hub.ResumeSession();
+
+            // Assert
+            client.DidNotReceive().CaptureSession(Arg.Any<SessionUpdate>());
+        }
+
+        [Fact]
+        public void ResumeSession_NoPausedSession_DoesNothing()
+        {
+            // Arrange
+            var client = Substitute.For<ISentryClient>();
+            var clock = Substitute.For<ISystemClock>();
+
+            var options = new SentryOptions
+            {
+                Dsn = DsnSamples.ValidDsnWithSecret,
+                AutoSessionTrackingInterval = TimeSpan.FromMilliseconds(10)
+            };
+
+            var hub = new Hub(client, clock, new GlobalSessionManager(options, clock), options);
+
+            clock.GetUtcNow().Returns(DateTimeOffset.Now);
+
+            hub.StartSession();
+
+            clock.GetUtcNow().Returns(DateTimeOffset.Now + TimeSpan.FromDays(1));
+
+            // Act
+            hub.ResumeSession();
+
+            // Assert
+            client.DidNotReceive().CaptureSession(Arg.Is<SessionUpdate>(s => s.EndStatus != null));
+        }
     }
 }