From 33f988a58a0afc7640cb5222209bace72535ff3f Mon Sep 17 00:00:00 2001 From: Yaakov Smith Date: Tue, 18 Feb 2020 09:33:20 +1100 Subject: [PATCH 01/12] Add initial tests --- .../UnitTests/HttpRequestExceptionTests.cs | 39 +++++++++++++++++++ .../System.Net.Http.Unit.Tests.csproj | 1 + 2 files changed, 40 insertions(+) create mode 100644 src/libraries/System.Net.Http/tests/UnitTests/HttpRequestExceptionTests.cs diff --git a/src/libraries/System.Net.Http/tests/UnitTests/HttpRequestExceptionTests.cs b/src/libraries/System.Net.Http/tests/UnitTests/HttpRequestExceptionTests.cs new file mode 100644 index 00000000000000..eb3a820717e7e1 --- /dev/null +++ b/src/libraries/System.Net.Http/tests/UnitTests/HttpRequestExceptionTests.cs @@ -0,0 +1,39 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System.Collections.Generic; +using System.Diagnostics; +using System.Net.Http; +using Microsoft.DotNet.RemoteExecutor; +using Xunit; +using Xunit.Abstractions; + +namespace System.Net.Http.Tests +{ + public class HttpRequestExceptionTests + { + [Fact] + public void DefaultConstructors_HasNoStatusCode() + { + var exception = new HttpRequestException(); + Assert.Null(exception.StatusCode); + } + + [Fact] + public void StoresStatusCode() + { + var exception = new HttpRequestException("message", null, HttpStatusCode.InternalServerError); + Assert.Equal(HttpStatusCode.InternalServerError, exception.StatusCode); + } + + [Fact] + public void StoresNonStandardStatusCode() + { + var statusCode = (HttpStatusCode)999; + + var exception = new HttpRequestException("message", null, statusCode); + Assert.Equal(statusCode, exception.StatusCode); + } + } +} \ No newline at end of file diff --git a/src/libraries/System.Net.Http/tests/UnitTests/System.Net.Http.Unit.Tests.csproj b/src/libraries/System.Net.Http/tests/UnitTests/System.Net.Http.Unit.Tests.csproj index 29595a25fede71..fff618fc14b6f7 100644 --- a/src/libraries/System.Net.Http/tests/UnitTests/System.Net.Http.Unit.Tests.csproj +++ b/src/libraries/System.Net.Http/tests/UnitTests/System.Net.Http.Unit.Tests.csproj @@ -415,6 +415,7 @@ + From 1b4bc7e729b9e1579891fcf593025a187f7a6b5b Mon Sep 17 00:00:00 2001 From: Yaakov Smith Date: Tue, 18 Feb 2020 09:35:06 +1100 Subject: [PATCH 02/12] Add new constructor and property --- .../src/System/Net/Http/HttpRequestException.cs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestException.cs b/src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestException.cs index 68662c0f47aa01..06e4ef4ed18fa9 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestException.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestException.cs @@ -28,6 +28,14 @@ public HttpRequestException(string message, Exception inner) } } + public HttpRequestException(string message, Exception inner, HttpStatusCode? statusCode) + : this(message, inner) + { + StatusCode = statusCode; + } + + public HttpStatusCode? StatusCode { get; } + // This constructor is used internally to indicate that a request was not successfully sent due to an IOException, // and the exception occurred early enough so that the request may be retried on another connection. internal HttpRequestException(string message, Exception inner, RequestRetryType allowRetry) From ae3f381247b81788a2b46c4a20781d11527db2b0 Mon Sep 17 00:00:00 2001 From: Yaakov Smith Date: Tue, 18 Feb 2020 09:41:15 +1100 Subject: [PATCH 03/12] Fix style error (trailing whitespace) --- .../System.Net.Http/src/System/Net/Http/HttpRequestException.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestException.cs b/src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestException.cs index 06e4ef4ed18fa9..dda73dbb0f5b9d 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestException.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestException.cs @@ -31,7 +31,7 @@ public HttpRequestException(string message, Exception inner) public HttpRequestException(string message, Exception inner, HttpStatusCode? statusCode) : this(message, inner) { - StatusCode = statusCode; + StatusCode = statusCode; } public HttpStatusCode? StatusCode { get; } From 118072bef17bbf7f92a8e1b455e59d4129ece340 Mon Sep 17 00:00:00 2001 From: Yaakov Smith Date: Tue, 18 Feb 2020 09:41:28 +1100 Subject: [PATCH 04/12] GenerateReferenceSource --- src/libraries/System.Net.Http/ref/System.Net.Http.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Net.Http/ref/System.Net.Http.cs b/src/libraries/System.Net.Http/ref/System.Net.Http.cs index ebbc2c0bb0b088..b1c02bb8efe1da 100644 --- a/src/libraries/System.Net.Http/ref/System.Net.Http.cs +++ b/src/libraries/System.Net.Http/ref/System.Net.Http.cs @@ -128,9 +128,9 @@ public abstract partial class HttpContent : System.IDisposable protected HttpContent() { } public System.Net.Http.Headers.HttpContentHeaders Headers { get { throw null; } } public System.Threading.Tasks.Task CopyToAsync(System.IO.Stream stream) { throw null; } - public System.Threading.Tasks.Task CopyToAsync(System.IO.Stream stream, System.Threading.CancellationToken cancellationToken) { throw null; } public System.Threading.Tasks.Task CopyToAsync(System.IO.Stream stream, System.Net.TransportContext context) { throw null; } public System.Threading.Tasks.Task CopyToAsync(System.IO.Stream stream, System.Net.TransportContext context, System.Threading.CancellationToken cancellationToken) { throw null; } + public System.Threading.Tasks.Task CopyToAsync(System.IO.Stream stream, System.Threading.CancellationToken cancellationToken) { throw null; } protected virtual System.Threading.Tasks.Task CreateContentReadStreamAsync() { throw null; } protected virtual System.Threading.Tasks.Task CreateContentReadStreamAsync(System.Threading.CancellationToken cancellationToken) { throw null; } public void Dispose() { } @@ -186,6 +186,8 @@ public partial class HttpRequestException : System.Exception public HttpRequestException() { } public HttpRequestException(string message) { } public HttpRequestException(string message, System.Exception inner) { } + public HttpRequestException(string message, System.Exception inner, System.Net.HttpStatusCode? statusCode) { } + public System.Net.HttpStatusCode? StatusCode { get { throw null; } } } public partial class HttpRequestMessage : System.IDisposable { From 743b772e9b37d4ca3fa604b46fca68e4704f279f Mon Sep 17 00:00:00 2001 From: Yaakov Smith Date: Tue, 18 Feb 2020 09:41:52 +1100 Subject: [PATCH 05/12] Add tests for remaining pre-existing constructors --- .../tests/UnitTests/HttpRequestExceptionTests.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/libraries/System.Net.Http/tests/UnitTests/HttpRequestExceptionTests.cs b/src/libraries/System.Net.Http/tests/UnitTests/HttpRequestExceptionTests.cs index eb3a820717e7e1..e66eab6f15d0f9 100644 --- a/src/libraries/System.Net.Http/tests/UnitTests/HttpRequestExceptionTests.cs +++ b/src/libraries/System.Net.Http/tests/UnitTests/HttpRequestExceptionTests.cs @@ -18,6 +18,12 @@ public void DefaultConstructors_HasNoStatusCode() { var exception = new HttpRequestException(); Assert.Null(exception.StatusCode); + + exception = new HttpRequestException("message"); + Assert.Null(exception.StatusCode); + + exception = new HttpRequestException("message", new InvalidOperationException()); + Assert.Null(exception.StatusCode); } [Fact] From d88193535be2e5766d66b1887db76e46add992fe Mon Sep 17 00:00:00 2001 From: Yaakov Smith Date: Tue, 18 Feb 2020 09:47:30 +1100 Subject: [PATCH 06/12] Update tests for EnsureSuccessStatusCode --- .../tests/FunctionalTests/HttpResponseMessageTest.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpResponseMessageTest.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpResponseMessageTest.cs index 549a33c4a7e4a9..fdeeeb065ca236 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpResponseMessageTest.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpResponseMessageTest.cs @@ -105,12 +105,14 @@ public void EnsureSuccessStatusCode_VariousStatusCodes_ThrowIfNot2xx() { using (var m = new HttpResponseMessage(HttpStatusCode.MultipleChoices)) { - Assert.Throws(() => m.EnsureSuccessStatusCode()); + var ex = Assert.Throws(() => m.EnsureSuccessStatusCode()); + Assert.Equal(HttpStatusCode.MultipleChoices, ex.StatusCode); } using (var m = new HttpResponseMessage(HttpStatusCode.BadGateway)) { - Assert.Throws(() => m.EnsureSuccessStatusCode()); + var ex = Assert.Throws(() => m.EnsureSuccessStatusCode()); + Assert.Equal(HttpStatusCode.BadGateway, ex.StatusCode); } using (var response = new HttpResponseMessage(HttpStatusCode.OK)) From 6b27a457f52d879d8ef6ad842930550f27562ce3 Mon Sep 17 00:00:00 2001 From: Yaakov Smith Date: Tue, 18 Feb 2020 09:51:10 +1100 Subject: [PATCH 07/12] Include status code in exception from EnsureSuccessStatusCode --- .../src/System/Net/Http/HttpResponseMessage.cs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/HttpResponseMessage.cs b/src/libraries/System.Net.Http/src/System/Net/Http/HttpResponseMessage.cs index 0b2e4aea1528f5..4b9f0d6813c868 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/HttpResponseMessage.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/HttpResponseMessage.cs @@ -169,11 +169,14 @@ public HttpResponseMessage EnsureSuccessStatusCode() { if (!IsSuccessStatusCode) { - throw new HttpRequestException(SR.Format( - System.Globalization.CultureInfo.InvariantCulture, - SR.net_http_message_not_success_statuscode, - (int)_statusCode, - ReasonPhrase)); + throw new HttpRequestException( + SR.Format( + System.Globalization.CultureInfo.InvariantCulture, + SR.net_http_message_not_success_statuscode, + (int)_statusCode, + ReasonPhrase), + null, + _statusCode); } return this; From 023fb4e9478e4dfaaa9e324a1ddecf48f0841fe8 Mon Sep 17 00:00:00 2001 From: Yaakov Smith Date: Tue, 18 Feb 2020 15:25:01 +1100 Subject: [PATCH 08/12] Remove unused usings from test code. --- .../tests/UnitTests/HttpRequestExceptionTests.cs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/libraries/System.Net.Http/tests/UnitTests/HttpRequestExceptionTests.cs b/src/libraries/System.Net.Http/tests/UnitTests/HttpRequestExceptionTests.cs index e66eab6f15d0f9..4500e51de7a5be 100644 --- a/src/libraries/System.Net.Http/tests/UnitTests/HttpRequestExceptionTests.cs +++ b/src/libraries/System.Net.Http/tests/UnitTests/HttpRequestExceptionTests.cs @@ -2,12 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using System.Collections.Generic; -using System.Diagnostics; -using System.Net.Http; -using Microsoft.DotNet.RemoteExecutor; using Xunit; -using Xunit.Abstractions; namespace System.Net.Http.Tests { @@ -42,4 +37,4 @@ public void StoresNonStandardStatusCode() Assert.Equal(statusCode, exception.StatusCode); } } -} \ No newline at end of file +} From c93027787270c22d5f529627f8cc8acad87e23aa Mon Sep 17 00:00:00 2001 From: Yaakov Smith Date: Tue, 18 Feb 2020 15:33:36 +1100 Subject: [PATCH 09/12] Add xmldoc to new members --- .../src/System/Net/Http/HttpRequestException.cs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestException.cs b/src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestException.cs index dda73dbb0f5b9d..0245169a348fbb 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestException.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestException.cs @@ -28,12 +28,21 @@ public HttpRequestException(string message, Exception inner) } } + /// + /// Initializes a new instance of the class with a specific message that describes the current exception, an inner exception, and an HTTP status code. + /// + /// A message that describes the current exception. + /// The inner exception. + /// The HTTP status code. public HttpRequestException(string message, Exception inner, HttpStatusCode? statusCode) : this(message, inner) { StatusCode = statusCode; } + /// + /// The HTTP status code to be returned with the exception. + /// public HttpStatusCode? StatusCode { get; } // This constructor is used internally to indicate that a request was not successfully sent due to an IOException, From 33bfd8ca9102cad64ece4f3a0ce9ffafaf0f45e3 Mon Sep 17 00:00:00 2001 From: Yaakov Date: Wed, 19 Feb 2020 09:51:37 +1100 Subject: [PATCH 10/12] Update src/libraries/System.Net.Http/src/System/Net/Http/HttpResponseMessage.cs Co-Authored-By: Cory Nelson --- .../System.Net.Http/src/System/Net/Http/HttpResponseMessage.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/HttpResponseMessage.cs b/src/libraries/System.Net.Http/src/System/Net/Http/HttpResponseMessage.cs index 4b9f0d6813c868..d52f14a2a98af5 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/HttpResponseMessage.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/HttpResponseMessage.cs @@ -175,7 +175,7 @@ public HttpResponseMessage EnsureSuccessStatusCode() SR.net_http_message_not_success_statuscode, (int)_statusCode, ReasonPhrase), - null, + inner: null, _statusCode); } From e6644e03a4706241661ce6d2b252559691237b28 Mon Sep 17 00:00:00 2001 From: Yaakov Date: Wed, 19 Feb 2020 18:10:42 +1100 Subject: [PATCH 11/12] Update exception doc comments --- .../src/System/Net/Http/HttpRequestException.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestException.cs b/src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestException.cs index 0245169a348fbb..da0bdf64033e21 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestException.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestException.cs @@ -41,8 +41,11 @@ public HttpRequestException(string message, Exception inner, HttpStatusCode? sta } /// - /// The HTTP status code to be returned with the exception. + /// Gets the HTTP status code to be returned with the exception. /// + /// + /// An HTTP status code if the exception represents a non-successful result, otherwise null. + /// public HttpStatusCode? StatusCode { get; } // This constructor is used internally to indicate that a request was not successfully sent due to an IOException, From 9d26f7fe247bd0f8ec0774166fa3526bc094d740 Mon Sep 17 00:00:00 2001 From: Yaakov Date: Wed, 19 Feb 2020 18:55:38 +1100 Subject: [PATCH 12/12] Test for status code in existing convenience helpers --- .../tests/FunctionalTests/HttpClientTest.cs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientTest.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientTest.cs index 83efb460d574bc..f7fff196ea6c00 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientTest.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientTest.cs @@ -192,9 +192,16 @@ public async Task GetContentAsync_ErrorStatusCode_ExpectedExceptionThrown(bool w Content = withResponseContent ? new ByteArrayContent(new byte[1]) : null })))) { - await Assert.ThrowsAsync(() => client.GetStringAsync(CreateFakeUri())); - await Assert.ThrowsAsync(() => client.GetByteArrayAsync(CreateFakeUri())); - await Assert.ThrowsAsync(() => client.GetStreamAsync(CreateFakeUri())); + HttpRequestException ex; + + ex = await Assert.ThrowsAsync(() => client.GetStringAsync(CreateFakeUri())); + Assert.Equal(HttpStatusCode.BadRequest, ex.StatusCode); + + ex = await Assert.ThrowsAsync(() => client.GetByteArrayAsync(CreateFakeUri())); + Assert.Equal(HttpStatusCode.BadRequest, ex.StatusCode); + + ex = await Assert.ThrowsAsync(() => client.GetStreamAsync(CreateFakeUri())); + Assert.Equal(HttpStatusCode.BadRequest, ex.StatusCode); } }