spec: parse the server timeout into a Duration
A finite but huge :timeout like 1e30 passed validation and then panicked
in Duration::from_secs_f64 while waiting for the handshake.
Assisted-by: Claude Opus 5.5 via Claude Code
diff --git a/src/error.rs b/src/error.rs
index c075db3..534da35 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -3,6 +3,7 @@ use camino::Utf8PathBuf;
use miette::Diagnostic;
use std::io;
use std::process::ExitStatus;
+use std::time::Duration;
#[derive(Debug, thiserror::Error, Diagnostic)]
pub enum Error {
@@ -57,7 +58,7 @@ pub enum Error {
tail: String,
},
- #[error("{variant}: no handshake from the server after {timeout}s\n{tail}")]
+ #[error("{variant}: no handshake from the server after {timeout:?}\n{tail}")]
#[diagnostic(
code(parallax::server_timeout),
help(
@@ -66,7 +67,7 @@ pub enum Error {
)]
ServerTimeout {
variant: String,
- timeout: f64,
+ timeout: Duration,
tail: String,
},
}
diff --git a/src/server.rs b/src/server.rs
index d161c5e..e39f246 100644
--- a/src/server.rs
+++ b/src/server.rs
@@ -233,10 +233,10 @@ enum HandshakeError {
)]
fn wait_for_handshake(
path: &Utf8Path,
- timeout: f64,
+ timeout: Duration,
mut exited: impl FnMut() -> io::Result<Option<ExitStatus>>,
) -> io::Result<Result<Handshake, HandshakeError>> {
- let deadline = Instant::now() + Duration::from_secs_f64(timeout);
+ let deadline = Instant::now() + timeout;
loop {
if let Some(handshake) = read_handshake(path) {
return Ok(Ok(handshake));
@@ -307,7 +307,7 @@ mod tests {
/// A shell step that completes the handshake, for tests about teardown.
const HANDSHAKE: &str = r#"printf '{"url":"http://127.0.0.1:1"}' > "$PARALLAX_HANDSHAKE""#;
- fn spec(program: &str, args: &[&str], timeout: f64) -> ServerSpec {
+ fn spec(program: &str, args: &[&str], timeout: Duration) -> ServerSpec {
ServerSpec {
program: program.to_owned(),
args: args.iter().map(|&arg| arg.to_owned()).collect(),
@@ -315,7 +315,7 @@ mod tests {
}
}
- fn sh(script: &str, timeout: f64) -> ServerSpec {
+ fn sh(script: &str, timeout: Duration) -> ServerSpec {
spec("sh", &["-c", script], timeout)
}
@@ -335,9 +335,13 @@ mod tests {
async fn reads_the_handshake_url_and_data() {
let output = camino_tempfile::tempdir().expect("tempdir");
let script = r#"printf '{"url":"http://127.0.0.1:1","data":{"x":1}}' > "$PARALLAX_HANDSHAKE"; sleep 300"#;
- let server = Server::start(&sh(script, 5.0), "desktop", output.path())
- .await
- .expect("server starts");
+ let server = Server::start(
+ &sh(script, Duration::from_secs(5)),
+ "desktop",
+ output.path(),
+ )
+ .await
+ .expect("server starts");
assert_eq!(server.handshake().url, "http://127.0.0.1:1");
assert_eq!(server.handshake().data, json!({"x": 1}));
server.stop().await.expect("server stops");
@@ -349,7 +353,7 @@ mod tests {
let cmd = spec(
"cargo",
&["run", "--quiet", "--package", "parallax-fixture"],
- 120.0,
+ Duration::from_secs(120),
);
let server = Server::start(&cmd, "desktop", output.path())
.await
@@ -364,9 +368,13 @@ mod tests {
async fn wrappers_leave_no_orphans() {
let output = camino_tempfile::tempdir().expect("tempdir");
let script = format!("sleep 300 & {HANDSHAKE}; sleep 300");
- let server = Server::start(&sh(&script, 5.0), "desktop", output.path())
- .await
- .expect("server starts");
+ let server = Server::start(
+ &sh(&script, Duration::from_secs(5)),
+ "desktop",
+ output.path(),
+ )
+ .await
+ .expect("server starts");
let pgid = server.pgid;
server.stop().await.expect("server stops");
assert_group_gone(pgid);
@@ -376,9 +384,13 @@ mod tests {
async fn dropping_a_server_kills_its_group() {
let output = camino_tempfile::tempdir().expect("tempdir");
let script = format!("sleep 300 & {HANDSHAKE}; sleep 300");
- let server = Server::start(&sh(&script, 5.0), "desktop", output.path())
- .await
- .expect("server starts");
+ let server = Server::start(
+ &sh(&script, Duration::from_secs(5)),
+ "desktop",
+ output.path(),
+ )
+ .await
+ .expect("server starts");
let pgid = server.pgid;
drop(server);
assert_group_gone(pgid);
@@ -391,9 +403,13 @@ mod tests {
// would kill it before the handler could run. The handshake
// comes after the trap, so the handler is installed by then.
let script = format!("trap 'echo terminated-by-term' TERM; {HANDSHAKE}; sleep 300");
- let server = Server::start(&sh(&script, 5.0), "desktop", output.path())
- .await
- .expect("server starts");
+ let server = Server::start(
+ &sh(&script, Duration::from_secs(5)),
+ "desktop",
+ output.path(),
+ )
+ .await
+ .expect("server starts");
let log = server.log.clone();
server.stop().await.expect("server stops");
let text = fs_err::read_to_string(&log).expect("log readable");
@@ -406,9 +422,13 @@ mod tests {
// trap '' leaves SIGTERM ignored, and the fg sleep inherits
// that, so only SIGKILL can take the group down.
let script = format!("trap '' TERM; {HANDSHAKE}; sleep 300");
- let server = Server::start(&sh(&script, 5.0), "desktop", output.path())
- .await
- .expect("server starts");
+ let server = Server::start(
+ &sh(&script, Duration::from_secs(5)),
+ "desktop",
+ output.path(),
+ )
+ .await
+ .expect("server starts");
let pgid = server.pgid;
let started = Instant::now();
server.stop().await.expect("server stops");
@@ -420,9 +440,13 @@ mod tests {
#[tokio::test]
async fn an_app_that_exits_before_the_handshake_fails_with_its_tail() {
let output = camino_tempfile::tempdir().expect("tempdir");
- let error = Server::start(&sh("echo boom >&2; exit 3", 5.0), "desktop", output.path())
- .await
- .expect_err("server must fail");
+ let error = Server::start(
+ &sh("echo boom >&2; exit 3", Duration::from_secs(5)),
+ "desktop",
+ output.path(),
+ )
+ .await
+ .expect_err("server must fail");
assert!(
matches!(&error, Error::ServerExited { variant, tail, .. }
if variant == "desktop" && tail.contains("boom")),
@@ -434,13 +458,13 @@ mod tests {
#[tokio::test]
async fn a_server_that_never_handshakes_times_out() {
let output = camino_tempfile::tempdir().expect("tempdir");
- let cmd = sh("echo starting >&2; sleep 300", 0.25);
+ let cmd = sh("echo starting >&2; sleep 300", Duration::from_millis(250));
let error = Server::start(&cmd, "desktop", output.path())
.await
.expect_err("server must fail");
assert!(
matches!(&error, Error::ServerTimeout { timeout, tail, .. }
- if *timeout == 0.25 && tail.contains("starting")),
+ if *timeout == Duration::from_millis(250) && tail.contains("starting")),
"got {error:?}"
);
}
@@ -449,16 +473,24 @@ mod tests {
async fn a_handshake_without_a_url_keeps_waiting() {
let output = camino_tempfile::tempdir().expect("tempdir");
let script = r#"printf '{"data":{"x":1}}' > "$PARALLAX_HANDSHAKE"; sleep 300"#;
- let error = Server::start(&sh(script, 0.25), "desktop", output.path())
- .await
- .expect_err("server must fail");
+ let error = Server::start(
+ &sh(script, Duration::from_millis(250)),
+ "desktop",
+ output.path(),
+ )
+ .await
+ .expect_err("server must fail");
assert!(matches!(error, Error::ServerTimeout { .. }));
}
#[tokio::test]
async fn a_missing_command_is_named() {
let output = camino_tempfile::tempdir().expect("tempdir");
- let cmd = spec("parallax-no-such-command-xyzzy", &["serve"], 1.0);
+ let cmd = spec(
+ "parallax-no-such-command-xyzzy",
+ &["serve"],
+ Duration::from_secs(1),
+ );
let error = Server::start(&cmd, "desktop", output.path())
.await
.expect_err("spawn must fail");
@@ -473,9 +505,13 @@ mod tests {
async fn server_output_lands_in_the_log() {
let output = camino_tempfile::tempdir().expect("tempdir");
let script = format!("echo an-out-line; echo an-err-line >&2; {HANDSHAKE}; sleep 300");
- let server = Server::start(&sh(&script, 5.0), "desktop", output.path())
- .await
- .expect("server starts");
+ let server = Server::start(
+ &sh(&script, Duration::from_secs(5)),
+ "desktop",
+ output.path(),
+ )
+ .await
+ .expect("server starts");
let log = server.log.clone();
assert_eq!(log, output.path().join("desktop").join("server.log"));
server.stop().await.expect("server stops");
@@ -488,7 +524,9 @@ mod tests {
fn a_failed_exit_check_is_an_io_error() {
let dir = camino_tempfile::tempdir().expect("tempdir");
let path = dir.path().join("handshake.json");
- let outcome = wait_for_handshake(&path, 5.0, || Err(io::Error::other("no child")));
+ let outcome = wait_for_handshake(&path, Duration::from_secs(5), || {
+ Err(io::Error::other("no child"))
+ });
assert!(outcome.is_err());
}
diff --git a/src/spec.rs b/src/spec.rs
index 37d4215..9990bea 100644
--- a/src/spec.rs
+++ b/src/spec.rs
@@ -6,6 +6,7 @@
use std::collections::HashMap;
use std::fmt;
use std::num::NonZeroU32;
+use std::time::Duration;
use serde::Deserialize;
use serde::de::IgnoredAny;
@@ -158,7 +159,7 @@ pub struct ServerSpec {
pub program: String,
pub args: Vec<String>,
- pub timeout: f64,
+ pub timeout: Duration,
}
impl ServerSpec {
@@ -187,15 +188,18 @@ impl<'de> Deserialize<'de> for ServerSpec {
let Some(program) = words.next() else {
return Err(serde::de::Error::custom("cmd must not be empty"));
};
- if !table.timeout.is_finite() || table.timeout <= 0.0 {
+ let Some(timeout) = Duration::try_from_secs_f64(table.timeout)
+ .ok()
+ .filter(|timeout| !timeout.is_zero())
+ else {
return Err(serde::de::Error::custom(
"timeout must be a positive number of seconds",
));
- }
+ };
Ok(ServerSpec {
program,
args: words.collect(),
- timeout: table.timeout,
+ timeout,
})
}
}
@@ -310,7 +314,7 @@ mod tests {
let server: ServerSpec = lua.from_value(value).expect("server table");
assert_eq!(server.program, "true");
assert!(server.args.is_empty());
- assert_eq!(server.timeout, 30.0);
+ assert_eq!(server.timeout, Duration::from_secs(30));
});
}
@@ -323,7 +327,7 @@ mod tests {
assert_eq!(server.program, "app");
assert_eq!(server.args, vec!["serve".to_owned()]);
assert_eq!(server.command_line(), "app serve");
- assert_eq!(server.timeout, 5.0);
+ assert_eq!(server.timeout, Duration::from_secs(5));
},
);
}
@@ -343,6 +347,10 @@ mod tests {
r#"return {cmd = {"true"}, timeout = -1}"#,
"timeout must be a positive",
),
+ (
+ r#"return {cmd = {"true"}, timeout = 1e30}"#,
+ "timeout must be a positive",
+ ),
];
for (body, fragment) in cases {
with_value(body, |lua, value| {