feat(core): Bump to 45s timeout for some domains (#1348)

BREAKING CHANGE: `SmtpError::TimeoutError` has been removed in favor of the one async-smtp uses, namely `std::io::Error` with `ErrorKind::TimeoutError`

core/src/rules.json

@@ -9,11 +9,14 @@
 	},
 	"by_mx_suffix": {
 		".antispamcloud.com.": {
-			"rules": ["SkipCatchAll"],
-			"_comment": "Some <RCPT TO> take exactly 30s to respond, so we skip the catch-all one, and bump the timeout."
+			"rules": ["SmtpTimeout45s", "SkipCatchAll"],
+			"_comment": "Some <RCPT TO> take 30s to respond (sometimes only on 2nd attempt, not deterministic), so we skip the catch-all one, and bump the timeout to well over 30s."
 		}
 	},
 	"rules": {
-		"SkipCatchAll": { "_comment": "Don't perform catch-all check." }
+		"SkipCatchAll": { "_comment": "Don't perform catch-all check" },
+		"SmtpTimeout45s": {
+			"_comment": "Set SMTP connection timeout to at least 45s. If the user request set an even higher timeout, take that one. Please note that this timeout is **per SMTP connection**. By default, we try 2 connections per email: if the 1st one failed, then we connect again to avoid potential greylisting, in which case the whole verification takes 1min30s."
+		}
 	}
 }

core/src/rules.rs

@@ -28,6 +28,8 @@ use std::collections::HashMap;
 pub enum Rule {
 	/// Don't perform catch-all check.
 	SkipCatchAll,
+	/// Set the SMTP timeout to 45s.
+	SmtpTimeout45s,
 }
 
 #[derive(Debug, Deserialize, Serialize)]

core/src/smtp/connect.rs

@@ -20,7 +20,6 @@ use async_smtp::{
 	smtp::{commands::*, extension::ClientId, ServerAddress, Socks5Config},
 	ClientTlsParameters, EmailAddress, SmtpClient, SmtpTransport,
 };
-use async_std::future;
 use rand::rngs::SmallRng;
 use rand::{distributions::Alphanumeric, Rng, SeedableRng};
 use std::iter;
@@ -49,16 +48,32 @@ macro_rules! try_smtp (
 
 /// Attempt to connect to host via SMTP, and return SMTP client on success.
 async fn connect_to_host(
+	domain: &str,
 	host: &str,
 	port: u16,
 	input: &CheckEmailInput,
 ) -> Result<SmtpTransport, SmtpError> {
+	let smtp_timeout = if let Some(t) = input.smtp_timeout {
+		if has_rule(domain, host, &Rule::SmtpTimeout45s) {
+			log::debug!(
+				target: LOG_TARGET,
+				"[email={}] Bumping SMTP timeout to at least 45s",
+				input.to_email,
+			);
+			Some(t.max(Duration::from_secs(45)))
+		} else {
+			input.smtp_timeout
+		}
+	} else {
+		None
+	};
+
 	// hostname verification fails if it ends with '.', for example, using
 	// SOCKS5 proxies we can `io: incomplete` error.
 	let host = host.trim_end_matches('.').to_string();
 
 	let security = {
-		let tls_params = ClientTlsParameters::new(
+		let tls_params: ClientTlsParameters = ClientTlsParameters::new(
 			host.clone(),
 			TlsConnector::new()
 				.use_sni(true)
@@ -77,7 +92,7 @@ async fn connect_to_host(
 		security,
 	)
 	.hello_name(ClientId::Domain(input.hello_name.clone()))
-	.timeout(Some(Duration::new(30, 0))); // Set timeout to 30s
+	.timeout(smtp_timeout);
 
 	if let Some(proxy) = &input.proxy {
 		let socks5_config = match (&proxy.username, &proxy.password) {
@@ -259,7 +274,7 @@ async fn create_smtp_future(
 ) -> Result<(bool, Deliverability), SmtpError> {
 	// FIXME If the SMTP is not connectable, we should actually return an
 	// Ok(SmtpDetails { can_connect_smtp: false, ... }).
-	let mut smtp_transport = connect_to_host(host, port, input).await?;
+	let mut smtp_transport = connect_to_host(domain, host, port, input).await?;
 
 	let is_catch_all = smtp_is_catch_all(&mut smtp_transport, domain, host, input)
 		.await
@@ -288,7 +303,7 @@ async fn create_smtp_future(
 				);
 
 				let _ = smtp_transport.close().await;
-				smtp_transport = connect_to_host(host, port, input).await?;
+				smtp_transport = connect_to_host(domain, host, port, input).await?;
 				result = email_deliverable(&mut smtp_transport, to_email).await;
 			}
 		}
@@ -311,11 +326,7 @@ async fn check_smtp_without_retry(
 	input: &CheckEmailInput,
 ) -> Result<SmtpDetails, SmtpError> {
 	let fut = create_smtp_future(to_email, host, port, domain, input);
-	let (is_catch_all, deliverability) = if let Some(smtp_timeout) = input.smtp_timeout {
-		future::timeout(smtp_timeout, fut).await??
-	} else {
-		fut.await?
-	};
+	let (is_catch_all, deliverability) = fut.await?;
 
 	Ok(SmtpDetails {
 		can_connect_smtp: true,

core/src/smtp/error.rs

@@ -22,7 +22,6 @@ use super::parser;
 use super::yahoo::YahooError;
 use crate::util::ser_with_display::ser_with_display;
 use async_smtp::smtp::error::Error as AsyncSmtpError;
-use async_std::future;
 use fast_socks5::SocksError;
 use serde::Serialize;
 
@@ -36,9 +35,6 @@ pub enum SmtpError {
 	/// Error when communicating with SMTP server.
 	#[serde(serialize_with = "ser_with_display")]
 	SmtpError(AsyncSmtpError),
-	/// Time-out error.
-	#[serde(serialize_with = "ser_with_display")]
-	TimeoutError(future::TimeoutError),
 	/// Error when verifying a Yahoo email via HTTP requests.
 	YahooError(YahooError),
 	/// Error when verifying a Gmail email via a HTTP request.
@@ -58,12 +54,6 @@ impl From<SocksError> for SmtpError {
 	}
 }
 
-impl From<future::TimeoutError> for SmtpError {
-	fn from(e: future::TimeoutError) -> Self {
-		SmtpError::TimeoutError(e)
-	}
-}
-
 impl From<YahooError> for SmtpError {
 	fn from(e: YahooError) -> Self {
 		SmtpError::YahooError(e)

core/src/smtp/mod.rs

@@ -110,7 +110,7 @@ pub async fn check_smtp(
 #[cfg(test)]
 mod tests {
 	use super::{check_smtp, CheckEmailInput, SmtpError};
-	use async_smtp::EmailAddress;
+	use async_smtp::{smtp::error::Error, EmailAddress};
 	use std::{str::FromStr, time::Duration};
 	use tokio::runtime::Runtime;
 	use trust_dns_proto::rr::Name;
@@ -120,13 +120,13 @@ mod tests {
 		let runtime = Runtime::new().unwrap();
 
 		let to_email = EmailAddress::from_str("foo@gmail.com").unwrap();
-		let host = Name::from_str("gmail.com").unwrap();
+		let host = Name::from_str("alt4.aspmx.l.google.com.").unwrap();
 		let mut input = CheckEmailInput::default();
 		input.set_smtp_timeout(Some(Duration::from_millis(1)));
 
 		let res = runtime.block_on(check_smtp(&to_email, &host, 25, "gmail.com", &input));
 		match res {
-			Err(SmtpError::TimeoutError(_)) => (),
+			Err(SmtpError::SmtpError(Error::Io(_))) => (), // ErrorKind == Timeout
 			_ => panic!("check_smtp did not time out"),
 		}
 	}