From d1bfe871a26d0148ce5fafe1763ace845c2f86c5 Mon Sep 17 00:00:00 2001 From: bitsandfoxes Date: Thu, 29 Feb 2024 15:48:18 +0100 Subject: [PATCH 1/5] made the middleware re-entrant --- src/Sentry.AspNetCore/SentryMiddleware.cs | 10 +++++-- src/Sentry/Internal/MainExceptionProcessor.cs | 2 +- .../SentryMiddlewareTests.cs | 28 +++++++++++++++++++ 3 files changed, 36 insertions(+), 4 deletions(-) diff --git a/src/Sentry.AspNetCore/SentryMiddleware.cs b/src/Sentry.AspNetCore/SentryMiddleware.cs index 9338fd0bb1..4988474b78 100644 --- a/src/Sentry.AspNetCore/SentryMiddleware.cs +++ b/src/Sentry.AspNetCore/SentryMiddleware.cs @@ -97,9 +97,13 @@ public async Task InvokeAsync(HttpContext context, RequestDelegate next) var transactionContext = hub.ContinueTrace(traceHeader, baggageHeader); // Adding the headers and the TransactionContext to the context to be picked up by the Sentry tracing middleware - context.Items.Add(TraceHeaderItemKey, traceHeader); - context.Items.Add(BaggageHeaderItemKey, baggageHeader); - context.Items.Add(TransactionContextItemKey, transactionContext); + var didAdd = context.Items.TryAdd(TraceHeaderItemKey, traceHeader); + if (!didAdd) + { + _options.LogDebug("Sentry trace was already added. Did you initialize Sentry twice?"); + } + context.Items.TryAdd(BaggageHeaderItemKey, baggageHeader); + context.Items.TryAdd(TransactionContextItemKey, transactionContext); hub.ConfigureScope(scope => { diff --git a/src/Sentry/Internal/MainExceptionProcessor.cs b/src/Sentry/Internal/MainExceptionProcessor.cs index 90ec5e649d..cd9f283d03 100644 --- a/src/Sentry/Internal/MainExceptionProcessor.cs +++ b/src/Sentry/Internal/MainExceptionProcessor.cs @@ -36,7 +36,7 @@ internal IReadOnlyList CreateSentryExceptions(Exception excepti { var exceptions = WalkExceptions(exception).Reverse().ToList(); - // In the case of only one exception, ExceptionId and ParentId are useless. + // In the case of only one exception, ExceptionId and ParentId are useless./ if (exceptions.Count == 1 && exceptions[0].Mechanism is { } mechanism) { mechanism.ExceptionId = null; diff --git a/test/Sentry.AspNetCore.Tests/SentryMiddlewareTests.cs b/test/Sentry.AspNetCore.Tests/SentryMiddlewareTests.cs index f2bedc76e8..9778c1c11b 100644 --- a/test/Sentry.AspNetCore.Tests/SentryMiddlewareTests.cs +++ b/test/Sentry.AspNetCore.Tests/SentryMiddlewareTests.cs @@ -749,6 +749,34 @@ public async Task InvokeAsync_RequestContainsSentryHeaders_AddsHeadersAndTransac Assert.NotNull(transactionContext); } + [Fact] + public async Task InvokeAsync_InvokingWithTheSameContextTwice_DoesNotThrow() + { + var sut = _fixture.GetSut(); + var request = Substitute.For(); + var fakeHeaders = new HeaderDictionary + { + { "Sentry-Trace", "4b4d2878507b43d3af7dd8c4ab7a96d9-3cc6fd1337d243de"}, + { "Baggage", "sentry-trace_id=4b4d2878507b43d3af7dd8c4ab7a96d9, sentry-public_key=eb18e953812b41c3aeb042e666fd3b5c"}, + }; + var contextItems = new Dictionary(); + _fixture.HttpContext.Items.When(items => items.Add(Arg.Any(), Arg.Any())) + .Do(info => + { + contextItems.Add(info.Args()[0], info.Args()[1]); + }); + _ = request.Headers.Returns(fakeHeaders); + _ = _fixture.HttpContext.Request.Returns(request); + _ = request.HttpContext.Returns(_fixture.HttpContext); + + // Faking having added the middleware twice by invoking the request with the same context twice + await sut.InvokeAsync(_fixture.HttpContext, _fixture.RequestDelegate); + await sut.InvokeAsync(_fixture.HttpContext, _fixture.RequestDelegate); + + _fixture.HttpContext.Items.Received(3); // Sanity check + Assert.Equal(3, contextItems.Count); + } + [Fact] public void PopulateScope_AddEventProcessors() { From 9189f2b6ea6a7c55673841243766fbafff6a4565 Mon Sep 17 00:00:00 2001 From: bitsandfoxes Date: Thu, 29 Feb 2024 15:49:58 +0100 Subject: [PATCH 2/5] . --- src/Sentry/Internal/MainExceptionProcessor.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Sentry/Internal/MainExceptionProcessor.cs b/src/Sentry/Internal/MainExceptionProcessor.cs index cd9f283d03..90ec5e649d 100644 --- a/src/Sentry/Internal/MainExceptionProcessor.cs +++ b/src/Sentry/Internal/MainExceptionProcessor.cs @@ -36,7 +36,7 @@ internal IReadOnlyList CreateSentryExceptions(Exception excepti { var exceptions = WalkExceptions(exception).Reverse().ToList(); - // In the case of only one exception, ExceptionId and ParentId are useless./ + // In the case of only one exception, ExceptionId and ParentId are useless. if (exceptions.Count == 1 && exceptions[0].Mechanism is { } mechanism) { mechanism.ExceptionId = null; From 6846a8492c91ff92bf2ae651cf346a077526a190 Mon Sep 17 00:00:00 2001 From: Stefan Jandl Date: Tue, 5 Mar 2024 12:38:30 +0100 Subject: [PATCH 3/5] Update src/Sentry.AspNetCore/SentryMiddleware.cs Co-authored-by: Bruno Garcia --- src/Sentry.AspNetCore/SentryMiddleware.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Sentry.AspNetCore/SentryMiddleware.cs b/src/Sentry.AspNetCore/SentryMiddleware.cs index 4988474b78..728bbb2b56 100644 --- a/src/Sentry.AspNetCore/SentryMiddleware.cs +++ b/src/Sentry.AspNetCore/SentryMiddleware.cs @@ -100,7 +100,7 @@ public async Task InvokeAsync(HttpContext context, RequestDelegate next) var didAdd = context.Items.TryAdd(TraceHeaderItemKey, traceHeader); if (!didAdd) { - _options.LogDebug("Sentry trace was already added. Did you initialize Sentry twice?"); + _options.LogWarning("Sentry trace was already added. Did you initialize Sentry twice?"); } context.Items.TryAdd(BaggageHeaderItemKey, baggageHeader); context.Items.TryAdd(TransactionContextItemKey, transactionContext); From 8e2157cea7a306a75b4b09bae9ad9d2b349724d7 Mon Sep 17 00:00:00 2001 From: bitsandfoxes Date: Tue, 5 Mar 2024 13:50:48 +0100 Subject: [PATCH 4/5] test --- test/Sentry.AspNetCore.Tests/SentryMiddlewareTests.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/test/Sentry.AspNetCore.Tests/SentryMiddlewareTests.cs b/test/Sentry.AspNetCore.Tests/SentryMiddlewareTests.cs index 9778c1c11b..b9ea10ff5b 100644 --- a/test/Sentry.AspNetCore.Tests/SentryMiddlewareTests.cs +++ b/test/Sentry.AspNetCore.Tests/SentryMiddlewareTests.cs @@ -761,10 +761,7 @@ public async Task InvokeAsync_InvokingWithTheSameContextTwice_DoesNotThrow() }; var contextItems = new Dictionary(); _fixture.HttpContext.Items.When(items => items.Add(Arg.Any(), Arg.Any())) - .Do(info => - { - contextItems.Add(info.Args()[0], info.Args()[1]); - }); + .Do(info => contextItems.TryAdd(info.Args()[0], info.Args()[1])); _ = request.Headers.Returns(fakeHeaders); _ = _fixture.HttpContext.Request.Returns(request); _ = request.HttpContext.Returns(_fixture.HttpContext); From 7dedb2dbfc0c4851bf12cfcceaf7e016664e2bed Mon Sep 17 00:00:00 2001 From: bitsandfoxes Date: Tue, 5 Mar 2024 13:54:23 +0100 Subject: [PATCH 5/5] Updated CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 92cffb4275..dd0f633c22 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ ### Fixes +- The Sentry Middleware on ASP.NET Core no longer throws an exception after having been initialized multiple times ([#3185](https://github.com/getsentry/sentry-dotnet/pull/3185)) - Empty strings are used instead of underscores to replace invalid metric tag values ([#3176](https://github.com/getsentry/sentry-dotnet/pull/3176)) ### Dependencies