openssh-rust / openssh

Scriptable SSH through OpenSSH in Rust
Apache License 2.0
232 stars 37 forks source link

Automatically cleaning up `.ssh-connectionXXXXX` on connection failure #122

Closed jaywonchung closed 1 year ago

jaywonchung commented 1 year ago

Hi,

I'm using the native-mux implementation for my project.

openssh = { version = "0.9.9", features = ["native-mux"], default-features = false }
...
[profile.release]
panic = "abort"
        let session = SSHSession::connect_mux(&host.hostname, KnownHosts::Add)
            .await
            .unwrap_or_else(|_| panic!("{} Failed to connect to host.", host));

Sometimes I mistype the hostname and connection rightfully fails, but perhaps because I have panic = "abort", the .ssh-connectionXXXXX directory (located under the current directory) is not removed. I'm on MacOS. I have panic = "abort" because I create SSH session inside an async task, and I want the program to terminate when a connection fails.

It seems like I can set the control_directory to point at somewhere like /tmp where it doesn't really matter, but since /tmp is shared, it'll be great if there's a native way to seamlessly clean up.

Thanks!

NobodyXu commented 1 year ago

We uses tempfile for creating temporary directory and it removes the dir on drop.

@jaywonchung Your code panic on error, which doesn't call the drop implementation of variables on stack unless it finds a catch, so I recommend switching to returning an error.

Using thiserror, creating a new error type is easy, I recommend doing this in library crate.

jaywonchung commented 1 year ago

Returning an error to the caller and panicking outside cleans up the directory well. Thanks for the great advice!