Skip to content

certbot: shutdown signal handling nits (late handler install, unreachable! on daemon return) #1014

Description

@kvinwang

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    easyrustPull requests that update rust code

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions