Change EnsureSuccessStatusCode to not dispose response content (#29795)

* Change EnsureSuccessStatusCode to not dispose response content

Historically, this method has always disposed the response content if it was
throwing an exception. The reason documented in the comment really doesn't
make much sense, especially to just dispose the content but not the actual
response object. The developer pattern we advise when getting an HttpResponseMessage
is to wrap it in a 'using' statement or use an explicit .Dispose() call. And
disposing the HttpResponseMessage would always dispose the inner .Content
anyways.

The use case of #26684 is that the response content should be available regardless
if this method throws an exception or not. Developers normally don't expect an object
to get disposed (partially in this case) as a side-affect of calling a method on the
object. So, we're changing the behavior to be more consistent generally with other
.NET objects.

Fixes #26684

* Address PR feedback

src/System.Net.Http/src/System/Net/Http/HttpResponseMessage.cs

@@ -155,18 +155,13 @@ public HttpResponseMessage EnsureSuccessStatusCode()
         {
             if (!IsSuccessStatusCode)
             {
-                // Disposing the content should help users: If users call EnsureSuccessStatusCode(), an exception is
-                // thrown if the response status code is != 2xx. I.e. the behavior is similar to a failed request (e.g.
-                // connection failure). Users don't expect to dispose the content in this case: If an exception is 
-                // thrown, the object is responsible fore cleaning up its state.
-                if (_content != null)
-                {
-                    _content.Dispose();
-                }
-
-                throw new HttpRequestException(string.Format(System.Globalization.CultureInfo.InvariantCulture, SR.net_http_message_not_success_statuscode, (int)_statusCode,
+                throw new HttpRequestException(string.Format(
+                    System.Globalization.CultureInfo.InvariantCulture,
+                    SR.net_http_message_not_success_statuscode,
+                    (int)_statusCode,
                     ReasonPhrase));
             }
+
             return this;
         }
 

src/System.Net.Http/tests/FunctionalTests/HttpResponseMessageTest.cs

@@ -120,15 +120,8 @@ public void EnsureSuccessStatusCode_VariousStatusCodes_ThrowIfNot2xx()
         }
 
         [Fact]
-        public void EnsureSuccessStatusCode_AddContentToMessage_ContentIsDisposed()
+        public void EnsureSuccessStatusCode_SuccessStatusCode_ContentIsNotDisposed()
         {
-            using (var response404 = new HttpResponseMessage(HttpStatusCode.NotFound))
-            {
-                response404.Content = new MockContent();
-                Assert.Throws<HttpRequestException>(() => response404.EnsureSuccessStatusCode());
-                Assert.True((response404.Content as MockContent).IsDisposed);
-            }
-
             using (var response200 = new HttpResponseMessage(HttpStatusCode.OK))
             {
                 response200.Content = new MockContent();
@@ -137,6 +130,18 @@ public void EnsureSuccessStatusCode_AddContentToMessage_ContentIsDisposed()
             }
         }
 
+        [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework, "netfx disposes content when method throws.")]
+        [Fact]
+        public void EnsureSuccessStatusCode_NonSuccessStatusCode_ContentIsNotDisposed()
+        {
+            using (var response404 = new HttpResponseMessage(HttpStatusCode.NotFound))
+            {
+                response404.Content = new MockContent();
+                Assert.Throws<HttpRequestException>(() => response404.EnsureSuccessStatusCode());
+                Assert.False((response404.Content as MockContent).IsDisposed);
+            }
+        }
+
         [Fact]
         public void Properties_SetPropertiesAndGetTheirValue_MatchingValues()
         {