Follow-up to #924.
Two small robustness nits in the shutdown path added by #924, both in dstack/certbot/cli/src/main.rs.
1. The signal handler is installed after build_bot().
let bot = bot_config.build_bot().await.context("Failed to build bot")?;
if once {
...
} else {
tokio::select! {
_ = bot.run() => ...,
result = shutdown_signal() => result?,
}
}
build_bot() can create the ACME account and do network I/O. A SIGTERM arriving during that window still hits the default disposition and hard-kills the process, so the "graceful" window only opens after startup completes. Registering the signal handler before build_bot() (and selecting over it for the build too) makes shutdown uniform across the process lifetime. Low impact, but startup is exactly when an orchestrator is most likely to change its mind.
2. unreachable!("certbot daemon returned") turns a future refactor into a panic.
_ = bot.run() => unreachable!("certbot daemon returned"),
This holds today only because CertBot::run() is an unconditional loop {} returning (). It is not enforced by the type system: the day someone adds an early return to run() — e.g. to bail out after N consecutive failures — the CLI panics instead of exiting cleanly, and the panic message actively misleads the reader into thinking the state is impossible.
Either encode the invariant (async fn run(&self) -> !, or return std::convert::Infallible) so the compiler enforces it, or make the arm a normal exit path.
Follow-up to #924.
Two small robustness nits in the shutdown path added by #924, both in
dstack/certbot/cli/src/main.rs.1. The signal handler is installed after
build_bot().build_bot()can create the ACME account and do network I/O. A SIGTERM arriving during that window still hits the default disposition and hard-kills the process, so the "graceful" window only opens after startup completes. Registering the signal handler beforebuild_bot()(and selecting over it for the build too) makes shutdown uniform across the process lifetime. Low impact, but startup is exactly when an orchestrator is most likely to change its mind.2.
unreachable!("certbot daemon returned")turns a future refactor into a panic.This holds today only because
CertBot::run()is an unconditionalloop {}returning(). It is not enforced by the type system: the day someone adds an earlyreturntorun()— e.g. to bail out after N consecutive failures — the CLI panics instead of exiting cleanly, and the panic message actively misleads the reader into thinking the state is impossible.Either encode the invariant (
async fn run(&self) -> !, or returnstd::convert::Infallible) so the compiler enforces it, or make the arm a normal exit path.