Skip to content

Conversation

Makaopior
Copy link

OperationCanceledException is usually treated like "The underlying operation has been canceled, terminate the upstream too", and that's not what is to be expected from an ordinary timeout. HttpClient has a similar problem too.
Changed behavior so that cancelling the async socket IO operation throws TimeoutException instead of passing cancellation exception through.

@jsakamoto jsakamoto requested a review from Copilot April 10, 2025 11:51
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 2 out of 4 changed files in this pull request and generated 1 comment.

Files not reviewed (2)
  • RELEASE-NOTES.txt: Language not supported
  • WhoisClient.NET/WhoisClient.NET.csproj: Language not supported
Comments suppressed due to low confidence (1)

WhoisClient.NET.Test/IrregularCaseTests.cs:131

  • [nitpick] Verify that the synchronous RawQuery method is intended to throw an IOException while the asynchronous RawQueryAsync method throws a TimeoutException. If the discrepancy is intentional, consider adding a clarifying comment explaining the rationale.
Assert.ThrowsAsync<IOException>(new AsyncTestDelegate(action));

Comment on lines +475 to +477
catch (OperationCanceledException) when (!token.IsCancellationRequested)
{
throw new TimeoutException("Socket operation timeout");
Copy link

Copilot AI Apr 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Consider including the original OperationCanceledException as the inner exception when throwing the TimeoutException to preserve additional context for debugging.

Suggested change
catch (OperationCanceledException) when (!token.IsCancellationRequested)
{
throw new TimeoutException("Socket operation timeout");
catch (OperationCanceledException ex) when (!token.IsCancellationRequested)
{
throw new TimeoutException("Socket operation timeout", ex);

Copilot uses AI. Check for mistakes.

@jsakamoto jsakamoto merged commit bb25150 into jsakamoto:master Apr 10, 2025
@jsakamoto
Copy link
Owner

Hi @Makaopior,
I've released version 6.1.

Thank you for your many contributions!

@Makaopior
Copy link
Author

Hi @jsakamoto ,
Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants