Improve SentryHttpFailedRequestHandler issue grouping

CHANGELOG.md

@@ -42,6 +42,7 @@
 - The `Serilog` integration captures _Structured Logs_ (when enabled) independently of captured Events and added Breadcrumbs ([#4691](https://github.com/getsentry/sentry-dotnet/pull/4691))
 - The SDK avoids redundant scope sync after transaction finish ([#4623](https://github.com/getsentry/sentry-dotnet/pull/4623))
 - sentry-native is now automatically disabled for WASM applications ([#4631](https://github.com/getsentry/sentry-dotnet/pull/4631))
+- Captured [Http Client Errors](https://docs.sentry.io/platforms/dotnet/guides/aspnet/configuration/http-client-errors/) on .NET 6+ now include a full stack trace in order to improve Issue grouping ([#4724](https://github.com/getsentry/sentry-dotnet/pull/4724))
 
 ### Dependencies
 

src/Sentry/HttpStatusCodeRangeExtensions.cs

@@ -0,0 +1,25 @@
+namespace Sentry;
+
+/// <summary>
+/// Extension methods for collections of <see cref="HttpStatusCodeRange"/>.
+/// </summary>
+internal static class HttpStatusCodeRangeExtensions
+{
+    /// <summary>
+    /// Checks if any range in the collection contains the given status code.
+    /// </summary>
+    /// <param name="ranges">Collection of ranges to check.</param>
+    /// <param name="statusCode">Status code to check.</param>
+    /// <returns>True if any range contains the given status code.</returns>
+    internal static bool ContainsStatusCode(this IEnumerable<HttpStatusCodeRange> ranges, int statusCode)
+        => ranges.Any(range => range.Contains(statusCode));
+
+    /// <summary>
+    /// Checks if any range in the collection contains the given status code.
+    /// </summary>
+    /// <param name="ranges">Collection of ranges to check.</param>
+    /// <param name="statusCode">Status code to check.</param>
+    /// <returns>True if any range contains the given status code.</returns>
+    internal static bool ContainsStatusCode(this IEnumerable<HttpStatusCodeRange> ranges, HttpStatusCode statusCode)
+        => ranges.ContainsStatusCode((int)statusCode);
+}

src/Sentry/SentryHttpFailedRequestHandler.cs

@@ -15,67 +15,85 @@ internal SentryHttpFailedRequestHandler(IHub hub, SentryOptions options)
     protected internal override void DoEnsureSuccessfulResponse([NotNull] HttpRequestMessage request, [NotNull] HttpResponseMessage response)
     {
         // Don't capture events for successful requests
-        if (!Options.FailedRequestStatusCodes.Any(range => range.Contains(response.StatusCode)))
+        if (!Options.FailedRequestStatusCodes.ContainsStatusCode(response.StatusCode))
         {
             return;
         }
 
         // Capture the event
-        try
+
+        var statusCode = (int)response.StatusCode;
+        // Match behavior of HttpResponseMessage.EnsureSuccessStatusCode
+        // See https://github.com/getsentry/sentry-dotnet/issues/2684
+        if (statusCode >= 200 && statusCode <= 299)
         {
-#if NET5_0_OR_GREATER
-            response.EnsureSuccessStatusCode();
+            return;
+        }
+
+        var exception = new HttpRequestException(
+            string.Format(
+                System.Globalization.CultureInfo.InvariantCulture,
+                "Response status code does not indicate success: {0}",
+                statusCode)
+            );
+
+#if NET6_0_OR_GREATER
+        // Add a full stack trace into the exception to improve Issue grouping,
+        // see https://github.com/getsentry/sentry-dotnet/issues/3582
+        ExceptionDispatchInfo.SetCurrentStackTrace(exception);
 #else
-            // Use our own implementation of EnsureSuccessStatusCode because older implementations of
-            // EnsureSuccessStatusCode disposes the content.
-            // See https://github.com/dotnet/runtime/issues/24845
-            response.StatusCode.EnsureSuccessStatusCode();
-#endif
+        // Where SetRemoteStackTrace is not available, throw and catch to get a basic stack trace
+        try
+        {
+            throw exception;
         }
-        catch (HttpRequestException exception)
+        catch (HttpRequestException ex)
         {
-            exception.SetSentryMechanism(MechanismType);
+            exception = ex;
+        }
+#endif
 
-            var @event = new SentryEvent(exception);
-            var hint = new SentryHint(HintTypes.HttpResponseMessage, response);
+        exception.SetSentryMechanism(MechanismType);
 
-            var uri = response.RequestMessage?.RequestUri;
-            var sentryRequest = new SentryRequest
-            {
-                QueryString = uri?.Query,
-                Method = response.RequestMessage?.Method.Method.ToUpperInvariant()
-            };
+        var @event = new SentryEvent(exception);
+        var hint = new SentryHint(HintTypes.HttpResponseMessage, response);
 
-            var responseContext = new Response
-            {
-                StatusCode = (short)response.StatusCode,
+        var uri = response.RequestMessage?.RequestUri;
+        var sentryRequest = new SentryRequest
+        {
+            QueryString = uri?.Query,
+            Method = response.RequestMessage?.Method.Method.ToUpperInvariant()
+        };
+
+        var responseContext = new Response
+        {
+            StatusCode = (short)response.StatusCode,
 #if NET5_0_OR_GREATER
-                // Starting with .NET 5, the content and headers are guaranteed to not be null.
-                BodySize = response.Content.Headers.ContentLength
+            // Starting with .NET 5, the content and headers are guaranteed to not be null.
+            BodySize = response.Content.Headers.ContentLength
 #else
-                // The ContentLength might be null (but that's ok).
-                // See https://github.com/dotnet/runtime/issues/16162
-                BodySize = response.Content?.Headers?.ContentLength
+            // The ContentLength might be null (but that's ok).
+            // See https://github.com/dotnet/runtime/issues/16162
+            BodySize = response.Content?.Headers?.ContentLength
 #endif
-            };
+        };
 
-            if (!Options.SendDefaultPii)
-            {
-                sentryRequest.Url = uri?.HttpRequestUrl();
-            }
-            else
-            {
-                sentryRequest.Url = uri?.AbsoluteUri;
-                sentryRequest.Cookies = request.Headers.GetCookies();
-                sentryRequest.AddHeaders(request.Headers);
-                responseContext.Cookies = response.Headers.GetCookies();
-                responseContext.AddHeaders(response.Headers);
-            }
+        if (!Options.SendDefaultPii)
+        {
+            sentryRequest.Url = uri?.HttpRequestUrl();
+        }
+        else
+        {
+            sentryRequest.Url = uri?.AbsoluteUri;
+            sentryRequest.Cookies = request.Headers.GetCookies();
+            sentryRequest.AddHeaders(request.Headers);
+            responseContext.Cookies = response.Headers.GetCookies();
+            responseContext.AddHeaders(response.Headers);
+        }
 
-            @event.Request = sentryRequest;
-            @event.Contexts[Response.Type] = responseContext;
+        @event.Request = sentryRequest;
+        @event.Contexts[Response.Type] = responseContext;
 
-            Hub.CaptureEvent(@event, hint: hint);
-        }
+        Hub.CaptureEvent(@event, hint: hint);
     }
 }

test/Sentry.Tests/HttpStatusCodeRangeExtensionsTests.cs

@@ -0,0 +1,171 @@
+namespace Sentry.Tests;
+
+public class HttpStatusCodeRangeExtensionsTests
+{
+    [Fact]
+    public void ContainsStatusCode_EmptyList_ReturnsFalse()
+    {
+        // Arrange
+        var ranges = new List<HttpStatusCodeRange>();
+
+        // Act
+        var result = ranges.ContainsStatusCode(404);
+
+        // Assert
+        result.Should().BeFalse();
+    }
+
+    [Theory]
+    [InlineData(400)]
+    [InlineData(450)]
+    [InlineData(499)]
+    public void ContainsStatusCode_SingleRangeInRange_ReturnsTrue(int statusCode)
+    {
+        // Arrange
+        var ranges = new List<HttpStatusCodeRange> { (400, 499) };
+
+        // Act
+        var result = ranges.ContainsStatusCode(statusCode);
+
+        // Assert
+        result.Should().BeTrue();
+    }
+
+    [Theory]
+    [InlineData(200)]
+    [InlineData(399)]
+    [InlineData(500)]
+    public void ContainsStatusCode_SingleRangeOutOfRange_ReturnsFalse(int statusCode)
+    {
+        // Arrange
+        var ranges = new List<HttpStatusCodeRange> { (400, 499) };
+
+        // Act
+        var result = ranges.ContainsStatusCode(statusCode);
+
+        // Assert
+        result.Should().BeFalse();
+    }
+
+    [Theory]
+    [InlineData(400)] // In first range
+    [InlineData(404)] // In first range
+    [InlineData(500)] // In second range
+    [InlineData(503)] // In second range
+    public void ContainsStatusCode_MultipleRangesInAnyRange_ReturnsTrue(int statusCode)
+    {
+        // Arrange
+        var ranges = new List<HttpStatusCodeRange> { (400, 404), (500, 503) };
+
+        // Act
+        var result = ranges.ContainsStatusCode(statusCode);
+
+        // Assert
+        result.Should().BeTrue();
+    }
+
+    [Theory]
+    [InlineData(200)] // Below ranges
+    [InlineData(405)] // Between ranges
+    [InlineData(499)] // Between ranges
+    [InlineData(504)] // Above ranges
+    public void ContainsStatusCode_MultipleRangesNotInAnyRange_ReturnsFalse(int statusCode)
+    {
+        // Arrange
+        var ranges = new List<HttpStatusCodeRange> { (400, 404), (500, 503) };
+
+        // Act
+        var result = ranges.ContainsStatusCode(statusCode);
+
+        // Assert
+        result.Should().BeFalse();
+    }
+
+    [Theory]
+    [InlineData(400)] // In first range only
+    [InlineData(425)] // In overlap
+    [InlineData(450)] // In overlap
+    [InlineData(475)] // In second range only
+    public void ContainsStatusCode_OverlappingRangesInUnion_ReturnsTrue(int statusCode)
+    {
+        // Arrange
+        var ranges = new List<HttpStatusCodeRange> { (400, 450), (425, 475) };
+
+        // Act
+        var result = ranges.ContainsStatusCode(statusCode);
+
+        // Assert
+        result.Should().BeTrue();
+    }
+
+    [Theory]
+    [InlineData(200)] // Below first range
+    [InlineData(399)] // Below first range
+    [InlineData(476)] // Above second range
+    [InlineData(500)] // Above second range
+    public void ContainsStatusCode_OverlappingRangesOutsideUnion_ReturnsFalse(int statusCode)
+    {
+        // Arrange
+        var ranges = new List<HttpStatusCodeRange> { (400, 450), (425, 475) };
+
+        // Act
+        var result = ranges.ContainsStatusCode(statusCode);
+
+        // Assert
+        result.Should().BeFalse();
+    }
+
+    [Fact]
+    public void ContainsStatusCode_SingleValueRangeExactMatch_ReturnsTrue()
+    {
+        // Arrange
+        var ranges = new List<HttpStatusCodeRange> { 404 };
+
+        // Act
+        var result = ranges.ContainsStatusCode(404);
+
+        // Assert
+        result.Should().BeTrue();
+    }
+
+    [Theory]
+    [InlineData(403)]
+    [InlineData(405)]
+    public void ContainsStatusCode_SingleValueRangeNoMatch_ReturnsFalse(int statusCode)
+    {
+        // Arrange
+        var ranges = new List<HttpStatusCodeRange> { 404 };
+
+        // Act
+        var result = ranges.ContainsStatusCode(statusCode);
+
+        // Assert
+        result.Should().BeFalse();
+    }
+
+    [Fact]
+    public void ContainsStatusCode_HttpStatusCodeEnumInRange_ReturnsTrue()
+    {
+        // Arrange
+        var ranges = new List<HttpStatusCodeRange> { (400, 499) };
+
+        // Act
+        var result = ranges.ContainsStatusCode(HttpStatusCode.NotFound); // 404
+
+        // Assert
+        result.Should().BeTrue();
+    }
+
+    [Fact]
+    public void ContainsStatusCode_HttpStatusCodeEnumOutOfRange_ReturnsFalse()
+    {
+        // Arrange
+        var ranges = new List<HttpStatusCodeRange> { (400, 499) };
+
+        // Act
+        var result = ranges.ContainsStatusCode(HttpStatusCode.OK); // 200
+
+        // Assert
+        result.Should().BeFalse();
+    }
+}