Fixed: SentryHttpFailedRequestHandler disposes Content on .net framework

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased
+
+### Fixes
+
+- response.Content is not disposed by SentryHttpFailedRequestHandler on .net framework ([#3306](https://github.com/getsentry/sentry-dotnet/pull/3306))
+
 ## 4.4.0
 
 ### Features

src/Sentry/Internal/Extensions/HttpStatusExtensions.cs

@@ -0,0 +1,28 @@
+namespace Sentry.Internal.Extensions;
+
+#if !NET5_0_OR_GREATER
+internal static class HttpStatusExtensions
+{
+    private const string HttpRequestExceptionMessage = "Response status code does not indicate success: {0}";
+
+    /// <summary>
+    /// This mimics the behaviour of <see cref="HttpResponseMessage.EnsureSuccessStatusCode"/> for netcore3.0 and later
+    /// by throwing an exception if the status code is outside the 200 range without disposing the content.
+    ///
+    /// See https://github.com/getsentry/sentry-dotnet/issues/2684
+    /// </summary>
+    /// <param name="statusCode"></param>
+    /// <exception cref="HttpRequestException"></exception>
+    public static void EnsureSuccessStatusCode(this HttpStatusCode statusCode)
+    {
+        if ((int)statusCode < 200 || (int)statusCode > 299)
+        {
+            throw new HttpRequestException(string.Format(
+                CultureInfo.InvariantCulture,
+                HttpRequestExceptionMessage,
+                (int)statusCode
+            ));
+        }
+    }
+}
+#endif

src/Sentry/SentryHttpFailedRequestHandler.cs

@@ -1,3 +1,4 @@
+using Sentry.Internal.Extensions;
 using Sentry.Protocol;
 
 namespace Sentry;
@@ -19,23 +20,17 @@ protected internal override void DoEnsureSuccessfulResponse([NotNull] HttpReques
             return;
         }
 
-#if NET5_0_OR_GREATER
-        // Starting with .NET 5, the content and headers are guaranteed to not be null.
-        var bodySize = response.Content.Headers.ContentLength;
-#else
-        // We have to get the content body size before calling EnsureSuccessStatusCode,
-        // because older implementations of EnsureSuccessStatusCode disposes the content.
-        // See https://github.com/dotnet/runtime/issues/24845
-
-        // The ContentLength might be null (but that's ok).
-        // See https://github.com/dotnet/runtime/issues/16162
-        var bodySize = response.Content?.Headers?.ContentLength;
-#endif
-
         // Capture the event
         try
         {
+#if NET5_0_OR_GREATER
             response.EnsureSuccessStatusCode();
+#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
         }
         catch (HttpRequestException exception)
         {
@@ -54,7 +49,14 @@ protected internal override void DoEnsureSuccessfulResponse([NotNull] HttpReques
             var responseContext = new Response
             {
                 StatusCode = (short)response.StatusCode,
-                BodySize = bodySize
+#if NET5_0_OR_GREATER
+                // 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
+#endif
             };
 
             if (!Options.SendDefaultPii)

test/Sentry.Tests/Internals/Extensions/HttpRequestExceptionMessageTests.cs

@@ -0,0 +1,41 @@
+namespace Sentry.Tests.Internals.Extensions;
+
+#if !NET5_0_OR_GREATER
+public class HttpRequestExceptionMessageTests
+{
+    [Fact]
+    public void EnsureSuccessStatusCode_StatusCodeInRange_DoesNotThrow()
+    {
+        // Arrange
+        const HttpStatusCode statusCode = HttpStatusCode.OK; // Any status code in the 200-299 range
+
+        // Act
+        var act = () => statusCode.EnsureSuccessStatusCode();
+
+        // Assert
+        act.Should().NotThrow<HttpRequestException>();
+    }
+
+    [Fact]
+    public void EnsureSuccessStatusCode_StatusCodeOutOfRange_ThrowsHttpRequestException()
+    {
+        var unsuccessfulStatusCodes = new List<int>();
+        unsuccessfulStatusCodes.AddRange(Enumerable.Range(0, 199));
+        unsuccessfulStatusCodes.AddRange(Enumerable.Range(300, 600));
+        foreach (var i in unsuccessfulStatusCodes)
+        {
+            // Arrange and Act
+            var statusCode = (HttpStatusCode)i;
+            var act = () => statusCode.EnsureSuccessStatusCode();
+
+            // Assert
+            act.Should().Throw<HttpRequestException>()
+                .WithMessage(string.Format(
+                    CultureInfo.InvariantCulture,
+                    "Response status code does not indicate success: {0}",
+                    (int)statusCode
+                ));
+        }
+    }
+}
+#endif