Issue with native-tls handshake

[mirandole]

Hello,

I got an issue using tungstenite with native-tls feature. Sometimes when my project try to connect to a websocket with tungteniste using native-tls, the connection with client_tls panic with this error : "Bug: TLS handshake not blocked". It's annoying because I don't know how to catch that kind of panic in my connection retry mechanism.

So my question is why does this code needs to panic when TlsHandshakeError::WouldBlock ? It would be much pleasant if the code will return an error instead of panic. So I can retry to connect with client_tls without getting my project exit with panic.

To try to find a way to avoid my rust program to panic, I now use rustls instead of native-tls. But I think it would be better to return an error instead of panic.

match mode {
                Mode::Plain => Ok(MaybeTlsStream::Plain(socket)),
                Mode::Tls => {
                    let try_connector = tls_connector.map_or_else(TlsConnector::new, Ok);
                    let connector = try_connector.map_err(TlsError::Native)?;
                    let connected = connector.connect(domain, socket);
                    match connected {
                        Err(e) => match e {
                            TlsHandshakeError::Failure(f) => Err(Error::Tls(f.into())),
                            TlsHandshakeError::WouldBlock(_) => {
                                panic!("Bug: TLS handshake not blocked")
                            }
                        },
                        Ok(s) => Ok(MaybeTlsStream::NativeTls(s)),
                    }
                }
            }

https://github.com/snapview/tungstenite-rs/blob/master/src/tls.rs

[daniel-abramov]

Tungstenite must not panic unless there is a bug in a crate. As the panic says, this error indicates a bug in a library and must not happen. I don't remember why this exact panic is there, i.e., why this error is considered a bug, as this commit has been there for more than seven years, so I'll mention @agalakhov here, he certainly knows this part better.

However, I must also say that we've never encountered this panic, and I don't remember it ever being reported. Is it reproducible?

[mirandole]

Yes, upon reflection, I was able to reproduce it some minutes ago. It happens when my internet connection drop while executing client_tls method using tcp_stream.set_read_timeout and tcp_stream.set_write_timeout. If I disconnect my internet connection during the sleep time just before client_tls(). The panic happens because of tcp_stream timeout.

Removing tcp_stream.set_read_timeout and tcp_stream.set_write_timeout resolved my issue but client_tls timeout happens without any panic after 5min which is quite long for me but ok to manage. It would be nice to be able to set a custom timeout in client_tls() or to remove this annoying panic! to be able to benefit of the tcpstream timeout feature.

use std::{net::{TcpStream, ToSocketAddrs}, time::Duration};
use tungstenite::{client::IntoClientRequest, connect, http::{self, request, Uri}, ClientRequestBuilder};
use tungstenite::client_tls;

fn main() {
    let timeout = Duration::from_secs(30);
    let hostname_port = "stream.binance.com:443";
    let socket_addr = hostname_port.to_socket_addrs().unwrap().next().unwrap();
    let wss_addr = "wss://stream.binance.com/ws/btcusdt@ticker";

    match TcpStream::connect_timeout(&socket_addr, timeout) {
        Ok(tcp_stream) => {
            println!("tcpstream connected");
            tcp_stream.set_read_timeout(Some(timeout)).expect("Can't set read timeout");
            tcp_stream.set_write_timeout(Some(timeout)).expect("Can't set write timeout");

            // DISCONNECT INTERNET CONNECTION DURING SLEEP TIME
            println!("10s sleep time to disconnect internet and reproduce panic");
            std::thread::sleep(std::time::Duration::from_secs(10));

            let uri: Uri = wss_addr.parse().unwrap();
            let request = ClientRequestBuilder::new(uri).into_client_request().unwrap();

            println!("begin client_tls connection");
            match client_tls(request, tcp_stream) {
                Ok(mut answer) => {
                    println!("client_tls connected");
                    match answer.0.get_mut() {
                        tungstenite::stream::MaybeTlsStream::Plain(stream) => {
                            println!("tungstenite::stream::MaybeTlsStream::Plain");
                            let _ = stream.set_nonblocking(true).unwrap();
                            println!("nonblocking set");
                        },
                        tungstenite::stream::MaybeTlsStream::NativeTls(stream) => {
                            println!("tungstenite::stream::MaybeTlsStream::NativeTls");
                            let _ = stream.get_mut().set_nonblocking(true).unwrap();
                            println!("nonblocking set");
                        },
                        _ => {
                            println!("unimplemented");
                            unimplemented!();
                        },
                    };

                },
                Err(e) => {
                    println!("{}", format!("Error during handshake {}", e));
                },
            }
        },
        Err(e) => {
            println!("{}", format!("error connecting to : {}\nconnection error: {}", &socket_addr, e));
        },
    }
}

PS: When I use rustls instead of native-tls, I got the same error message without panic.