Skip to content

Wrap GetHostAddresses/Async calls to catch SocketException#5664

Merged
NinoFloris merged 3 commits intonpgsql:mainfrom
pedrowindisch:ref/wrap-get-host-addresses-calls
Oct 29, 2025
Merged

Wrap GetHostAddresses/Async calls to catch SocketException#5664
NinoFloris merged 3 commits intonpgsql:mainfrom
pedrowindisch:ref/wrap-get-host-addresses-calls

Conversation

@pedrowindisch
Copy link
Contributor

@pedrowindisch pedrowindisch commented Apr 8, 2024

Closes #5606.

The GetHostAddresses method also throws:

  • ArgumentOutOfRangeException, if the host name exceeds 255 chars;
  • ArgumentNullException, if the host name is null;
  • ArgumentException, if the host address is an invalid IP address.

Should these other exceptions be wrapped into a NpgsqlException as well?

Copy link
Member

@NinoFloris NinoFloris left a comment

Choose a reason for hiding this comment

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

Thanks for doing this, I've made some tweaks to your PR but otherwise LGTM!

@NinoFloris NinoFloris force-pushed the ref/wrap-get-host-addresses-calls branch from d0ab5e0 to ffd29b0 Compare October 29, 2025 16:19
@NinoFloris NinoFloris merged commit 19d62f6 into npgsql:main Oct 29, 2025
16 checks passed
Comment on lines +1332 to +1333
ipAddresses = await Dns.GetHostAddressesAsync(Host, cancellationToken)
.WaitAsync(timeout.CheckAndGetTimeLeft(), cancellationToken).ConfigureAwait(false);
Copy link
Contributor

Choose a reason for hiding this comment

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

@NinoFloris I'm pretty sure before it was calling TaskTimeoutAndCancellation.ExecuteAsync instead of Dns.GetHostAddressesAsync directly

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Let me put up a PR to pull it out entirely and fix the timeout flowing into the token.

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.

SocketException not wrapped into NpgsqlException when hostname can not be resolved.

3 participants