Skip to content

Commit 0418af6

Browse files
authored
test(iroh-net): try fix flaky udp_blocked test
This test is flaky because somehow the executor is entirely blocked for a solid 5 seconds after which the entire netcheck report is timing out. Thus the part of the netcheck report that is being tested never executes. What blocks the executor however is a mystery to me. This fix assumes that it is generating the backtrace in the log, because there simply isn't anything else that it can be. Thus the relevant change is no longer producing the backtrace. The error message is already pretty obvious. To go further, this disables the anyhow backtrace feature entirely. This will probably make our lifes a bit harder in some situations but there are many places where they end up in the log otherwise and each time this happens it blocks an executor thread for a long time. This also tweaks the error reporting to not generate so much logging noise when the captive portal fails in an expected way. ## Notes & open questions An attempt at fixing #1901
1 parent 1389857 commit 0418af6

File tree

9 files changed

+22
-12
lines changed

9 files changed

+22
-12
lines changed

Cargo.lock

Lines changed: 0 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

iroh-base/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ rust-version = "1.72"
1515
workspace = true
1616

1717
[dependencies]
18-
anyhow = { version = "1", features = ["backtrace"] }
18+
anyhow = { version = "1" }
1919
bao-tree = { version = "0.9.1", features = ["tokio_fsm"], default-features = false, optional = true }
2020
data-encoding = { version = "2.3.3", optional = true }
2121
hex = "0.4.3"

iroh-bytes/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ rust-version = "1.72"
1616
workspace = true
1717

1818
[dependencies]
19-
anyhow = { version = "1", features = ["backtrace"] }
19+
anyhow = { version = "1" }
2020
bao-tree = { version = "0.9.1", features = ["tokio_fsm"], default-features = false }
2121
bytes = { version = "1.4", features = ["serde"] }
2222
chrono = "0.4.31"

iroh-gossip/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ workspace = true
1616

1717
[dependencies]
1818
# proto dependencies (required)
19-
anyhow = { version = "1", features = ["backtrace"] }
19+
anyhow = { version = "1" }
2020
blake3 = { package = "iroh-blake3", version = "1.4.3"}
2121
bytes = { version = "1.4.0", features = ["serde"] }
2222
data-encoding = "2.4.0"

iroh-metrics/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ rust-version = "1.72"
1515
workspace = true
1616

1717
[dependencies]
18-
anyhow = { version = "1", features = ["backtrace"] }
18+
anyhow = { version = "1" }
1919
erased_set = "0.7"
2020
http-body-util = "0.1.0"
2121
hyper = { version = "1", features = ["server", "http1"] }

iroh-net/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ workspace = true
1717

1818
[dependencies]
1919
aead = { version = "0.5.2", features = ["bytes"] }
20-
anyhow = { version = "1", features = ["backtrace"] }
20+
anyhow = { version = "1" }
2121
backoff = "0.4.0"
2222
bytes = "1"
2323
crypto_box = { version = "0.9.1", features = ["serde", "chacha20"] }

iroh-net/src/netcheck.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -949,6 +949,11 @@ mod tests {
949949
// create raw ICMP pings and we'll have to silenty accept this test is useless (if
950950
// we could, this would be a skip instead).
951951
let have_pinger = Pinger::new().is_ok();
952+
if !have_pinger {
953+
error!("pinger disabled, test effectively skipped");
954+
} else {
955+
info!("pinger enabled");
956+
}
952957

953958
// This is the test: we will fall back to sending ICMP pings. These should
954959
// succeed when we have a working pinger.

iroh-net/src/netcheck/reportgen.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,7 @@ impl Actor {
249249
break;
250250
}
251251
tokio::select! {
252+
biased;
252253
_ = &mut total_timer => {
253254
trace!("tick: total_timer expired");
254255
bail!("report timed out");
@@ -533,6 +534,7 @@ impl Actor {
533534
MaybeFuture {
534535
inner: Some(Box::pin(async move {
535536
tokio::time::sleep(CAPTIVE_PORTAL_DELAY).await;
537+
debug!("Captive portal check started after {CAPTIVE_PORTAL_DELAY:?}");
536538
let captive_portal_check = tokio::time::timeout(
537539
CAPTIVE_PORTAL_TIMEOUT,
538540
check_captive_portal(&dm, preferred_derp)
@@ -541,7 +543,14 @@ impl Actor {
541543
match captive_portal_check.await {
542544
Ok(Ok(found)) => Some(found),
543545
Ok(Err(err)) => {
544-
warn!("check_captive_portal error: {:?}", err);
546+
let err: Result<reqwest::Error, _> = err.downcast();
547+
match err {
548+
Ok(req_err) if req_err.is_connect() => {
549+
debug!("check_captive_portal failed: {req_err:#}");
550+
}
551+
Ok(req_err) => warn!("check_captive_portal error: {req_err:#}"),
552+
Err(any_err) => warn!("check_captive_portal error: {any_err:#}"),
553+
}
545554
None
546555
}
547556
Err(_) => {
@@ -881,7 +890,6 @@ async fn run_probe(
881890
/// return a "204 No Content" response and checking if that's what we get.
882891
///
883892
/// The boolean return is whether we think we have a captive portal.
884-
#[allow(clippy::unnecessary_unwrap)] // NOTE: clippy's suggestion makes the code a lot more complex
885893
async fn check_captive_portal(dm: &DerpMap, preferred_derp: Option<Url>) -> Result<bool> {
886894
// If we have a preferred DERP node, try that; otherwise, pick a random one not marked as "Avoid".
887895
let url = match preferred_derp {

iroh/Cargo.toml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ rust-version = "1.72"
1717
workspace = true
1818

1919
[dependencies]
20-
anyhow = { version = "1", features = ["backtrace"] }
20+
anyhow = { version = "1" }
2121
bao-tree = { version = "0.9.1", features = ["tokio_fsm"], default-features = false }
2222
bytes = "1"
2323
data-encoding = "2.4.0"
@@ -83,7 +83,7 @@ flat-db = ["iroh-bytes/flat-db"]
8383
test = []
8484

8585
[dev-dependencies]
86-
anyhow = { version = "1", features = ["backtrace"] }
86+
anyhow = { version = "1" }
8787
bytes = "1"
8888
console-subscriber = "0.2"
8989
duct = "0.13.6"

0 commit comments

Comments
 (0)