Skip to content

Commit

Permalink
Fix exception mapping during expect 100 cancellation. (#41800)
Browse files Browse the repository at this point in the history
  • Loading branch information
ManickaP authored Sep 9, 2020
1 parent c0b87c7 commit 287e168
Showing 1 changed file with 51 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -733,14 +733,22 @@ public async Task<HttpResponseMessage> SendAsyncCore(HttpRequestMessage request,
// We're awaiting the task to propagate the exception in this case.
if (Volatile.Read(ref _disposed) == Status_Disposed)
{
if (async)
try
{
await sendRequestContentTask.ConfigureAwait(false);
if (async)
{
await sendRequestContentTask.ConfigureAwait(false);
}
else
{
// No way around it here if we want to get the exception from the task.
sendRequestContentTask.GetAwaiter().GetResult();
}
}
else
// Map the exception the same way as we normally do.
catch (Exception ex) when (MapSendException(ex, cancellationToken, out Exception mappedEx))
{
// No way around it here if we want to get the exception from the task.
sendRequestContentTask.GetAwaiter().GetResult();
throw mappedEx;
}
}
LogExceptions(sendRequestContentTask);
Expand All @@ -751,42 +759,52 @@ public async Task<HttpResponseMessage> SendAsyncCore(HttpRequestMessage request,

// At this point, we're going to throw an exception; we just need to
// determine which exception to throw.

if (CancellationHelper.ShouldWrapInOperationCanceledException(error, cancellationToken))
{
// Cancellation was requested, so assume that the failure is due to
// the cancellation request. This is a bit unorthodox, as usually we'd
// prioritize a non-OperationCanceledException over a cancellation
// request to avoid losing potentially pertinent information. But given
// the cancellation design where we tear down the underlying connection upon
// a cancellation request, which can then result in a myriad of different
// exceptions (argument exceptions, object disposed exceptions, socket exceptions,
// etc.), as a middle ground we treat it as cancellation, but still propagate the
// original information as the inner exception, for diagnostic purposes.
throw CancellationHelper.CreateOperationCanceledException(error, cancellationToken);
}
else if (error is InvalidOperationException)
if (MapSendException(error, cancellationToken, out Exception mappedException))
{
// For consistency with other handlers we wrap the exception in an HttpRequestException.
throw new HttpRequestException(SR.net_http_client_execution_error, error);
}
else if (error is IOException ioe)
{
// For consistency with other handlers we wrap the exception in an HttpRequestException.
// If the request is retryable, indicate that on the exception.
throw new HttpRequestException(SR.net_http_client_execution_error, ioe, _canRetry ? RequestRetryType.RetryOnSameOrNextProxy : RequestRetryType.NoRetry);
}
else
{
// Otherwise, just allow the original exception to propagate.
throw;
throw mappedException;
}
// Otherwise, just allow the original exception to propagate.
throw;
}
}

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

private bool MapSendException(Exception exception, CancellationToken cancellationToken, out Exception mappedException)
{
if (CancellationHelper.ShouldWrapInOperationCanceledException(exception, cancellationToken))
{
// Cancellation was requested, so assume that the failure is due to
// the cancellation request. This is a bit unorthodox, as usually we'd
// prioritize a non-OperationCanceledException over a cancellation
// request to avoid losing potentially pertinent information. But given
// the cancellation design where we tear down the underlying connection upon
// a cancellation request, which can then result in a myriad of different
// exceptions (argument exceptions, object disposed exceptions, socket exceptions,
// etc.), as a middle ground we treat it as cancellation, but still propagate the
// original information as the inner exception, for diagnostic purposes.
mappedException = CancellationHelper.CreateOperationCanceledException(exception, cancellationToken);
return true;
}
if (exception is InvalidOperationException)
{
// For consistency with other handlers we wrap the exception in an HttpRequestException.
mappedException = new HttpRequestException(SR.net_http_client_execution_error, exception);
return true;
}
if (exception is IOException ioe)
{
// For consistency with other handlers we wrap the exception in an HttpRequestException.
// If the request is retryable, indicate that on the exception.
mappedException = new HttpRequestException(SR.net_http_client_execution_error, ioe, _canRetry ? RequestRetryType.RetryOnSameOrNextProxy : RequestRetryType.NoRetry);
return true;
}
// Otherwise, just allow the original exception to propagate.
mappedException = exception;
return false;
}

private HttpContentWriteStream CreateRequestContentStream(HttpRequestMessage request)
{
bool requestTransferEncodingChunked = request.HasHeaders && request.Headers.TransferEncodingChunked == true;
Expand Down

0 comments on commit 287e168

Please sign in to comment.