feat: lookup_host_with_cache(): Don't return empty address list (#7596)

All users of this function expect a nonempty list to be returned, otherwise they fail with hard to
understand errors like "No connection attempts were made", so it's better to fail early if DNS
resolution failed and the cache doesn't contain resolutions as well.
This commit is contained in:
iequidoo
2025-12-14 18:01:26 -03:00
committed by iequidoo
parent 7ba4a43253
commit e34fee72a0
2 changed files with 14 additions and 8 deletions

View File

@@ -1295,16 +1295,17 @@ def test_configure_error_msgs_invalid_server(acfactory):
ev = ac2._evtracker.get_matching("DC_EVENT_CONFIGURE_PROGRESS") ev = ac2._evtracker.get_matching("DC_EVENT_CONFIGURE_PROGRESS")
if ev.data1 == 0: if ev.data1 == 0:
break break
err_lower = ev.data2.lower()
# Can't connect so it probably should say something about "internet" # Can't connect so it probably should say something about "internet"
# again, should not repeat itself # again, should not repeat itself
# If this fails then probably `e.msg.to_lowercase().contains("could not resolve")` # If this fails then probably `e.msg.to_lowercase().contains("could not resolve")`
# in configure.rs returned false because the error message was changed # in configure.rs returned false because the error message was changed
# (i.e. did not contain "could not resolve" anymore) # (i.e. did not contain "could not resolve" anymore)
assert (ev.data2.count("internet") + ev.data2.count("network")) == 1 assert (err_lower.count("internet") + err_lower.count("network")) == 1
# Should mention that it can't connect: # Should mention that it can't connect:
assert ev.data2.count("connect") == 1 assert err_lower.count("connect") == 1
# The users do not know what "configuration" is # The users do not know what "configuration" is
assert "configuration" not in ev.data2.lower() assert "configuration" not in err_lower
def test_status(acfactory): def test_status(acfactory):

View File

@@ -40,7 +40,7 @@
//! used for successful connection timestamp of //! used for successful connection timestamp of
//! retrieving them from in-memory cache is used. //! retrieving them from in-memory cache is used.
use anyhow::{Context as _, Result}; use anyhow::{Context as _, Result, ensure};
use std::collections::HashMap; use std::collections::HashMap;
use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr}; use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr};
use std::str::FromStr; use std::str::FromStr;
@@ -850,7 +850,7 @@ pub(crate) async fn lookup_host_with_cache(
} }
}; };
if load_cache { let addrs = if load_cache {
let mut cache = lookup_cache(context, hostname, port, alpn, now).await?; let mut cache = lookup_cache(context, hostname, port, alpn, now).await?;
if let Some(ips) = DNS_PRELOAD.get(hostname) { if let Some(ips) = DNS_PRELOAD.get(hostname) {
for ip in ips { for ip in ips {
@@ -861,10 +861,15 @@ pub(crate) async fn lookup_host_with_cache(
} }
} }
Ok(merge_with_cache(resolved_addrs, cache)) merge_with_cache(resolved_addrs, cache)
} else { } else {
Ok(resolved_addrs) resolved_addrs
} };
ensure!(
!addrs.is_empty(),
"Could not find DNS resolutions for {hostname}:{port}. Check server hostname and your network"
);
Ok(addrs)
} }
/// Merges results received from DNS with cached results. /// Merges results received from DNS with cached results.