djc/bb8

`pool.get()` doesn't fail fast for errors like tls handshake error

Opened this issue · 1 comments

without database pool

It will get error "Error: Pool(Backend(Io { kind: UnexpectedEof, message: "tls handshake eof" }))" immediately when connecting to SQL Server 2014 on macos.

prisma/tiberius#364

use deadpool_tiberius;

#[tokio::main]
async fn main() -> deadpool_tiberius::SqlServerResult<()> {
    let pool = deadpool_tiberius::Manager::new()
        .host("192.168.50.74") // default to localhost
        .port(1433) // default to
        .basic_authentication("user", "password")
        //  or .authentication(tiberius::AuthMethod)
        .database("test")
        .trust_cert()
        .max_size(10)
        .wait_timeout(15)
        .pre_recycle_sync(|_client, _metrics| Ok(()))
        .create_pool()?;

    let mut conn = pool.get().await?;
    let _rows = conn.simple_query("SELECT 1").await.unwrap();

    Ok(())
}
[dependencies]
deadpool = "0.12"
deadpool-tiberius = { version = "0.1", default-features = false, features = [
    "rustls",
    # "native-tls",
    # "vendored-openssl",
] }
futures = "0.3"
futures-core = "0.3"
tiberius = { version = "0.12", default-features = false, features = [
    "rustls",
    # "native-tls",
    # "vendored-openssl",
    "tds73",
] }
tokio = { version = "1", features = ["full"] }
tokio-util = { version = "0.7", features = ["compat"] }

using deadpool

I will get error immediately: Error: Pool(Backend(Io { kind: UnexpectedEof, message: "tls handshake eof" }))

use deadpool_tiberius;

#[tokio::main]
async fn main() -> deadpool_tiberius::SqlServerResult<()> {
    let pool = deadpool_tiberius::Manager::new()
        .host("192.168.50.74") // default to localhost
        .port(1433) // default to
        .basic_authentication("user", "password")
        //  or .authentication(tiberius::AuthMethod)
        .database("test")
        .trust_cert()
        .max_size(10)
        .wait_timeout(15)
        .pre_recycle_sync(|_client, _metrics| Ok(()))
        .create_pool()?;

    let mut conn = pool.get().await?;
    let _rows = conn.simple_query("SELECT 1").await.unwrap();

    Ok(())
}

using bb8

It will not return error immediately, it will ignore the error in bb8::ManageConnection.connect.

This is not expected, since I use pool.get to get connection then query something to check the database is healthy at program startup, it should return error immediately when tls handshake eof error occurs.

bb8 = "0.8"
use bb8::Pool;
use thiserror::Error;
use tiberius::{Client, Config};
use tokio::net::TcpStream;
use tokio_util::compat::{Compat, TokioAsyncWriteCompatExt};

#[derive(Error, Debug)]
pub enum DatabaseConnectionError {
    #[error("Failed to parse connection string: {0}")]
    ParseConnectionString(String),

    #[error("Failed to create database pool: {0}")]
    CreatePool(String),

    #[error("Database query error: {0}")]
    QueryError(#[from] tiberius::error::Error),

    #[error("Other database error: {0}")]
    Other(String),
}

pub type DbPool = Pool<TiberiusConnectionManager>;

pub struct TiberiusConnectionManager {
    config: Config,
}

impl TiberiusConnectionManager {
    pub fn new(config: Config) -> Self {
        Self { config }
    }
}

#[async_trait::async_trait]
impl bb8::ManageConnection for TiberiusConnectionManager {
    type Connection = Client<Compat<TcpStream>>;
    type Error = tiberius::error::Error;

    async fn connect(&self) -> Result<Self::Connection, Self::Error> {
        connect(&self.config).await
    }

    async fn is_valid(&self, conn: &mut Self::Connection) -> Result<(), Self::Error> {
        conn.simple_query("SELECT 1").await.map(|_| ())
    }

    fn has_broken(&self, _: &mut Self::Connection) -> bool {
        false
    }
}

pub async fn connect(
    config: &tiberius::Config,
) -> Result<Client<Compat<TcpStream>>, tiberius::error::Error> {
    let tcp = TcpStream::connect(config.get_addr()).await?;
    tcp.set_nodelay(true)?;
    Client::connect(config.clone(), tcp.compat_write()).await
}

pub async fn create_db_pool(
    connection_string: &str,
    max_pool_size: u32,
) -> Result<DbPool, DatabaseConnectionError> {
    let mut config = tiberius::Config::from_ado_string(connection_string).map_err(|e| {
        DatabaseConnectionError::ParseConnectionString(format!(
            "Failed to parse connection string: {}",
            e
        ))
    })?;
    config.trust_cert();
    let manager = TiberiusConnectionManager::new(config);
    Pool::builder()
        .max_size(max_pool_size)
        .build(manager)
        .await
        .map_err(|e| {
            DatabaseConnectionError::CreatePool(format!("Failed to create database pool: {}", e))
        })
}
djc commented

I think this is effectively #141 (see also discussion in #147)? Happy to review a PR, or dig into it if your organization is able to sponsor a fix.