fix: replace missed panics with Result (#638)

Follow-up PR after #636
Also removes outdated mentions of panics

testcontainers/src/core/client.rs

@@ -17,6 +17,7 @@ use tokio_stream::wrappers::UnboundedReceiverStream;
 use crate::core::{
     client::exec::ExecResult,
     env,
+    env::ConfigurationError,
     logs::{LogSource, LogStreamAsync},
     ports::{PortMappingError, Ports},
 };
@@ -41,6 +42,8 @@ async fn is_in_container() -> bool {
 pub enum ClientError {
     #[error("failed to initialize a docker client: {0}")]
     Init(BollardError),
+    #[error("configuration error: {0}")]
+    Configuration(#[from] ConfigurationError),
     #[error("invalid docker host: {0}")]
     InvalidDockerHost(String),
     #[error("failed to pull the image '{descriptor}', error: {err}")]
@@ -85,7 +88,7 @@ pub(crate) struct Client {
 
 impl Client {
     async fn new() -> Result<Client, ClientError> {
-        let config = env::Config::load::<env::Os>().await;
+        let config = env::Config::load::<env::Os>().await?;
         let bollard = bollard_client::init(&config).map_err(ClientError::Init)?;
 
         Ok(Client { config, bollard })
@@ -343,10 +346,14 @@ impl Client {
     pub(crate) async fn docker_hostname(&self) -> Result<url::Host, ClientError> {
         let docker_host = self.config.docker_host();
         match docker_host.scheme() {
-            "tcp" | "http" | "https" => docker_host
-                .host()
-                .map(|host| host.to_owned())
-                .ok_or_else(|| ClientError::InvalidDockerHost(docker_host.to_string())),
+            "tcp" | "http" | "https" => {
+                docker_host
+                    .host()
+                    .map(|host| host.to_owned())
+                    .ok_or_else(|| {
+                        ConfigurationError::InvalidDockerHost(docker_host.to_string()).into()
+                    })
+            }
             "unix" | "npipe" => {
                 if is_in_container().await {
                     let host = self
@@ -363,7 +370,8 @@ impl Client {
                         .filter(|gateway| !gateway.trim().is_empty())
                         .unwrap_or_else(|| "localhost".to_string());
 
-                    url::Host::parse(&host).map_err(|_| ClientError::InvalidDockerHost(host))
+                    url::Host::parse(&host)
+                        .map_err(|_| ConfigurationError::InvalidDockerHost(host).into())
                 } else {
                     Ok(url::Host::Domain("localhost".to_string()))
                 }

testcontainers/src/core/containers/sync_container.rs

@@ -91,13 +91,7 @@ where
     /// IPv4 interfaces.
     ///
     /// This method does **not** magically expose the given port, it simply performs a mapping on
-    /// the already exposed ports. If a docker container does not expose a port, this method will panic.
-    ///
-    /// # Panics
-    ///
-    /// This method panics if the given port is not mapped.
-    /// Testcontainers is designed to be used in tests only. If a certain port is not mapped, the container
-    /// is unlikely to be useful.
+    /// the already exposed ports. If a docker container does not expose a port, this method returns an error.
     pub fn get_host_port_ipv4(&self, internal_port: u16) -> Result<u16> {
         self.rt()
             .block_on(self.async_impl().get_host_port_ipv4(internal_port))
@@ -107,13 +101,7 @@ where
     /// IPv6 interfaces.
     ///
     /// This method does **not** magically expose the given port, it simply performs a mapping on
-    /// the already exposed ports. If a docker container does not expose a port, this method will panic.
-    ///
-    /// # Panics
-    ///
-    /// This method panics if the given port is not mapped.
-    /// Testcontainers is designed to be used in tests only. If a certain port is not mapped, the container
-    /// is unlikely to be useful.
+    /// the already exposed ports. If a docker container does not expose a port, this method returns an error.
     pub fn get_host_port_ipv6(&self, internal_port: u16) -> Result<u16> {
         self.rt()
             .block_on(self.async_impl().get_host_port_ipv6(internal_port))

testcontainers/src/core/env.rs

@@ -1,5 +1,6 @@
 mod config;
 
+pub use config::ConfigurationError;
 pub(crate) use config::{Command, Config};
 
 /// Abstracts over reading a value from the environment.
@@ -34,18 +35,16 @@ mod tests {
     }
 
     #[test]
-    #[should_panic(
-        expected = "unknown command 'foobar' provided via TESTCONTAINERS_COMMAND env variable"
-    )]
-    fn panics_on_unknown_command() {
-        let _ = "foobar".parse::<Command>();
+    fn errors_on_unknown_command() {
+        let res = "foobar".parse::<Command>();
+        assert!(res.is_err());
     }
 
     #[test]
     fn command_looks_up_testcontainers_env_variables() {
         let cmd = FakeEnvAlwaysKeep::get_env_value("TESTCONTAINERS_COMMAND").unwrap();
 
-        assert_eq!(cmd.parse::<Command>(), Ok(Command::Keep))
+        assert!(matches!(cmd.parse::<Command>(), Ok(Command::Keep)),)
     }
 
     #[test]

testcontainers/src/core/env/config.rs

@@ -7,6 +7,17 @@ use url::Url;
 
 use crate::core::env::GetEnvValue;
 
+#[derive(Debug, thiserror::Error)]
+pub enum ConfigurationError {
+    #[error("invalid DOCKER_HOST: {0}")]
+    InvalidDockerHost(String),
+    #[error("unknown command '{0}' provided via TESTCONTAINERS_COMMAND env variable")]
+    UnknownCommand(String),
+    #[cfg(feature = "properties-config")]
+    #[error("failed to load testcontainers properties: {0}")]
+    WrongPropertiesFormat(#[from] serde_java_properties::de::Error),
+}
+
 /// The default path to the Docker configuration file.
 const DEFAULT_DOCKER_CONFIG_PATH: &str = ".docker/config.json";
 
@@ -48,65 +59,71 @@ struct TestcontainersProperties {
 
 #[cfg(feature = "properties-config")]
 impl TestcontainersProperties {
-    async fn load() -> Option<Self> {
+    async fn load() -> Option<Result<Self, ConfigurationError>> {
         let home_dir = dirs::home_dir()?;
         let properties_path = home_dir.join(TESTCONTAINERS_PROPERTIES);
 
         let content = tokio::fs::read(properties_path).await.ok()?;
         let properties =
-            serde_java_properties::from_slice(&content).expect("Failed to parse properties");
+            serde_java_properties::from_slice(&content).map_err(ConfigurationError::from);
 
         Some(properties)
     }
 }
 
 impl Config {
-    pub(crate) async fn load<E>() -> Self
+    pub(crate) async fn load<E>() -> Result<Self, ConfigurationError>
     where
         E: GetEnvValue,
     {
         #[cfg(feature = "properties-config")]
         {
-            let env_config = Self::load_from_env_config::<E>().await;
-            let properties = TestcontainersProperties::load().await.unwrap_or_default();
+            let env_config = Self::load_from_env_config::<E>().await?;
+            let properties = TestcontainersProperties::load()
+                .await
+                .transpose()?
+                .unwrap_or_default();
 
             // Environment variables take precedence over properties
-            Self {
+            Ok(Self {
                 tc_host: env_config.tc_host.or(properties.tc_host),
                 host: env_config.host.or(properties.host),
                 tls_verify: env_config.tls_verify.or(properties.tls_verify),
                 cert_path: env_config.cert_path.or(properties.cert_path),
                 command: env_config.command,
                 docker_auth_config: env_config.docker_auth_config,
-            }
+            })
         }
         #[cfg(not(feature = "properties-config"))]
         Self::load_from_env_config::<E>().await
     }
 
-    async fn load_from_env_config<E>() -> Self
+    async fn load_from_env_config<E>() -> Result<Self, ConfigurationError>
     where
         E: GetEnvValue,
     {
         let host = E::get_env_value("DOCKER_HOST")
             .as_deref()
             .map(FromStr::from_str)
             .transpose()
-            .expect("Invalid DOCKER_HOST");
+            .map_err(|e: url::ParseError| ConfigurationError::InvalidDockerHost(e.to_string()))?;
         let tls_verify = E::get_env_value("DOCKER_TLS_VERIFY").map(|v| v == "1");
         let cert_path = E::get_env_value("DOCKER_CERT_PATH").map(PathBuf::from);
-        let command = E::get_env_value("TESTCONTAINERS_COMMAND").and_then(|v| v.parse().ok());
+        let command = E::get_env_value("TESTCONTAINERS_COMMAND")
+            .filter(|v| !v.trim().is_empty())
+            .map(|v| v.parse())
+            .transpose()?;
 
         let docker_auth_config = read_docker_auth_config::<E>().await;
 
-        Config {
+        Ok(Config {
             host,
             tc_host: None,
             command,
             tls_verify,
             cert_path,
             docker_auth_config,
-        }
+        })
     }
 
     /// The Docker host to use. The host is resolved in the following order:
@@ -174,15 +191,13 @@ pub(crate) enum Command {
 }
 
 impl FromStr for Command {
-    type Err = ();
+    type Err = ConfigurationError;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         match s {
             "keep" => Ok(Command::Keep),
             "remove" => Ok(Command::Remove),
-            other => {
-                panic!("unknown command '{other}' provided via TESTCONTAINERS_COMMAND env variable",)
-            }
+            other => Err(ConfigurationError::UnknownCommand(other.to_string())),
         }
     }
 }

testcontainers/src/core/error.rs

@@ -1,6 +1,7 @@
 use std::error::Error;
 
-use crate::core::{client::ClientError, logs::WaitLogError};
+use crate::core::logs::WaitLogError;
+pub use crate::core::{client::ClientError, env::ConfigurationError};
 
 pub type Result<T> = std::result::Result<T, TestcontainersError>;
 

testcontainers/src/lib.rs

@@ -61,7 +61,6 @@
 //! # Usage in production code
 //!
 //! Although nothing inherently prevents testcontainers from being used in production code, the library itself was not designed with that in mind.
-//! For example, many methods will panic if something goes wrong but because the usage is intended to be within tests, this is deemed acceptable.
 //!
 //! [tc_website]: https://testcontainers.org
 //! [`Docker`]: https://docker.com

testcontainers/src/runners/async_runner.rs

@@ -417,7 +417,7 @@ mod tests {
     }
 
     #[tokio::test]
-    async fn async_should_panic_when_non_bridged_network_selected() -> anyhow::Result<()> {
+    async fn async_should_return_error_when_non_bridged_network_selected() -> anyhow::Result<()> {
         let web_server = GenericImage::new("simple_web_server", "latest")
             .with_wait_for(WaitFor::message_on_stdout("server is ready"))
             .with_wait_for(WaitFor::seconds(1));

testcontainers/src/runners/sync_runner.rs

@@ -30,23 +30,22 @@ where
     I: Image,
 {
     fn start(self) -> Result<Container<I>> {
-        let runtime = build_sync_runner();
+        let runtime = build_sync_runner()?;
         let async_container = runtime.block_on(super::AsyncRunner::start(self))?;
 
         Ok(Container::new(runtime, async_container))
     }
 
     fn pull_image(self) -> Result<RunnableImage<I>> {
-        let runtime = build_sync_runner();
+        let runtime = build_sync_runner()?;
         runtime.block_on(super::AsyncRunner::pull_image(self))
     }
 }
 
-fn build_sync_runner() -> tokio::runtime::Runtime {
-    tokio::runtime::Builder::new_current_thread()
+fn build_sync_runner() -> Result<tokio::runtime::Runtime> {
+    Ok(tokio::runtime::Builder::new_current_thread()
         .enable_all()
-        .build()
-        .expect("failed to build sync runner")
+        .build()?)
 }
 
 #[cfg(test)]
@@ -166,9 +165,12 @@ mod tests {
     }
 
     #[test]
-    fn sync_rm_command_should_panic_on_invalid_container() {
+    fn sync_rm_command_should_return_error_on_invalid_container() {
         let res = runtime().block_on(docker_client().rm("!INVALID_NAME_DUE_TO_SYMBOLS!"));
-        assert!(res.is_err(), "should panic on invalid container name");
+        assert!(
+            res.is_err(),
+            "should return an error on invalid container name"
+        );
     }
 
     #[test]
@@ -208,7 +210,7 @@ mod tests {
     }
 
     #[test]
-    fn sync_should_panic_when_non_bridged_network_selected() -> anyhow::Result<()> {
+    fn sync_should_return_error_when_non_bridged_network_selected() -> anyhow::Result<()> {
         let web_server = GenericImage::new("simple_web_server", "latest")
             .with_wait_for(WaitFor::message_on_stdout("server is ready"))
             .with_wait_for(WaitFor::seconds(1));