Skip to content

Commit

Permalink
Avoid adding an HttpClient.Timeout message to ConnectTimeout exceptio…
Browse files Browse the repository at this point in the history
…ns (#72274)

* Avoid adding an HttpClient.Timeout message to ConnectTimeout exceptions

* Avoid wrapping unknown OCEs as well

* Update comment

Co-authored-by: Natalia Kondratyeva <knatalia@microsoft.com>

Co-authored-by: Natalia Kondratyeva <knatalia@microsoft.com>
  • Loading branch information
MihaZupan and CarnaViire committed Jul 19, 2022
1 parent 779779a commit bf7cdc5
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 3 deletions.
13 changes: 10 additions & 3 deletions src/libraries/System.Net.Http/src/System/Net/Http/HttpClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -604,11 +604,18 @@ private void HandleFailure(Exception e, bool telemetryStarted, HttpResponseMessa
e = toThrow = new TaskCanceledException(oce.Message, oce.InnerException, cancellationToken);
}
}
else if (!pendingRequestsCts.IsCancellationRequested)
else if (cts.IsCancellationRequested && !pendingRequestsCts.IsCancellationRequested)
{
// If this exception is for cancellation, but cancellation wasn't requested, either by the caller's token or by the pending requests source,
// If the linked cancellation token source was canceled, but cancellation wasn't requested, either by the caller's token or by the pending requests source,
// the only other cause could be a timeout. Treat it as such.
e = toThrow = new TaskCanceledException(SR.Format(SR.net_http_request_timedout, _timeout.TotalSeconds), new TimeoutException(e.Message, e), oce.CancellationToken);

// cancellationToken could have been triggered right after we checked it, but before we checked the cts.
// We must check it again to avoid reporting a timeout when one did not occur.
if (!cancellationToken.IsCancellationRequested)
{
Debug.Assert(_timeout.TotalSeconds > 0);
e = toThrow = new TaskCanceledException(SR.Format(SR.net_http_request_timedout, _timeout.TotalSeconds), new TimeoutException(e.Message, e), oce.CancellationToken);
}
}
}
else if (e is HttpRequestException && cts.IsCancellationRequested) // if cancellationToken is canceled, cts will also be canceled
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -798,6 +798,48 @@ public async Task Timeout_SetTo30AndGetResponseQuickly_Success()
}
}

[ConditionalFact(typeof(SocketsHttpHandler), nameof(SocketsHttpHandler.IsSupported))]
public async Task ConnectTimeout_NotWrappedInMultipleTimeoutExceptions()
{
using var handler = CreateHttpClientHandler();

SocketsHttpHandler socketsHttpHandler = GetUnderlyingSocketsHttpHandler(handler);

socketsHttpHandler.ConnectTimeout = TimeSpan.FromMilliseconds(1);
socketsHttpHandler.ConnectCallback = async (context, cancellation) =>
{
await Task.Delay(-1, cancellation);
throw new UnreachableException();
};

using var client = CreateHttpClient(handler);
client.Timeout = TimeSpan.FromSeconds(42);

TaskCanceledException e = await Assert.ThrowsAsync<TaskCanceledException>(() => client.GetAsync(CreateFakeUri()));

TimeoutException connectTimeoutException = Assert.IsType<TimeoutException>(e.InnerException);
Assert.Contains("ConnectTimeout", connectTimeoutException.Message);

Assert.Null(connectTimeoutException.InnerException);
Assert.DoesNotContain("42", e.ToString());
}

[Fact]
public async Task UnknownOperationCanceledException_NotWrappedInATimeoutException()
{
using var client = new HttpClient(new CustomResponseHandler((request, cancellation) =>
{
throw new OperationCanceledException("Foo");
}));
client.Timeout = TimeSpan.FromSeconds(42);

OperationCanceledException e = await Assert.ThrowsAsync<OperationCanceledException>(() => client.GetAsync(CreateFakeUri()));

Assert.Null(e.InnerException);
Assert.Equal("Foo", e.Message);
Assert.DoesNotContain("42", e.ToString());
}

[Fact]
public void DefaultProxy_SetNull_Throws()
{
Expand Down

0 comments on commit bf7cdc5

Please sign in to comment.