Data from Scope in options should be applied on each request (#1270)

Co-authored-by: Sentry Github Bot <bot+github-bot@sentry.io> Co-authored-by: Simon Cropp <simon.cropp@gmail.com> Co-authored-by: Bruno Garcia <bruno@brunogarcia.com>
getsentry · Nov 1, 2021 · f8f1f8a · f8f1f8a
1 parent 9a339af
commit f8f1f8a
Show file tree

Hide file tree

Showing 4 changed files with 114 additions and 6 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,7 @@
 
 ### Fixes
 
+- ASP.NET Core: Data from Scope in options should be applied on each request ([#1270](https://github.com/getsentry/sentry-dotnet/pull/1270))
 - Add missing `ConfigureAwaits(false)` for `async using` ([#1276](https://github.com/getsentry/sentry-dotnet/pull/1276))
 - Fix missing handled tag when events are logged via an ASP.NET Core pipeline logger ([#1284](getsentry/sentry-dotnet/pull/1284))
 

diff --git a/src/Sentry.AspNetCore/SentryMiddleware.cs b/src/Sentry.AspNetCore/SentryMiddleware.cs
@@ -56,11 +56,6 @@ public SentryMiddleware(
             _next = next ?? throw new ArgumentNullException(nameof(next));
             _getHub = getHub ?? throw new ArgumentNullException(nameof(getHub));
             _options = options.Value;
-            var hub = _getHub();
-            foreach (var callback in _options.ConfigureScopeCallbacks)
-            {
-                hub.ConfigureScope(callback);
-            }
             _hostingEnvironment = hostingEnvironment;
             _logger = logger;
         }
@@ -104,7 +99,11 @@ public async Task InvokeAsync(HttpContext context)
                     // In case of event, all data made available through the HTTP Context at the time of the
                     // event creation will be sent to Sentry
 
-                    scope.OnEvaluating += (_, _) => PopulateScope(context, scope);
+                    scope.OnEvaluating += (_, _) =>
+                    {
+                        SyncOptionsScope(hub);
+                        PopulateScope(context, scope);
+                    };
                 });
 
                 try
@@ -156,6 +155,14 @@ void CaptureException(Exception e, string mechanism)
             }
         }
 
+        private void SyncOptionsScope(IHub newHub)
+        {
+            foreach (var callback in _options.ConfigureScopeCallbacks)
+            {
+                newHub.ConfigureScope(callback);
+            }
+        }
+
         internal void PopulateScope(HttpContext context, Scope scope)
         {
             scope.Sdk.Name = Constants.SdkName;

diff --git a/test/Sentry.AspNetCore.Tests/MiddlewareLoggerIntegration.cs b/test/Sentry.AspNetCore.Tests/MiddlewareLoggerIntegration.cs
@@ -45,6 +45,9 @@ public Fixture()
                 };
                 loggingOptions.InitializeSdk = false;
 
+                Client.When(client => client.CaptureEvent(Arg.Any<SentryEvent>(), Arg.Any<Scope>()))
+                .Do(callback => callback.Arg<Scope>().Evaluate());
+
                 var hub = new Hub(new SentryOptions { Dsn = DsnSamples.ValidDsnWithSecret });
                 hub.BindClient(Client);
                 Hub = hub;

diff --git a/test/Sentry.AspNetCore.Tests/SentryMiddlewareTests.cs b/test/Sentry.AspNetCore.Tests/SentryMiddlewareTests.cs
@@ -1,6 +1,8 @@
 using System;
 using System.Diagnostics;
 using System.IO;
+using System.Linq;
+using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Diagnostics;
 #if NETCOREAPP2_1 || NET461
@@ -31,9 +33,17 @@ private class Fixture
             public ILogger<SentryMiddleware> Logger { get; set; } = Substitute.For<ILogger<SentryMiddleware>>();
             public HttpContext HttpContext { get; set; } = Substitute.For<HttpContext>();
             public IFeatureCollection FeatureCollection { get; set; } = Substitute.For<IFeatureCollection>();
+            public Scope Scope { get; set; }
 
             public Fixture()
             {
+                Scope = new();
+                Hub.When(hub => hub.ConfigureScope(Arg.Any<Action<Scope>>()))
+                .Do(callback => callback.Arg<Action<Scope>>().Invoke(Scope));
+
+                Hub.When(hub => hub.CaptureEvent(Arg.Any<SentryEvent>(), Arg.Any<Scope>()))
+                .Do(_ => Scope.Evaluate());
+
                 HubAccessor = () => Hub;
                 _ = Hub.IsEnabled.Returns(true);
                 _ = Hub.StartTransaction("", "").ReturnsForAnyArgs(new TransactionTracer(Hub, "test", "test"));
@@ -521,5 +531,92 @@ public async Task InvokeAsync_FlushBeforeRequestCompletedTrue_RespectsTimeout()
 
             await _fixture.Hub.Received(1).FlushAsync(timeout);
         }
+
+        [Fact]
+        public async Task InvokeAsync_ScopeNotPopulated_CopyOptionsToScope()
+        {
+            // Arrange
+            var expectedAction = new Action<Scope>(scope => scope.SetTag("A", "B"));
+            _fixture.Options.ConfigureScope(expectedAction);
+            var expectedExceptionMessage = "Expected Exception";
+            _fixture.RequestDelegate = _ => throw new Exception(expectedExceptionMessage);
+            var sut = _fixture.GetSut();
+
+            // Act
+            try
+            {
+                await sut.InvokeAsync(_fixture.HttpContext);
+            }
+            catch (Exception ex) when (ex.Message == expectedExceptionMessage)
+            { }
+
+            // Assert
+            _fixture.Hub.Received(1).ConfigureScope(Arg.Is(expectedAction));
+        }
+
+        [Fact]
+        public async Task InvokeAsync_SameMiddleWareWithSameHubs_CopyOptionsOnce()
+        {
+            // Arrange
+            var expectedAction = new Action<Scope>(scope => scope.SetTag("A", "B"));
+            var expectedExceptionMessage = "Expected Exception";
+            _fixture.RequestDelegate = _ => throw new Exception(expectedExceptionMessage);
+            _fixture.Options.ConfigureScope(expectedAction);
+            var sut = _fixture.GetSut();
+
+            // Act
+            try
+            {
+                await sut.InvokeAsync(_fixture.HttpContext);
+            }
+            catch (Exception ex) when (ex.Message == expectedExceptionMessage)
+            { }
+
+            try
+            {
+                await sut.InvokeAsync(_fixture.HttpContext);
+            }
+            catch (Exception ex) when (ex.Message == expectedExceptionMessage)
+            { }
+
+            // Assert
+            _fixture.Hub.Received(1).ConfigureScope(Arg.Is(expectedAction));
+        }
+
+        [Fact]
+        public async Task InvokeAsync_SameMiddleWareWithDifferentHubs_CopyOptionsToAllHubs()
+        {
+            // Arrange
+            var firstHub = _fixture.Hub;
+            var expectedExceptionMessage = "Expected Exception";
+            _fixture.RequestDelegate = _ => throw new Exception(expectedExceptionMessage);
+            var expectedAction = new Action<Scope>(scope => scope.SetTag("A", "B"));
+            _fixture.Options.ConfigureScope(expectedAction);
+            var sut = _fixture.GetSut();
+
+            // Act
+            try
+            {
+                await sut.InvokeAsync(_fixture.HttpContext);
+            }
+            catch (Exception ex) when (ex.Message == expectedExceptionMessage)
+            { }
+
+            // Replacing the Hub
+            // Arrange
+            var secondHub = new Fixture().Hub;
+            _fixture.Hub = secondHub;
+
+            // Act
+            try
+            {
+                await sut.InvokeAsync(_fixture.HttpContext);
+            }
+            catch { }
+
+            // Assert
+            firstHub.Received(1).ConfigureScope(Arg.Is(expectedAction));
+            secondHub.Received(1).ConfigureScope(Arg.Is(expectedAction));
+        }
     }
 }