From 2c8621f6c7d5d103410343879efd73d2fcb48450 Mon Sep 17 00:00:00 2001 From: Leynos Date: Sat, 26 Jul 2025 15:28:39 +0100 Subject: [PATCH 1/4] Handle comments for panic tests --- Cargo.lock | 233 +++++++++++++++++- Cargo.toml | 17 +- ...eframe-a-guide-to-production-resilience.md | 3 + docs/roadmap.md | 4 +- ...-set-philosophy-and-capability-maturity.md | 3 + src/server.rs | 54 +++- tests/cucumber.rs | 8 + tests/features/connection_panic.feature | 6 + tests/steps/mod.rs | 1 + tests/steps/panic_steps.rs | 18 ++ tests/world.rs | 77 ++++++ 11 files changed, 414 insertions(+), 10 deletions(-) create mode 100644 tests/cucumber.rs create mode 100644 tests/features/connection_panic.feature create mode 100644 tests/steps/mod.rs create mode 100644 tests/steps/panic_steps.rs create mode 100644 tests/world.rs diff --git a/Cargo.lock b/Cargo.lock index 97cfd39b..21630ed5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -38,6 +38,62 @@ dependencies = [ "memchr", ] +[[package]] +name = "anstream" +version = "0.6.19" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "301af1932e46185686725e0fad2f8f2aa7da69dd70bf6ecc44d6b703844a3933" +dependencies = [ + "anstyle", + "anstyle-parse", + "anstyle-query", + "anstyle-wincon", + "colorchoice", + "is_terminal_polyfill", + "utf8parse", +] + +[[package]] +name = "anstyle" +version = "1.0.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "862ed96ca487e809f1c8e5a8447f6ee2cf102f846893800b20cebdf541fc6bbd" + +[[package]] +name = "anstyle-parse" +version = "0.2.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4e7644824f0aa2c7b9384579234ef10eb7efb6a0deb83f9630a49594dd9c15c2" +dependencies = [ + "utf8parse", +] + +[[package]] +name = "anstyle-query" +version = "1.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6c8bdeb6047d8983be085bab0ba1472e6dc604e7041dbf6fcd5e71523014fae9" +dependencies = [ + "windows-sys 0.59.0", +] + +[[package]] +name = "anstyle-wincon" +version = "3.0.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "403f75924867bb1033c59fbf0797484329750cfbe3c4325cd33127941fabc882" +dependencies = [ + "anstyle", + "once_cell_polyfill", + "windows-sys 0.59.0", +] + +[[package]] +name = "anyhow" +version = "1.0.98" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e16d2d3311acee920a9eb8d33b8cbc1787ce4a264e85f964c2404b969bdcd487" + [[package]] name = "async-stream" version = "0.3.6" @@ -185,6 +241,12 @@ version = "0.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5e764a1d40d510daf35e07be9eb06e75770908c27d411ee6c92109c9840eaaf7" +[[package]] +name = "bitflags" +version = "1.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bef38d45163c2f1dde094a7dfd33ccf595c92905c8f8f4fdc18d06fb1037718a" + [[package]] name = "bitflags" version = "2.9.1" @@ -483,6 +545,23 @@ dependencies = [ "wasi 0.14.2+wasi-0.2.4", ] +[[package]] +name = "gherkin" +version = "0.14.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "20b79820c0df536d1f3a089a2fa958f61cb96ce9e0f3f8f507f5a31179567755" +dependencies = [ + "heck 0.4.1", + "peg", + "quote", + "serde", + "serde_json", + "syn", + "textwrap", + "thiserror", + "typed-builder", +] + [[package]] name = "gimli" version = "0.31.1" @@ -987,6 +1066,53 @@ dependencies = [ "windows-targets 0.52.6", ] +[[package]] +name = "peg" +version = "0.6.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9f76678828272f177ac33b7e2ac2e3e73cc6c1cd1e3e387928aa69562fa51367" +dependencies = [ + "peg-macros", + "peg-runtime", +] + +[[package]] +name = "peg-macros" +version = "0.6.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "636d60acf97633e48d266d7415a9355d4389cea327a193f87df395d88cd2b14d" +dependencies = [ + "peg-runtime", + "proc-macro2", + "quote", +] + +[[package]] +name = "peg-runtime" +version = "0.6.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9555b1514d2d99d78150d3c799d4c357a3e2c2a8062cd108e93a06d9057629c5" + +[[package]] +name = "pin-project" +version = "1.1.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "677f1add503faace112b9f1373e43e9e054bfdd22ff1a63c1bc485eaec6a6a8a" +dependencies = [ + "pin-project-internal", +] + +[[package]] +name = "pin-project-internal" +version = "1.1.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6e918e4ff8c4549eb882f14b3a4bc8c8bc93de829416eacf579f1207a8fbf861" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "pin-project-lite" version = "0.2.16" @@ -1041,7 +1167,7 @@ checksum = "6fcdab19deb5195a31cf7726a210015ff1496ba1464fd42cb4f537b8b01b471f" dependencies = [ "bit-set", "bit-vec", - "bitflags", + "bitflags 2.9.1", "lazy_static", "num-traits", "rand", @@ -1161,7 +1287,7 @@ version = "0.5.15" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7e8af0dde094006011e6a740d4879319439489813bd0bcdc7d821beaeeff48ec" dependencies = [ - "bitflags", + "bitflags 2.9.1", ] [[package]] @@ -1202,6 +1328,12 @@ version = "0.6.29" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f162c6dd7b008981e4d40210aca20b4bd0f9b60ca9271061b07f78537722f2e1" +[[package]] +name = "regex-syntax" +version = "0.7.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dbb5fb1acd8a1a18b3dd5be62d25485eb770e05afb408a9627d14d451bae12da" + [[package]] name = "regex-syntax" version = "0.8.5" @@ -1284,7 +1416,7 @@ version = "0.38.44" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "fdb5bc1ae2baa591800df16c9ca78619bf65c0488b41b96ccec5d11220d8c154" dependencies = [ - "bitflags", + "bitflags 2.9.1", "errno", "libc", "linux-raw-sys 0.4.15", @@ -1369,6 +1501,21 @@ dependencies = [ "wait-timeout", ] +[[package]] +name = "ryu" +version = "1.0.20" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "28d3b2b1366ec20994f1fd18c3c594f05c5dd4bc44d8bb0c1c632c8d6829481f" + +[[package]] +name = "same-file" +version = "1.0.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "93fc1dc3aaa9bfed95e02e6eadabb4baf7e3078b0bd1b4d7b6b0b68378900502" +dependencies = [ + "winapi-util", +] + [[package]] name = "scc" version = "2.3.4" @@ -1454,6 +1601,18 @@ dependencies = [ "syn", ] +[[package]] +name = "serde_json" +version = "1.0.141" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "30b9eff21ebe718216c6ec64e1d9ac57087aad11efc64e32002bce4a0d4c03d3" +dependencies = [ + "itoa", + "memchr", + "ryu", + "serde", +] + [[package]] name = "serial_test" version = "3.2.0" @@ -1521,6 +1680,23 @@ version = "1.15.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "67b1b7a3b5fe4f1376887184045fcf45c69e92af734b7aaddc05fb777b6fbd03" +[[package]] +name = "smart-default" +version = "0.7.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0eb01866308440fc64d6c44d9e86c5cc17adfe33c4d6eed55da9145044d0ffc1" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "smawk" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b7c388c1b5e93756d0c740965c41e8822f866621d41acbdf6336a6a168f8840c" + [[package]] name = "socket2" version = "0.5.10" @@ -1548,6 +1724,39 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "synthez" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a3d2c2202510a1e186e63e596d9318c91a8cbe85cd1a56a7be0c333e5f59ec8d" +dependencies = [ + "syn", + "synthez-codegen", + "synthez-core", +] + +[[package]] +name = "synthez-codegen" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f724aa6d44b7162f3158a57bccd871a77b39a4aef737e01bcdff41f4772c7746" +dependencies = [ + "syn", + "synthez-core", +] + +[[package]] +name = "synthez-core" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "78bfa6ec52465e2425fd43ce5bbbe0f0b623964f7c63feb6b10980e816c654ea" +dependencies = [ + "proc-macro2", + "quote", + "sealed", + "syn", +] + [[package]] name = "tempfile" version = "3.20.0" @@ -1743,6 +1952,12 @@ version = "0.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6d49784317cd0d1ee7ec5c716dd598ec5b4483ea832a2dced265471cc0f690ae" +[[package]] +name = "utf8parse" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "06abde3611657adf66d383f00b093d7faecc7fa57071cce2578660c9f1010821" + [[package]] name = "valuable" version = "0.1.1" @@ -1895,6 +2110,15 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ac3b87c63620426dd9b991e5ce0329eff545bccbbb34f3be09ff6fb6ab51b7b6" +[[package]] +name = "winapi-util" +version = "0.1.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cf221c93e13a30d793f7645a0e7762c55d169dbb0a49671918a2319d289b10bb" +dependencies = [ + "windows-sys 0.59.0", +] + [[package]] name = "winapi-x86_64-pc-windows-gnu" version = "0.4.0" @@ -2175,6 +2399,7 @@ dependencies = [ "async-trait", "bincode", "bytes", + "cucumber", "dashmap", "futures", "leaky-bucket", @@ -2214,7 +2439,7 @@ version = "0.39.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6f42320e61fe2cfd34354ecb597f86f413484a798ba44a8ca1165c58d42da6c1" dependencies = [ - "bitflags", + "bitflags 2.9.1", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 32abd532..0c1d6198 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -6,7 +6,16 @@ edition = "2024" [dependencies] serde = { version = "1", features = ["derive"] } bincode = "2" -tokio = { version = "1", default-features = false, features = ["net", "signal", "rt-multi-thread", "macros", "sync", "time", "io-util"] } +tokio = { version = "1", default-features = false, features = [ + "net", + "signal", + "rt-multi-thread", + "macros", + "sync", + "time", + "io-util", + "test-util", +] } tokio-util = { version = "0.7", features = ["rt"] } futures = "0.3" async-trait = "0.1" @@ -25,8 +34,9 @@ logtest = "^2.0" proptest = "^1.0" loom = "^0.7" async-stream = "0.3" -tokio = { version = "1", default-features = false, features = ["test-util"] } serial_test = "3.1" +# Permit compatible bug fixes but block breaking updates +cucumber = ">=0.20, <0.21" metrics-util = "0.20" metrics-exporter-prometheus = "0.17" @@ -38,3 +48,6 @@ advanced-tests = [] [lints.clippy] pedantic = "warn" +[[test]] +name = "cucumber" +harness = false # Cucumber defines its own async main diff --git a/docs/hardening-wireframe-a-guide-to-production-resilience.md b/docs/hardening-wireframe-a-guide-to-production-resilience.md index 8d1e2ba6..8659c969 100644 --- a/docs/hardening-wireframe-a-guide-to-production-resilience.md +++ b/docs/hardening-wireframe-a-guide-to-production-resilience.md @@ -162,6 +162,9 @@ This ensures that resources are cleaned up correctly even in the case of panics or early returns from user code, relying on the RAII (Resource Acquisition Is Initialization) pattern for safety. +Connection tasks are wrapped with `catch_unwind` to log and discard panics. +Each panicking connection is isolated so it cannot terminate the entire server. + ### 3.2 Leak-Proof Registries with `Weak`/`Arc` A global `SessionRegistry` that stores `PushHandle`s to active connections is a diff --git a/docs/roadmap.md b/docs/roadmap.md index 6c1fd0a5..5f4d7f25 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -158,9 +158,9 @@ production environments. - [x] Provide an integration guide for popular monitoring systems (e.g., Prometheus). -- [ ] **Advanced Error Handling:** +- [x] **Advanced Error Handling:** - - [ ] Implement panic handlers in connection tasks to prevent a single + - [x] Implement panic handlers in connection tasks to prevent a single connection from crashing the server. - [ ] **Testing:** diff --git a/docs/the-road-to-wireframe-1-0-feature-set-philosophy-and-capability-maturity.md b/docs/the-road-to-wireframe-1-0-feature-set-philosophy-and-capability-maturity.md index 7255b3db..f9fedb65 100644 --- a/docs/the-road-to-wireframe-1-0-feature-set-philosophy-and-capability-maturity.md +++ b/docs/the-road-to-wireframe-1-0-feature-set-philosophy-and-capability-maturity.md @@ -238,6 +238,9 @@ expressive error-handling strategy. push fails due to a full queue, the frame can be routed to a separate "dead letter" channel for later inspection, logging, or reprocessing, enhancing the system's overall resilience. +- **Panic Handling:** Connection tasks are wrapped in a panic handler using + `catch_unwind`. A misbehaving connection can no longer terminate the entire + server. ### B. First-Class Developer Ergonomics diff --git a/src/server.rs b/src/server.rs index fd2ba4ea..87b9b513 100644 --- a/src/server.rs +++ b/src/server.rs @@ -369,7 +369,23 @@ async fn worker_task( let failure = on_failure.clone(); let factory = factory.clone(); let t = tracker.clone(); - t.spawn(process_stream(stream, factory, success, failure)); + // Capture peer address for better error context + let peer_addr = stream.peer_addr().ok(); + t.spawn(async move { + use futures::FutureExt as _; + if let Err(panic) = std::panic::AssertUnwindSafe( + process_stream(stream, factory, success, failure), + ) + .catch_unwind() + .await + { + tracing::error!( + ?panic, + ?peer_addr, + "connection task panicked" + ); + } + }); delay = Duration::from_millis(10); } Err(e) => { @@ -453,7 +469,8 @@ mod tests { use bincode::{Decode, Encode}; use rstest::{fixture, rstest}; use tokio::{ - net::TcpListener, + net::{TcpListener, TcpStream}, + sync::oneshot, time::{Duration, timeout}, }; use tokio_util::{sync::CancellationToken, task::TaskTracker}; @@ -869,4 +886,37 @@ mod tests { fn test_server_debug_compilation_guard() { assert!(cfg!(debug_assertions)); } + + #[rstest] + #[tokio::test] + async fn connection_panic_is_caught( + factory: impl Fn() -> WireframeApp + Send + Sync + Clone + 'static, + ) { + let factory = move || { + factory() + .on_connection_setup(|| async { panic!("boom") }) + .unwrap() + }; + let server = WireframeServer::new(factory) + .workers(1) + .bind("127.0.0.1:0".parse().unwrap()) + .expect("bind"); + let addr = server.local_addr().unwrap(); + + let (tx, rx) = oneshot::channel(); + let handle = tokio::spawn(async move { + server + .run_with_shutdown(async { + let _ = rx.await; + }) + .await + .unwrap(); + }); + + TcpStream::connect(addr).await.unwrap(); + TcpStream::connect(addr).await.unwrap(); + + let _ = tx.send(()); + handle.await.unwrap(); + } } diff --git a/tests/cucumber.rs b/tests/cucumber.rs new file mode 100644 index 00000000..c9b92b67 --- /dev/null +++ b/tests/cucumber.rs @@ -0,0 +1,8 @@ +mod steps; +mod world; + +use cucumber::World; +use world::PanicWorld; + +#[tokio::main] +async fn main() { PanicWorld::run("tests/features").await; } diff --git a/tests/features/connection_panic.feature b/tests/features/connection_panic.feature new file mode 100644 index 00000000..05bbc4aa --- /dev/null +++ b/tests/features/connection_panic.feature @@ -0,0 +1,6 @@ +Feature: Connection panic resilience + Scenario: connection panic does not crash server + Given a running wireframe server with a panic in connection setup + When I connect to the server + And I connect to the server again + Then both connections succeed diff --git a/tests/steps/mod.rs b/tests/steps/mod.rs new file mode 100644 index 00000000..1820a024 --- /dev/null +++ b/tests/steps/mod.rs @@ -0,0 +1 @@ +mod panic_steps; diff --git a/tests/steps/panic_steps.rs b/tests/steps/panic_steps.rs new file mode 100644 index 00000000..17c3330a --- /dev/null +++ b/tests/steps/panic_steps.rs @@ -0,0 +1,18 @@ +//! Cucumber step implementations for panic resilience testing. +//! +//! Defines Given-When-Then steps that verify server stability +//! when connection tasks panic during setup. + +use cucumber::{given, then, when}; + +use crate::world::PanicWorld; + +#[given("a running wireframe server with a panic in connection setup")] +async fn start_server(world: &mut PanicWorld) { world.start_panic_server().await; } + +#[when("I connect to the server")] +#[when("I connect to the server again")] +async fn connect(world: &mut PanicWorld) { world.connect_once().await; } + +#[then("both connections succeed")] +async fn verify(world: &mut PanicWorld) { world.verify_and_shutdown().await; } diff --git a/tests/world.rs b/tests/world.rs new file mode 100644 index 00000000..49343202 --- /dev/null +++ b/tests/world.rs @@ -0,0 +1,77 @@ +//! Test world state for Cucumber panic resilience tests. +//! +//! Provides shared state management for behavioural tests verifying +//! server resilience against connection task panics. + +use std::net::SocketAddr; + +use cucumber::World; +use tokio::{ + net::TcpStream, + sync::oneshot::{self, Sender}, +}; +use wireframe::{app::WireframeApp, server::WireframeServer}; + +#[derive(Debug, Default, World)] +pub struct PanicWorld { + pub addr: Option, + pub attempts: usize, + pub shutdown: Option>, +} + +impl PanicWorld { + /// Start a server that panics during connection setup. + /// + /// # Panics + /// Panics if binding the server fails or the server task fails. + pub async fn start_panic_server(&mut self) { + let factory = || { + WireframeApp::new() + .expect("Failed to create WireframeApp") + .on_connection_setup(|| async { panic!("boom") }) + .expect("Failed to set connection setup callback") + }; + let server = WireframeServer::new(factory) + .workers(1) + .bind("127.0.0.1:0".parse().expect("Failed to parse address")) + .expect("bind"); + + self.addr = Some(server.local_addr().expect("Failed to get server address")); + let (tx, rx) = oneshot::channel(); + self.shutdown = Some(tx); + + tokio::spawn(async move { + server + .run_with_shutdown(async { + let _ = rx.await; + }) + .await + .expect("Server task failed"); + }); + + tokio::task::yield_now().await; + } + + /// Connect to the running server once. + /// + /// # Panics + /// Panics if the server address is unknown or the connection fails. + pub async fn connect_once(&mut self) { + TcpStream::connect(self.addr.expect("Server address not set")) + .await + .expect("Failed to connect"); + self.attempts += 1; + } + + /// Verify both connections succeeded and shut down the server. + /// + /// # Panics + /// Panics if the connection attempts do not match the expected count. + pub async fn verify_and_shutdown(&mut self) { + assert_eq!(self.attempts, 2); + if let Some(tx) = self.shutdown.take() { + let _ = tx.send(()); + } + tokio::task::yield_now().await; + } +} From aa9838bdb5e0943632597312ebc8921b792df8cb Mon Sep 17 00:00:00 2001 From: Leynos Date: Sat, 26 Jul 2025 16:41:14 +0100 Subject: [PATCH 2/4] Gate cucumber test harness --- Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/Cargo.toml b/Cargo.toml index 0c1d6198..a82c8428 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -51,3 +51,4 @@ pedantic = "warn" [[test]] name = "cucumber" harness = false # Cucumber defines its own async main +required-features = ["advanced-tests"] From 3ed0150a385a51a350063e3495c9721f7c36895f Mon Sep 17 00:00:00 2001 From: Leynos Date: Sat, 26 Jul 2025 20:33:04 +0100 Subject: [PATCH 3/4] Synchronise panic test shutdown --- README.md | 5 +++-- tests/steps/panic_steps.rs | 5 ++++- tests/world.rs | 13 +++++++------ 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 99650191..4fbc6550 100644 --- a/README.md +++ b/README.md @@ -84,8 +84,9 @@ WireframeServer::new(|| { This example showcases how derive macros and the framing abstraction simplify a binary protocol server. See the -[full example](docs/rust-binary-router-library-design.md#5-6-illustrative-api-usage-examples) -in the design document for further details. +[full example](docs/rust-binary-router-library-design.md#5-6-illustrative-api-usage-examples) + in the design document for further +details. ## Custom Envelopes diff --git a/tests/steps/panic_steps.rs b/tests/steps/panic_steps.rs index 17c3330a..9dec136f 100644 --- a/tests/steps/panic_steps.rs +++ b/tests/steps/panic_steps.rs @@ -8,7 +8,10 @@ use cucumber::{given, then, when}; use crate::world::PanicWorld; #[given("a running wireframe server with a panic in connection setup")] -async fn start_server(world: &mut PanicWorld) { world.start_panic_server().await; } +async fn start_server(world: &mut PanicWorld) { + world.start_panic_server(); + std::future::ready(()).await; +} #[when("I connect to the server")] #[when("I connect to the server again")] diff --git a/tests/world.rs b/tests/world.rs index 49343202..1bd61024 100644 --- a/tests/world.rs +++ b/tests/world.rs @@ -17,6 +17,7 @@ pub struct PanicWorld { pub addr: Option, pub attempts: usize, pub shutdown: Option>, + pub handle: Option>, } impl PanicWorld { @@ -24,7 +25,7 @@ impl PanicWorld { /// /// # Panics /// Panics if binding the server fails or the server task fails. - pub async fn start_panic_server(&mut self) { + pub fn start_panic_server(&mut self) { let factory = || { WireframeApp::new() .expect("Failed to create WireframeApp") @@ -40,16 +41,14 @@ impl PanicWorld { let (tx, rx) = oneshot::channel(); self.shutdown = Some(tx); - tokio::spawn(async move { + self.handle = Some(tokio::spawn(async move { server .run_with_shutdown(async { let _ = rx.await; }) .await .expect("Server task failed"); - }); - - tokio::task::yield_now().await; + })); } /// Connect to the running server once. @@ -72,6 +71,8 @@ impl PanicWorld { if let Some(tx) = self.shutdown.take() { let _ = tx.send(()); } - tokio::task::yield_now().await; + if let Some(handle) = self.handle.take() { + handle.await.expect("Server task join failed"); + } } } From c4dbd74015b23291197c5c1139d83dea9ddfcecb Mon Sep 17 00:00:00 2001 From: Leynos Date: Sat, 26 Jul 2025 21:10:41 +0100 Subject: [PATCH 4/4] Signal server readiness for panic tests - add ready channel support in WireframeServer- synchronise PanicWorld server start- document cucumber modules and steps- clarify panic test assertions --- Cargo.toml | 4 +++- src/server.rs | 41 +++++++++++++++++++++++++++++--------- tests/cucumber.rs | 5 +++++ tests/steps/mod.rs | 5 +++++ tests/steps/panic_steps.rs | 5 +---- tests/world.rs | 6 +++++- 6 files changed, 51 insertions(+), 15 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index a82c8428..f5c5c28f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -48,7 +48,9 @@ advanced-tests = [] [lints.clippy] pedantic = "warn" +# The Cucumber test runner defines its own async main function, +# so the standard test harness must be disabled. [[test]] name = "cucumber" -harness = false # Cucumber defines its own async main +harness = false required-features = ["advanced-tests"] diff --git a/src/server.rs b/src/server.rs index 87b9b513..1fbe302d 100644 --- a/src/server.rs +++ b/src/server.rs @@ -28,6 +28,7 @@ pub type PreambleCallback = Arc< pub type PreambleErrorCallback = Arc; use tokio::{ net::TcpListener, + sync::oneshot, time::{Duration, sleep}, }; use tokio_util::{sync::CancellationToken, task::TaskTracker}; @@ -57,6 +58,7 @@ where workers: usize, on_preamble_success: Option>, on_preamble_failure: Option, + ready_tx: Option>, _preamble: PhantomData, } @@ -92,6 +94,7 @@ where workers, on_preamble_success: None, on_preamble_failure: None, + ready_tx: None, _preamble: PhantomData, } } @@ -132,6 +135,7 @@ where workers: self.workers, on_preamble_success: None, on_preamble_failure: None, + ready_tx: None, _preamble: PhantomData, } } @@ -189,6 +193,14 @@ where self } + /// Configure a channel used to signal when the server is ready to accept + /// connections. + #[must_use] + pub fn ready_signal(mut self, tx: oneshot::Sender<()>) -> Self { + self.ready_tx = Some(tx); + self + } + /// Returns the configured number of worker tasks for the server. /// /// # Examples @@ -312,6 +324,9 @@ where S: futures::Future + Send, { let listener = self.listener.expect("`bind` must be called before `run`"); + if let Some(tx) = self.ready_tx { + let _ = tx.send(()); + } let shutdown_token = CancellationToken::new(); let tracker = TaskTracker::new(); @@ -379,11 +394,14 @@ async fn worker_task( .catch_unwind() .await { - tracing::error!( - ?panic, - ?peer_addr, - "connection task panicked" - ); + let panic_msg = if let Some(s) = panic.downcast_ref::<&str>() { + (*s).to_string() + } else if let Some(s) = panic.downcast_ref::() { + s.clone() + } else { + format!("{panic:?}") + }; + tracing::error!(panic = %panic_msg, ?peer_addr, "connection task panicked"); } }); delay = Duration::from_millis(10); @@ -887,17 +905,18 @@ mod tests { assert!(cfg!(debug_assertions)); } + /// Ensure the server survives panicking connection tasks. #[rstest] #[tokio::test] async fn connection_panic_is_caught( factory: impl Fn() -> WireframeApp + Send + Sync + Clone + 'static, ) { - let factory = move || { + let app_factory = move || { factory() .on_connection_setup(|| async { panic!("boom") }) .unwrap() }; - let server = WireframeServer::new(factory) + let server = WireframeServer::new(app_factory) .workers(1) .bind("127.0.0.1:0".parse().unwrap()) .expect("bind"); @@ -913,8 +932,12 @@ mod tests { .unwrap(); }); - TcpStream::connect(addr).await.unwrap(); - TcpStream::connect(addr).await.unwrap(); + TcpStream::connect(addr) + .await + .expect("first connection should succeed"); + TcpStream::connect(addr) + .await + .expect("second connection should succeed after panic"); let _ = tx.send(()); handle.await.unwrap(); diff --git a/tests/cucumber.rs b/tests/cucumber.rs index c9b92b67..8837c3ae 100644 --- a/tests/cucumber.rs +++ b/tests/cucumber.rs @@ -1,3 +1,8 @@ +//! Cucumber test runner for panic resilience integration tests. +//! +//! Runs behavioural tests defined in `tests/features/` using the +//! `PanicWorld` test context to verify server panic handling. + mod steps; mod world; diff --git a/tests/steps/mod.rs b/tests/steps/mod.rs index 1820a024..d422baa7 100644 --- a/tests/steps/mod.rs +++ b/tests/steps/mod.rs @@ -1 +1,6 @@ +//! Aggregates step definitions for Cucumber tests. +//! +//! This module exposes all Given-When-Then steps used by the +//! behaviour-driven tests under `tests/features`. + mod panic_steps; diff --git a/tests/steps/panic_steps.rs b/tests/steps/panic_steps.rs index 9dec136f..17c3330a 100644 --- a/tests/steps/panic_steps.rs +++ b/tests/steps/panic_steps.rs @@ -8,10 +8,7 @@ use cucumber::{given, then, when}; use crate::world::PanicWorld; #[given("a running wireframe server with a panic in connection setup")] -async fn start_server(world: &mut PanicWorld) { - world.start_panic_server(); - std::future::ready(()).await; -} +async fn start_server(world: &mut PanicWorld) { world.start_panic_server().await; } #[when("I connect to the server")] #[when("I connect to the server again")] diff --git a/tests/world.rs b/tests/world.rs index 1bd61024..10f14329 100644 --- a/tests/world.rs +++ b/tests/world.rs @@ -25,7 +25,7 @@ impl PanicWorld { /// /// # Panics /// Panics if binding the server fails or the server task fails. - pub fn start_panic_server(&mut self) { + pub async fn start_panic_server(&mut self) { let factory = || { WireframeApp::new() .expect("Failed to create WireframeApp") @@ -39,16 +39,20 @@ impl PanicWorld { self.addr = Some(server.local_addr().expect("Failed to get server address")); let (tx, rx) = oneshot::channel(); + let (ready_tx, ready_rx) = oneshot::channel(); self.shutdown = Some(tx); self.handle = Some(tokio::spawn(async move { server + .ready_signal(ready_tx) .run_with_shutdown(async { let _ = rx.await; }) .await .expect("Server task failed"); })); + + ready_rx.await.expect("Server did not signal ready"); } /// Connect to the running server once.