webdriver refactor

Signed-off-by: Andrei Gherghescu <[email protected]>

Author: andrei-ng

plotly_static/src/lib.rs

@@ -1311,7 +1311,7 @@ mod tests {
         assert!(dst.exists());
         let metadata = std::fs::metadata(&dst).expect("Could not retrieve file metadata");
         let file_size = metadata.len();
-        assert!(file_size > 0,);
+        assert!(file_size > 900000,);
         // assert!(std::fs::remove_file(dst.as_path()).is_ok());
     }
 

plotly_static/src/webdriver.rs

@@ -208,95 +208,116 @@ impl WebDriver {
 
     fn spawn_process(&self, driver_path: &Option<PathBuf>, port: u32) -> Result<Child> {
         let driver_path = driver_path.as_ref().unwrap(); // Safe unwrap since we validated above
-        let mut command = Command::new(driver_path);
-        command.arg(format!("--port={port}"));
-
-        // Add verbose flag only for chromedriver
-        #[cfg(feature = "chromedriver")]
-        command.arg("--verbose");
-
-        command
-            .stdin(Stdio::piped())
-            .stdout(Stdio::piped())
-            .stderr(Stdio::piped());
-
-        info!(
-            "Executing command: {:?} {:?}",
-            command.get_program(),
-            command.get_args()
-        );
 
-        #[cfg(target_os = "windows")]
-        {
-            use std::os::windows::process::CommandExt;
-            // Try with CREATE_NO_WINDOW for headless operation
-            command.creation_flags(0x08000000); // CREATE_NO_WINDOW flag
-        }
+        let mut command = Self::create_command(driver_path, port);
+        Self::log_command(&command);
 
-        // Try to spawn the process
         match command.spawn() {
             Ok(child) => Ok(child),
             Err(e) => {
+                #[cfg(not(target_os = "windows"))]
+                {
+                    Err(self.handle_spawn_error(e, &command, "standard method"))
+                }
                 #[cfg(target_os = "windows")]
                 {
                     // If CREATE_NO_WINDOW fails, try without any special flags
                     error!("Failed to spawn with CREATE_NO_WINDOW: {e}");
                     error!("Trying without special creation flags...");
 
-                    let mut fallback_command = Command::new(driver_path);
-                    fallback_command.arg(format!("--port={port}"));
-
-                    #[cfg(feature = "chromedriver")]
-                    fallback_command.arg("--verbose");
-
-                    fallback_command
-                        .stdin(Stdio::piped())
-                        .stdout(Stdio::piped())
-                        .stderr(Stdio::piped());
+                    let mut fallback_command = Self::standard_command(driver_path, port);
+                    Self::log_command(&fallback_command);
 
                     match fallback_command.spawn() {
                         Ok(child) => {
                             info!("Successfully spawned WebDriver without special creation flags");
                             Ok(child)
                         }
                         Err(fallback_e) => {
-                            error!("Failed to spawn '{WEBDRIVER_BIN}' with fallback method: {fallback_e}");
                             error!("Original error: {e}");
                             error!("Fallback error: {fallback_e}");
-                            error!(
-                                "Command was: {:?} {:?}",
-                                command.get_program(),
-                                command.get_args()
-                            );
-                            error!("Windows: Check if antivirus is blocking the process");
-                            error!("Windows: Check if the binary has proper permissions");
-                            error!(
-                                "WebDriver diagnostics after spawn failure:\n{}",
-                                self.get_diagnostics()
-                            );
-                            Err(anyhow!("Failed to spawn '{WEBDRIVER_BIN}': {}", fallback_e))
+                            Err(self.handle_spawn_error(
+                                fallback_e,
+                                &fallback_command,
+                                "fallback method",
+                            ))
                         }
                     }
                 }
-
-                #[cfg(not(target_os = "windows"))]
-                {
-                    error!("Failed to spawn '{WEBDRIVER_BIN}': {e}");
-                    error!(
-                        "Command was: {:?} {:?}",
-                        command.get_program(),
-                        command.get_args()
-                    );
-                    error!(
-                        "WebDriver diagnostics after spawn failure:\n{}",
-                        self.get_diagnostics()
-                    );
-                    Err(anyhow!("Failed to spawn '{WEBDRIVER_BIN}': {}", e))
-                }
             }
         }
     }
 
+    /// Creates a command with standard configuration (no Windows flags)
+    fn standard_command(driver_path: &PathBuf, port: u32) -> Command {
+        let mut command = Command::new(driver_path);
+        command.arg(format!("--port={port}"));
+
+        // Add verbose flag only for chromedriver
+        #[cfg(feature = "chromedriver")]
+        command.arg("--verbose");
+
+        command
+            .stdin(Stdio::piped())
+            .stdout(Stdio::piped())
+            .stderr(Stdio::piped());
+
+        command
+    }
+
+    /// Creates a command with Windows-specific flags
+    #[cfg(target_os = "windows")]
+    fn create_command(driver_path: &PathBuf, port: u32) -> Command {
+        use std::os::windows::process::CommandExt;
+
+        let mut command = Self::standard_command(driver_path, port);
+        // Try with CREATE_NO_WINDOW for headless operation
+        command.creation_flags(0x08000000); // CREATE_NO_WINDOW flag
+        command
+    }
+
+    #[cfg(not(target_os = "windows"))]
+    fn create_command(driver_path: &PathBuf, port: u32) -> Command {
+        Self::standard_command(driver_path, port)
+    }
+
+    /// Logs command execution details
+    fn log_command(command: &Command) {
+        info!(
+            "Executing command: {:?} {:?}",
+            command.get_program(),
+            command.get_args()
+        );
+    }
+
+    /// Handles spawn errors with appropriate logging and diagnostics
+    fn handle_spawn_error(
+        &self,
+        e: std::io::Error,
+        command: &Command,
+        attempt: &str,
+    ) -> anyhow::Error {
+        error!("Failed to spawn '{WEBDRIVER_BIN}' with {attempt}: {e}");
+        error!(
+            "Command was: {:?} {:?}",
+            command.get_program(),
+            command.get_args()
+        );
+
+        #[cfg(target_os = "windows")]
+        if attempt == "CREATE_NO_WINDOW" {
+            error!("Windows: Check if antivirus is blocking the process");
+            error!("Windows: Check if the binary has proper permissions");
+        }
+
+        error!(
+            "WebDriver diagnostics after spawn failure:\n{}",
+            self.get_diagnostics()
+        );
+
+        anyhow!("Failed to spawn '{WEBDRIVER_BIN}': {}", e)
+    }
+
     fn setup_output_monitoring(&self, child: &mut Child, port: u32) {
         // Monitor stderr
         if let Some(stderr) = child.stderr.take() {