Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
kzhui125 opened this issue Oct 23, 2024 · 1 comment
Open

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

kzhui125 opened this issue Oct 23, 2024 · 1 comment

Comments

@kzhui125
Copy link

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
Copy link
Owner

djc commented Oct 24, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants