Skip to content

Commit

Permalink
React to HttpRequestError API changes (#89124)
Browse files Browse the repository at this point in the history 
* React to HttpRequestError API changes

* Use InvalidResponse for net_http_invalid_response_multiple_status_codes

* Use HttpRequestError.Unknown for the rest

* Add simple tests for the HttpRequestError property

* Fix the inner exception being removed by mistake
  • Loading branch information
@MihaZupan
MihaZupan committed Jul 18, 2023
1 parent 6cd933a commit c0d0b1a
Show file tree
Hide file tree
Showing 13 changed files with 75 additions and 58 deletions.
4 changes: 2 additions & 2 deletions src/libraries/System.Net.Http/ref/System.Net.Http.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -272,8 +272,8 @@ 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 HttpRequestException(string? message, System.Exception? inner = null, System.Net.HttpStatusCode? statusCode = null, System.Net.Http.HttpRequestError? httpRequestError = null) { }
public System.Net.Http.HttpRequestError? HttpRequestError { get { throw null; } }
public HttpRequestException(System.Net.Http.HttpRequestError httpRequestError, string? message = null, System.Exception? inner = null, System.Net.HttpStatusCode? statusCode = null) { }
public System.Net.Http.HttpRequestError HttpRequestError { get { throw null; } }
public System.Net.HttpStatusCode? StatusCode { get { throw null; } }
}
public partial class HttpRequestMessage : System.IDisposable
Expand Down
7 changes: 4 additions & 3 deletions src/libraries/System.Net.Http/src/System/Net/Http/HttpContent.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.Buffers;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Globalization;
using System.IO;
using System.Net.Http.Headers;
using System.Text;
Expand Down Expand Up @@ -608,7 +609,7 @@ private bool CreateTemporaryBuffer(long maxBufferSize, out MemoryStream? tempBuf
// This should only be hit when called directly; HttpClient/HttpClientHandler
// will not exceed this limit.
throw new ArgumentOutOfRangeException(nameof(maxBufferSize), maxBufferSize,
SR.Format(System.Globalization.CultureInfo.InvariantCulture,
SR.Format(CultureInfo.InvariantCulture,
SR.net_http_content_buffersize_limit, HttpContent.MaxBufferSize));
}

Expand Down Expand Up @@ -720,7 +721,7 @@ internal static Exception WrapStreamCopyException(Exception e)
{
Debug.Assert(StreamCopyExceptionNeedsWrapping(e));
HttpRequestError error = e is HttpIOException ioEx ? ioEx.HttpRequestError : HttpRequestError.Unknown;
return new HttpRequestException(SR.net_http_content_stream_copy_error, e, httpRequestError: error);
return new HttpRequestException(error, SR.net_http_content_stream_copy_error, e);
}

private static int GetPreambleLength(ArraySegment<byte> buffer, Encoding encoding)
Expand Down Expand Up @@ -835,7 +836,7 @@ private static async Task<TResult> WaitAndReturnAsync<TState, TResult>(Task wait

private static HttpRequestException CreateOverCapacityException(long maxBufferSize)
{
return new HttpRequestException(SR.Format(System.Globalization.CultureInfo.InvariantCulture, SR.net_http_content_buffersize_exceeded, maxBufferSize), httpRequestError: HttpRequestError.ConfigurationLimitExceeded);
return new HttpRequestException(HttpRequestError.ConfigurationLimitExceeded, SR.Format(CultureInfo.InvariantCulture, SR.net_http_content_buffersize_exceeded, maxBufferSize));
}

internal sealed class LimitMemoryStream : MemoryStream
Expand Down
13 changes: 5 additions & 8 deletions src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestException.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,11 @@ public HttpRequestException(string? message, Exception? inner, HttpStatusCode? s
/// <summary>
/// Initializes a new instance of the <see cref="HttpRequestException" /> class with a specific message an inner exception, and an HTTP status code and an <see cref="HttpRequestError"/>.
/// </summary>
/// <param name="httpRequestError">The <see cref="HttpRequestError"/> that caused the exception.</param>
/// <param name="message">A message that describes the current exception.</param>
/// <param name="inner">The inner exception.</param>
/// <param name="statusCode">The HTTP status code.</param>
/// <param name="httpRequestError">The <see cref="HttpRequestError"/> that caused the exception.</param>
public HttpRequestException(string? message, Exception? inner = null, HttpStatusCode? statusCode = null, HttpRequestError? httpRequestError = null)
public HttpRequestException(HttpRequestError httpRequestError, string? message = null, Exception? inner = null, HttpStatusCode? statusCode = null)
: this(message, inner, statusCode)
{
HttpRequestError = httpRequestError;
Expand All @@ -51,10 +51,7 @@ public HttpRequestException(string? message, Exception? inner = null, HttpStatus
/// <summary>
/// Gets the <see cref="Http.HttpRequestError"/> that caused the exception.
/// </summary>
/// <value>
/// The <see cref="Http.HttpRequestError"/> or <see langword="null"/> if the underlying <see cref="HttpMessageHandler"/> did not provide it.
/// </value>
public HttpRequestError? HttpRequestError { get; }
public HttpRequestError HttpRequestError { get; }

/// <summary>
/// Gets the HTTP status code to be returned with the exception.
Expand All @@ -66,8 +63,8 @@ public HttpRequestException(string? message, Exception? inner = null, HttpStatus

// 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, HttpRequestError? httpRequestError = null)
: this(message, inner, httpRequestError: httpRequestError)
internal HttpRequestException(HttpRequestError httpRequestError, string? message, Exception? inner, RequestRetryType allowRetry)
: this(httpRequestError, message, inner)
{
AllowRetry = allowRetry;
}
Expand Down
2 changes: 1 addition & 1 deletion ...ies/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/AuthenticationHelper.NtAuth.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ private static async Task<HttpResponseMessage> SendWithNtAuthAsync(HttpRequestMe
{
isNewConnection = false;
connection.Dispose();
throw new HttpRequestException(SR.Format(SR.net_http_authvalidationfailure, statusCode), null, HttpStatusCode.Unauthorized, HttpRequestError.UserAuthenticationError);
throw new HttpRequestException(HttpRequestError.UserAuthenticationError, SR.Format(SR.net_http_authvalidationfailure, statusCode), statusCode: HttpStatusCode.Unauthorized);
}
break;
}
Expand Down
4 changes: 2 additions & 2 deletions src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectHelper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ public static async ValueTask<SslStream> EstablishSslConnectionAsync(SslClientAu
throw CancellationHelper.CreateOperationCanceledException(e, cancellationToken);
}

HttpRequestException ex = new HttpRequestException(SR.net_http_ssl_connection_failed, e, httpRequestError: HttpRequestError.SecureConnectionError);
HttpRequestException ex = new HttpRequestException(HttpRequestError.SecureConnectionError, SR.net_http_ssl_connection_failed, e);
if (request.IsExtendedConnectRequest)
{
// Extended connect request is negotiating strictly for ALPN = "h2" because HttpClient is unaware of a possible downgrade.
Expand Down Expand Up @@ -139,7 +139,7 @@ internal static Exception CreateWrappedException(Exception exception, string hos
{
return CancellationHelper.ShouldWrapInOperationCanceledException(exception, cancellationToken) ?
CancellationHelper.CreateOperationCanceledException(exception, cancellationToken) :
new HttpRequestException($"{exception.Message} ({host}:{port})", exception, RequestRetryType.RetryOnNextProxy, DeduceError(exception));
new HttpRequestException(DeduceError(exception), $"{exception.Message} ({host}:{port})", exception, RequestRetryType.RetryOnNextProxy);

static HttpRequestError DeduceError(Exception exception)
{
Expand Down
6 changes: 3 additions & 3 deletions src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2104,11 +2104,11 @@ await Task.WhenAny(requestBodyTask, responseHeadersTask).ConfigureAwait(false) =
}
catch (HttpIOException e)
{
throw new HttpRequestException(e.Message, e, httpRequestError: e.HttpRequestError);
throw new HttpRequestException(e.HttpRequestError, e.Message, e);
}
catch (Exception e) when (e is IOException || e is ObjectDisposedException || e is InvalidOperationException)
{
throw new HttpRequestException(SR.net_http_client_execution_error, e, httpRequestError: HttpRequestError.Unknown);
throw new HttpRequestException(HttpRequestError.Unknown, SR.net_http_client_execution_error, e);
}
}

Expand Down Expand Up @@ -2205,7 +2205,7 @@ internal void Trace(int streamId, string message, [CallerMemberName] string? mem

[DoesNotReturn]
private static void ThrowRetry(string message, Exception? innerException = null) =>
throw new HttpRequestException(message, innerException, allowRetry: RequestRetryType.RetryOnConnectionFailure);
throw new HttpRequestException(HttpRequestError.Unknown, message, innerException, RequestRetryType.RetryOnConnectionFailure);

private static Exception GetRequestAbortedException(Exception? innerException = null) =>
innerException as HttpIOException ?? new IOException(SR.net_http_request_aborted, innerException);
Expand Down
16 changes: 8 additions & 8 deletions src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,7 @@ void IHttpStreamHeadersHandler.OnStaticIndexedHeader(int index)
if (index <= LastHPackRequestPseudoHeaderId)
{
if (NetEventSource.Log.IsEnabled()) Trace($"Invalid request pseudo-header ID {index}.");
throw new HttpRequestException(SR.net_http_invalid_response, httpRequestError: HttpRequestError.InvalidResponse);
throw new HttpRequestException(HttpRequestError.InvalidResponse, SR.net_http_invalid_response);
}
else if (index <= LastHPackStatusPseudoHeaderId)
{
Expand All @@ -563,7 +563,7 @@ void IHttpStreamHeadersHandler.OnStaticIndexedHeader(int index, ReadOnlySpan<byt
if (index <= LastHPackRequestPseudoHeaderId)
{
if (NetEventSource.Log.IsEnabled()) Trace($"Invalid request pseudo-header ID {index}.");
throw new HttpRequestException(SR.net_http_invalid_response, httpRequestError: HttpRequestError.InvalidResponse);
throw new HttpRequestException(HttpRequestError.InvalidResponse, SR.net_http_invalid_response);
}
else if (index <= LastHPackStatusPseudoHeaderId)
{
Expand All @@ -589,7 +589,7 @@ private void AdjustHeaderBudget(int amount)
_headerBudgetRemaining -= amount;
if (_headerBudgetRemaining < 0)
{
throw new HttpRequestException(SR.Format(SR.net_http_response_headers_exceeded_length, _connection._pool.Settings.MaxResponseHeadersByteLength), httpRequestError: HttpRequestError.ConfigurationLimitExceeded);
throw new HttpRequestException(HttpRequestError.ConfigurationLimitExceeded, SR.Format(SR.net_http_response_headers_exceeded_length, _connection._pool.Settings.MaxResponseHeadersByteLength));
}
}

Expand All @@ -611,14 +611,14 @@ private void OnStatus(int statusCode)
if (_responseProtocolState == ResponseProtocolState.ExpectingHeaders)
{
if (NetEventSource.Log.IsEnabled()) Trace("Received extra status header.");
throw new HttpRequestException(SR.net_http_invalid_response_multiple_status_codes, httpRequestError: HttpRequestError.ConfigurationLimitExceeded);
throw new HttpRequestException(HttpRequestError.InvalidResponse, SR.net_http_invalid_response_multiple_status_codes);
}

if (_responseProtocolState != ResponseProtocolState.ExpectingStatus)
{
// Pseudo-headers are allowed only in header block
if (NetEventSource.Log.IsEnabled()) Trace($"Status pseudo-header received in {_responseProtocolState} state.");
throw new HttpRequestException(SR.net_http_invalid_response_pseudo_header_in_trailer, httpRequestError: HttpRequestError.InvalidResponse);
throw new HttpRequestException(HttpRequestError.InvalidResponse, SR.net_http_invalid_response_pseudo_header_in_trailer);
}

Debug.Assert(_response != null);
Expand Down Expand Up @@ -681,7 +681,7 @@ private void OnHeader(HeaderDescriptor descriptor, ReadOnlySpan<byte> value)
if (_responseProtocolState != ResponseProtocolState.ExpectingHeaders && _responseProtocolState != ResponseProtocolState.ExpectingTrailingHeaders)
{
if (NetEventSource.Log.IsEnabled()) Trace("Received header before status.");
throw new HttpRequestException(SR.net_http_invalid_response, httpRequestError: HttpRequestError.InvalidResponse);
throw new HttpRequestException(HttpRequestError.InvalidResponse, SR.net_http_invalid_response);
}

Encoding? valueEncoding = _connection._pool.Settings._responseHeaderEncodingSelector?.Invoke(descriptor.Name, _request);
Expand Down Expand Up @@ -725,7 +725,7 @@ public void OnHeader(ReadOnlySpan<byte> name, ReadOnlySpan<byte> value)
else
{
if (NetEventSource.Log.IsEnabled()) Trace($"Invalid response pseudo-header '{Encoding.ASCII.GetString(name)}'.");
throw new HttpRequestException(SR.net_http_invalid_response, httpRequestError: HttpRequestError.InvalidResponse);
throw new HttpRequestException(HttpRequestError.InvalidResponse, SR.net_http_invalid_response);
}
}
else
Expand All @@ -734,7 +734,7 @@ public void OnHeader(ReadOnlySpan<byte> name, ReadOnlySpan<byte> value)
if (!HeaderDescriptor.TryGet(name, out HeaderDescriptor descriptor))
{
// Invalid header name
throw new HttpRequestException(SR.Format(SR.net_http_invalid_response_header_name, Encoding.ASCII.GetString(name)), httpRequestError: HttpRequestError.InvalidResponse);
throw new HttpRequestException(HttpRequestError.InvalidResponse, SR.Format(SR.net_http_invalid_response_header_name, Encoding.ASCII.GetString(name)));
}

OnHeader(descriptor, value);
Expand Down
6 changes: 3 additions & 3 deletions src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ public async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, lon

if (quicStream == null)
{
throw new HttpRequestException(SR.net_http_request_aborted, null, RequestRetryType.RetryOnConnectionFailure);
throw new HttpRequestException(HttpRequestError.Unknown, SR.net_http_request_aborted, null, RequestRetryType.RetryOnConnectionFailure);
}

requestStream!.StreamId = quicStream.Id;
Expand All @@ -221,7 +221,7 @@ public async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, lon

if (goAway)
{
throw new HttpRequestException(SR.net_http_request_aborted, null, RequestRetryType.RetryOnConnectionFailure);
throw new HttpRequestException(HttpRequestError.Unknown, SR.net_http_request_aborted, null, RequestRetryType.RetryOnConnectionFailure);
}

if (NetEventSource.Log.IsEnabled()) Trace($"Sending request: {request}");
Expand All @@ -237,7 +237,7 @@ public async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, lon
{
// This will happen if we aborted _connection somewhere and we have pending OpenOutboundStreamAsync call.
// note that _abortException may be null if we closed the connection in response to a GOAWAY frame
throw new HttpRequestException(SR.net_http_client_execution_error, _abortException, RequestRetryType.RetryOnConnectionFailure);
throw new HttpRequestException(HttpRequestError.Unknown, SR.net_http_client_execution_error, _abortException, RequestRetryType.RetryOnConnectionFailure);
}
finally
{
Expand Down
Loading

0 comments on commit c0d0b1a

Please sign in to comment.