Skip to content

Commit 287e168

Browse files
authored
Fix exception mapping during expect 100 cancellation. (#41800)
1 parent c0b87c7 commit 287e168

File tree

1 file changed

+51
-33
lines changed
  • src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler

1 file changed

+51
-33
lines changed

src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs

Lines changed: 51 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -733,14 +733,22 @@ public async Task<HttpResponseMessage> SendAsyncCore(HttpRequestMessage request,
733733
// We're awaiting the task to propagate the exception in this case.
734734
if (Volatile.Read(ref _disposed) == Status_Disposed)
735735
{
736-
if (async)
736+
try
737737
{
738-
await sendRequestContentTask.ConfigureAwait(false);
738+
if (async)
739+
{
740+
await sendRequestContentTask.ConfigureAwait(false);
741+
}
742+
else
743+
{
744+
// No way around it here if we want to get the exception from the task.
745+
sendRequestContentTask.GetAwaiter().GetResult();
746+
}
739747
}
740-
else
748+
// Map the exception the same way as we normally do.
749+
catch (Exception ex) when (MapSendException(ex, cancellationToken, out Exception mappedEx))
741750
{
742-
// No way around it here if we want to get the exception from the task.
743-
sendRequestContentTask.GetAwaiter().GetResult();
751+
throw mappedEx;
744752
}
745753
}
746754
LogExceptions(sendRequestContentTask);
@@ -751,42 +759,52 @@ public async Task<HttpResponseMessage> SendAsyncCore(HttpRequestMessage request,
751759

752760
// At this point, we're going to throw an exception; we just need to
753761
// determine which exception to throw.
754-
755-
if (CancellationHelper.ShouldWrapInOperationCanceledException(error, cancellationToken))
756-
{
757-
// Cancellation was requested, so assume that the failure is due to
758-
// the cancellation request. This is a bit unorthodox, as usually we'd
759-
// prioritize a non-OperationCanceledException over a cancellation
760-
// request to avoid losing potentially pertinent information. But given
761-
// the cancellation design where we tear down the underlying connection upon
762-
// a cancellation request, which can then result in a myriad of different
763-
// exceptions (argument exceptions, object disposed exceptions, socket exceptions,
764-
// etc.), as a middle ground we treat it as cancellation, but still propagate the
765-
// original information as the inner exception, for diagnostic purposes.
766-
throw CancellationHelper.CreateOperationCanceledException(error, cancellationToken);
767-
}
768-
else if (error is InvalidOperationException)
762+
if (MapSendException(error, cancellationToken, out Exception mappedException))
769763
{
770-
// For consistency with other handlers we wrap the exception in an HttpRequestException.
771-
throw new HttpRequestException(SR.net_http_client_execution_error, error);
772-
}
773-
else if (error is IOException ioe)
774-
{
775-
// For consistency with other handlers we wrap the exception in an HttpRequestException.
776-
// If the request is retryable, indicate that on the exception.
777-
throw new HttpRequestException(SR.net_http_client_execution_error, ioe, _canRetry ? RequestRetryType.RetryOnSameOrNextProxy : RequestRetryType.NoRetry);
778-
}
779-
else
780-
{
781-
// Otherwise, just allow the original exception to propagate.
782-
throw;
764+
throw mappedException;
783765
}
766+
// Otherwise, just allow the original exception to propagate.
767+
throw;
784768
}
785769
}
786770

787771
public sealed override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, bool async, CancellationToken cancellationToken) =>
788772
SendAsyncCore(request, async, cancellationToken);
789773

774+
private bool MapSendException(Exception exception, CancellationToken cancellationToken, out Exception mappedException)
775+
{
776+
if (CancellationHelper.ShouldWrapInOperationCanceledException(exception, cancellationToken))
777+
{
778+
// Cancellation was requested, so assume that the failure is due to
779+
// the cancellation request. This is a bit unorthodox, as usually we'd
780+
// prioritize a non-OperationCanceledException over a cancellation
781+
// request to avoid losing potentially pertinent information. But given
782+
// the cancellation design where we tear down the underlying connection upon
783+
// a cancellation request, which can then result in a myriad of different
784+
// exceptions (argument exceptions, object disposed exceptions, socket exceptions,
785+
// etc.), as a middle ground we treat it as cancellation, but still propagate the
786+
// original information as the inner exception, for diagnostic purposes.
787+
mappedException = CancellationHelper.CreateOperationCanceledException(exception, cancellationToken);
788+
return true;
789+
}
790+
if (exception is InvalidOperationException)
791+
{
792+
// For consistency with other handlers we wrap the exception in an HttpRequestException.
793+
mappedException = new HttpRequestException(SR.net_http_client_execution_error, exception);
794+
return true;
795+
}
796+
if (exception is IOException ioe)
797+
{
798+
// For consistency with other handlers we wrap the exception in an HttpRequestException.
799+
// If the request is retryable, indicate that on the exception.
800+
mappedException = new HttpRequestException(SR.net_http_client_execution_error, ioe, _canRetry ? RequestRetryType.RetryOnSameOrNextProxy : RequestRetryType.NoRetry);
801+
return true;
802+
}
803+
// Otherwise, just allow the original exception to propagate.
804+
mappedException = exception;
805+
return false;
806+
}
807+
790808
private HttpContentWriteStream CreateRequestContentStream(HttpRequestMessage request)
791809
{
792810
bool requestTransferEncodingChunked = request.HasHeaders && request.Headers.TransferEncodingChunked == true;

0 commit comments

Comments
 (0)