treewide: unify command processing and improve error handling

src/cli.rs

@@ -1,13 +1,10 @@
 use clap::{Parser, Subcommand};
 use mrc::SOCKET_PATH;
+use mrc::commands::Commands;
 use mrc::interactive::InteractiveMode;
-use mrc::{
-    MrcError, Result, get_property, loadfile, playlist_clear, playlist_move, playlist_next,
-    playlist_prev, playlist_remove, quit, seek, set_property,
-};
-use serde_json::json;
+use mrc::{MrcError, Result};
 use std::path::PathBuf;
-use tracing::{debug, error, info};
+use tracing::{debug, error};
 
 #[derive(Parser)]
 #[command(author, version, about)]
@@ -109,98 +106,55 @@ async fn main() -> Result<()> {
 
     match cli.command {
         CommandOptions::Play { index } => {
-            if let Some(idx) = index {
-                info!("Playing media at index: {}", idx);
-                set_property("playlist-pos", &json!(idx), None).await?;
-            }
-            info!("Unpausing playback");
-            set_property("pause", &json!(false), None).await?;
+            Commands::play(index).await?;
         }
 
         CommandOptions::Pause => {
-            info!("Pausing playback");
-            set_property("pause", &json!(true), None).await?;
+            Commands::pause().await?;
         }
 
         CommandOptions::Stop => {
-            info!("Stopping playback and quitting MPV");
-            quit(None).await?;
+            Commands::stop().await?;
         }
 
         CommandOptions::Next => {
-            info!("Skipping to next item in the playlist");
-            playlist_next(None).await?;
+            Commands::next().await?;
         }
 
         CommandOptions::Prev => {
-            info!("Skipping to previous item in the playlist");
-            playlist_prev(None).await?;
+            Commands::prev().await?;
         }
 
         CommandOptions::Seek { seconds } => {
-            info!("Seeking to {} seconds", seconds);
-            seek(seconds.into(), None).await?;
+            Commands::seek_to(seconds.into()).await?;
         }
 
         CommandOptions::Move { index1, index2 } => {
-            info!("Moving item from index {} to {}", index1, index2);
-            playlist_move(index1, index2, None).await?;
+            Commands::move_item(index1, index2).await?;
         }
 
         CommandOptions::Remove { index } => {
-            if let Some(idx) = index {
-                info!("Removing item at index {}", idx);
-                playlist_remove(Some(idx), None).await?;
-            } else {
-                info!("Removing current item from playlist");
-                playlist_remove(None, None).await?;
-            }
+            Commands::remove_item(index).await?;
         }
 
         CommandOptions::Clear => {
-            info!("Clearing the playlist");
-            playlist_clear(None).await?;
+            Commands::clear_playlist().await?;
         }
 
         CommandOptions::List => {
-            info!("Listing playlist items");
-            if let Some(data) = get_property("playlist", None).await? {
-                let pretty_json =
-                    serde_json::to_string_pretty(&data).map_err(MrcError::ParseError)?;
-                println!("{}", pretty_json);
-            }
+            Commands::list_playlist().await?;
         }
 
         CommandOptions::Add { filenames } => {
-            if filenames.is_empty() {
-                let e = "No files provided to add to the playlist";
-                error!("{}", e);
-                return Err(MrcError::InvalidInput(e.to_string()));
-            }
-
-            info!("Adding {} files to the playlist", filenames.len());
-            for filename in filenames {
-                loadfile(&filename, true, None).await?;
-            }
+            Commands::add_files(&filenames).await?;
         }
 
         CommandOptions::Replace { filenames } => {
-            info!("Replacing current playlist with {} files", filenames.len());
-            if let Some(first_file) = filenames.first() {
-                loadfile(first_file, false, None).await?;
-                for filename in &filenames[1..] {
-                    loadfile(filename, true, None).await?;
-                }
-            }
+            Commands::replace_playlist(&filenames).await?;
         }
 
         CommandOptions::Prop { properties } => {
-            info!("Fetching properties: {:?}", properties);
-            for property in properties {
-                if let Some(data) = get_property(&property, None).await? {
-                    println!("{property}: {data}");
-                }
-            }
+            Commands::get_properties(&properties).await?;
         }
 
         CommandOptions::Interactive => {

src/commands.rs

@@ -0,0 +1,217 @@
+//! Command processing module for MRC.
+//!
+//! # Examples
+//!
+//! ```rust
+//! use mrc::commands::Commands;
+//!
+//! # async fn example() -> mrc::Result<()> {
+//! // Play media at a specific playlist index
+//! Commands::play(Some(0)).await?;
+//!
+//! // Pause playback
+//! Commands::pause().await?;
+//!
+//! // Add files to playlist
+//! let files = vec!["movie1.mp4".to_string(), "movie2.mp4".to_string()];
+//! Commands::add_files(&files).await?;
+//! # Ok(())
+//! # }
+//! ```
+
+use crate::{
+    MrcError, Result, get_property, loadfile, playlist_clear, playlist_move, playlist_next,
+    playlist_prev, playlist_remove, quit, seek, set_property,
+};
+use serde_json::json;
+use tracing::info;
+
+/// Centralized command processing for MPV operations.
+pub struct Commands;
+
+impl Commands {
+    /// Plays media, optionally at a specific playlist index.
+    ///
+    /// If an index is provided, seeks to that position in the playlist first.
+    /// Always unpauses playback regardless of whether an index is specified.
+    ///
+    /// # Arguments
+    ///
+    /// * `index` - Optional playlist index to play from
+    pub async fn play(index: Option<usize>) -> Result<()> {
+        if let Some(idx) = index {
+            info!("Playing media at index: {}", idx);
+            set_property("playlist-pos", &json!(idx), None).await?;
+        }
+        info!("Unpausing playback");
+        set_property("pause", &json!(false), None).await?;
+        Ok(())
+    }
+
+    /// Pauses the currently playing media.
+    pub async fn pause() -> Result<()> {
+        info!("Pausing playback");
+        set_property("pause", &json!(true), None).await?;
+        Ok(())
+    }
+
+    /// Stops playback and quits MPV.
+    ///
+    /// This is a destructive operation that will terminate the MPV process.
+    pub async fn stop() -> Result<()> {
+        info!("Stopping playback and quitting MPV");
+        quit(None).await?;
+        Ok(())
+    }
+
+    /// Advances to the next item in the playlist.
+    pub async fn next() -> Result<()> {
+        info!("Skipping to next item in the playlist");
+        playlist_next(None).await?;
+        Ok(())
+    }
+
+    /// Goes back to the previous item in the playlist.
+    pub async fn prev() -> Result<()> {
+        info!("Skipping to previous item in the playlist");
+        playlist_prev(None).await?;
+        Ok(())
+    }
+
+    /// Seeks to a specific time position in the current media.
+    ///
+    /// # Arguments
+    ///
+    /// * `seconds` - The time position to seek to, in seconds
+    pub async fn seek_to(seconds: f64) -> Result<()> {
+        info!("Seeking to {} seconds", seconds);
+        seek(seconds, None).await?;
+        Ok(())
+    }
+
+    /// Moves a playlist item from one index to another.
+    ///
+    /// # Arguments
+    ///
+    /// * `from_index` - The current index of the item to move
+    /// * `to_index` - The target index to move the item to
+    pub async fn move_item(from_index: usize, to_index: usize) -> Result<()> {
+        info!("Moving item from index {} to {}", from_index, to_index);
+        playlist_move(from_index, to_index, None).await?;
+        Ok(())
+    }
+
+    /// Removes an item from the playlist.
+    ///
+    /// # Arguments
+    ///
+    /// * `index` - Optional index of the item to remove. If `None`, removes the current item
+    pub async fn remove_item(index: Option<usize>) -> Result<()> {
+        if let Some(idx) = index {
+            info!("Removing item at index {}", idx);
+            playlist_remove(Some(idx), None).await?;
+        } else {
+            info!("Removing current item from playlist");
+            playlist_remove(None, None).await?;
+        }
+        Ok(())
+    }
+
+    /// Clears all items from the playlist.
+    pub async fn clear_playlist() -> Result<()> {
+        info!("Clearing the playlist");
+        playlist_clear(None).await?;
+        Ok(())
+    }
+
+    /// Lists all items in the playlist.
+    ///
+    /// This outputs the playlist as formatted JSON to stdout.
+    pub async fn list_playlist() -> Result<()> {
+        info!("Listing playlist items");
+        if let Some(data) = get_property("playlist", None).await? {
+            let pretty_json = serde_json::to_string_pretty(&data).map_err(MrcError::ParseError)?;
+            println!("{}", pretty_json);
+        }
+        Ok(())
+    }
+
+    /// Adds multiple files to the playlist.
+    ///
+    /// # Arguments
+    ///
+    /// * `filenames` - A slice of file paths to add to the playlist
+    ///
+    /// # Errors
+    ///
+    /// Returns [`MrcError::InvalidInput`] if the filenames slice is empty.
+    pub async fn add_files(filenames: &[String]) -> Result<()> {
+        if filenames.is_empty() {
+            let e = "No files provided to add to the playlist";
+            return Err(MrcError::InvalidInput(e.to_string()));
+        }
+
+        info!("Adding {} files to the playlist", filenames.len());
+        for filename in filenames {
+            loadfile(filename, true, None).await?;
+        }
+        Ok(())
+    }
+
+    /// Replaces the current playlist with new files.
+    ///
+    /// The first file replaces the current playlist, and subsequent files are appended.
+    ///
+    /// # Arguments
+    ///
+    /// * `filenames` - A slice of file paths to replace the playlist with
+    pub async fn replace_playlist(filenames: &[String]) -> Result<()> {
+        info!("Replacing current playlist with {} files", filenames.len());
+        if let Some(first_file) = filenames.first() {
+            loadfile(first_file, false, None).await?;
+            for filename in &filenames[1..] {
+                loadfile(filename, true, None).await?;
+            }
+        }
+        Ok(())
+    }
+
+    /// Retrieves and displays multiple MPV properties.
+    ///
+    /// # Arguments
+    ///
+    /// * `properties` - A slice of property names to retrieve
+    pub async fn get_properties(properties: &[String]) -> Result<()> {
+        info!("Fetching properties: {:?}", properties);
+        for property in properties {
+            if let Some(data) = get_property(property, None).await? {
+                println!("{property}: {data}");
+            }
+        }
+        Ok(())
+    }
+
+    /// Retrieves and displays a single MPV property.
+    ///
+    /// # Arguments
+    ///
+    /// * `property` - The name of the property to retrieve
+    pub async fn get_single_property(property: &str) -> Result<()> {
+        if let Some(data) = get_property(property, None).await? {
+            println!("{property}: {data}");
+        }
+        Ok(())
+    }
+
+    /// Sets an MPV property to a specific value.
+    ///
+    /// # Arguments
+    ///
+    /// * `property` - The name of the property to set
+    /// * `value` - The JSON value to set the property to
+    pub async fn set_single_property(property: &str, value: &serde_json::Value) -> Result<()> {
+        set_property(property, value, None).await?;
+        println!("Set {property} to {value}");
+        Ok(())
+    }
+}

src/interactive.rs

@@ -1,10 +1,7 @@
-use crate::{
-    MrcError, Result, get_property, loadfile, playlist_clear, playlist_next, playlist_prev, quit,
-    seek, set_property,
-};
+use crate::commands::Commands;
+use crate::{MrcError, Result};
 use serde_json::json;
 use std::io::{self, Write};
-use tracing::info;
 
 pub struct InteractiveMode;
 
@@ -76,85 +73,66 @@ impl InteractiveMode {
 
         match parts.as_slice() {
             ["play"] => {
-                info!("Unpausing playback");
-                set_property("pause", &json!(false), None).await?;
+                Commands::play(None).await?;
             }
 
             ["play", index] => {
                 if let Ok(idx) = index.parse::<usize>() {
-                    info!("Playing media at index: {}", idx);
-                    set_property("playlist-pos", &json!(idx), None).await?;
-                    set_property("pause", &json!(false), None).await?;
+                    Commands::play(Some(idx)).await?;
                 } else {
                     println!("Invalid index: {}", index);
                 }
             }
 
             ["pause"] => {
-                info!("Pausing playback");
-                set_property("pause", &json!(true), None).await?;
+                Commands::pause().await?;
             }
 
             ["stop"] => {
-                info!("Stopping playback and quitting MPV");
-                quit(None).await?;
+                Commands::stop().await?;
             }
 
             ["next"] => {
-                info!("Skipping to next item in the playlist");
-                playlist_next(None).await?;
+                Commands::next().await?;
             }
 
             ["prev"] => {
-                info!("Skipping to previous item in the playlist");
-                playlist_prev(None).await?;
+                Commands::prev().await?;
             }
 
             ["seek", seconds] => {
                 if let Ok(sec) = seconds.parse::<i32>() {
-                    info!("Seeking to {} seconds", sec);
-                    seek(sec.into(), None).await?;
+                    Commands::seek_to(sec.into()).await?;
                 } else {
                     println!("Invalid seconds: {}", seconds);
                 }
             }
 
             ["clear"] => {
-                info!("Clearing the playlist");
-                playlist_clear(None).await?;
+                Commands::clear_playlist().await?;
             }
 
             ["list"] => {
-                info!("Listing playlist items");
-                if let Some(data) = get_property("playlist", None).await? {
-                    let pretty_json =
-                        serde_json::to_string_pretty(&data).map_err(MrcError::ParseError)?;
-                    println!("{}", pretty_json);
-                }
+                Commands::list_playlist().await?;
             }
 
             ["add", files @ ..] => {
-                if files.is_empty() {
+                let file_strings: Vec<String> = files.iter().map(|s| s.to_string()).collect();
+                if file_strings.is_empty() {
                     println!("No files provided to add to the playlist");
                 } else {
-                    info!("Adding {} files to the playlist", files.len());
-                    for file in files {
-                        loadfile(file, true, None).await?;
-                    }
+                    Commands::add_files(&file_strings).await?;
                 }
             }
 
             ["get", property] => {
-                if let Some(data) = get_property(property, None).await? {
-                    println!("{}: {}", property, data);
-                }
+                Commands::get_single_property(property).await?;
             }
 
             ["set", property, value] => {
                 let json_value = serde_json::from_str::<serde_json::Value>(value)
                     .unwrap_or_else(|_| json!(value));
-                set_property(property, &json_value, None).await?;
-                println!("Set {} to {}", property, value);
+                Commands::set_single_property(property, &json_value).await?;
             }
 
             _ => {

src/lib.rs

@@ -39,6 +39,7 @@
 //! Default path for the MPV IPC socket: `/tmp/mpvsocket`
 //!
 
+pub mod commands;
 pub mod interactive;
 
 use serde_json::{Value, json};

src/server.rs

@@ -4,15 +4,11 @@ use std::sync::Arc;
 
 use clap::Parser;
 use native_tls::{Identity, TlsAcceptor as NativeTlsAcceptor};
-use serde_json::json;
 use tokio::io::{AsyncReadExt, AsyncWriteExt};
 use tokio_native_tls::TlsAcceptor;
-use tracing::{debug, error, info};
+use tracing::{debug, error, info, warn};
 
-use mrc::{
-    MrcError, Result as MrcResult, get_property, playlist_clear, playlist_next, playlist_prev,
-    quit, seek, set_property,
-};
+use mrc::{MrcError, Result as MrcResult, commands::Commands};
 
 #[derive(Parser)]
 #[command(author, version, about)]
@@ -56,111 +52,133 @@ async fn handle_connection(
     let auth_token = match env::var("AUTH_TOKEN") {
         Ok(token) => token,
         Err(_) => {
-            error!("Authentication token is not set. Connection cannot be accepted.");
-            stream.write_all(b"Authentication token not set\n").await?;
-
-            // You know what? I do not care to panic when the authentication token is
-            // missing in the environment. Start the goddamned server and hell, even
-            // accept incoming connections. Authenticated requests will be refused
-            // when the token is incorrect or not set, so we can simply continue here.
+            warn!("AUTH_TOKEN environment variable not set. Authentication disabled.");
+            let response = "HTTP/1.1 401 Unauthorized\r\nContent-Length: 29\r\n\r\nAuthentication token not set\n";
+            stream.write_all(response.as_bytes()).await?;
             return Ok(());
         }
     };
 
-    if token != auth_token {
-        stream.write_all(b"Authentication failed\n").await?;
+    if token.is_empty() || token != auth_token {
+        warn!(
+            "Authentication failed for token: {}",
+            if token.is_empty() {
+                "<empty>"
+            } else {
+                "<redacted>"
+            }
+        );
+        let response =
+            "HTTP/1.1 401 Unauthorized\r\nContent-Length: 21\r\n\r\nAuthentication failed\n";
+        stream.write_all(response.as_bytes()).await?;
         return Ok(());
     }
 
-    info!("Client authenticated");
-    stream.write_all(b"Authenticated\n").await?;
+    info!("Client authenticated successfully");
 
-    let command = request.split("\r\n\r\n").last().unwrap_or("");
-    info!("Received command: {}", command);
+    let command = request.split("\r\n\r\n").last().unwrap_or("").trim();
 
-    let response = match process_command(command.trim()).await {
-        Ok(response) => response,
+    if command.is_empty() {
+        warn!("Received empty command");
+        let response =
+            "HTTP/1.1 400 Bad Request\r\nContent-Length: 20\r\n\r\nNo command provided\n";
+        stream.write_all(response.as_bytes()).await?;
+        return Ok(());
+    }
+
+    info!("Processing command: {}", command);
+
+    let (status_code, response_body) = match process_command(command).await {
+        Ok(response) => ("200 OK", response),
         Err(e) => {
-            error!("Error processing command: {}", e);
-            format!("Error: {:?}", e)
+            error!("Error processing command '{}': {}", command, e);
+            ("400 Bad Request", format!("Error: {}\n", e))
         }
     };
 
     let http_response = format!(
-        "HTTP/1.1 200 OK\r\nContent-Length: {}\r\n\r\n{}",
-        response.len(),
-        response
+        "HTTP/1.1 {}\r\nContent-Length: {}\r\nContent-Type: text/plain\r\n\r\n{}",
+        status_code,
+        response_body.len(),
+        response_body
     );
+
     stream.write_all(http_response.as_bytes()).await?;
 
     Ok(())
 }
 
 async fn process_command(command: &str) -> MrcResult<String> {
-    match command {
-        "pause" => {
-            info!("Pausing playback");
-            set_property("pause", &json!(true), None).await?;
+    let parts: Vec<&str> = command.split_whitespace().collect();
+
+    match parts.as_slice() {
+        ["pause"] => {
+            Commands::pause().await?;
             Ok("Paused playback\n".to_string())
         }
 
-        "play" => {
-            info!("Unpausing playback");
-            set_property("pause", &json!(false), None).await?;
+        ["play"] => {
+            Commands::play(None).await?;
             Ok("Resumed playback\n".to_string())
         }
 
-        "stop" => {
-            info!("Stopping playback and quitting MPV");
-            quit(None).await?;
+        ["play", index] => {
+            if let Ok(idx) = index.parse::<usize>() {
+                Commands::play(Some(idx)).await?;
+                Ok(format!("Playing from index {}\n", idx))
+            } else {
+                Err(MrcError::InvalidInput(format!("Invalid index: {}", index)))
+            }
+        }
+
+        ["stop"] => {
+            Commands::stop().await?;
             Ok("Stopped playback\n".to_string())
         }
 
-        "next" => {
-            info!("Skipping to next item in the playlist");
-            playlist_next(None).await?;
+        ["next"] => {
+            Commands::next().await?;
             Ok("Skipped to next item\n".to_string())
         }
 
-        "prev" => {
-            info!("Skipping to previous item in the playlist");
-            playlist_prev(None).await?;
+        ["prev"] => {
+            Commands::prev().await?;
             Ok("Skipped to previous item\n".to_string())
         }
 
-        "seek" => {
-            let parts: Vec<&str> = command.split_whitespace().collect();
-            if let Some(seconds) = parts.get(1) {
-                if let Ok(sec) = seconds.parse::<i32>() {
-                    info!("Seeking to {} seconds", sec);
-                    seek(sec.into(), None).await?;
-                    return Ok(format!("Seeking to {} seconds\n", sec));
-                }
+        ["seek", seconds] => {
+            if let Ok(sec) = seconds.parse::<f64>() {
+                Commands::seek_to(sec).await?;
+                Ok(format!("Seeking to {} seconds\n", sec))
+            } else {
+                Err(MrcError::InvalidInput(format!(
+                    "Invalid seconds: {}",
+                    seconds
+                )))
             }
-            Err(MrcError::InvalidInput("Invalid seek command".to_string()))
         }
 
-        "clear" => {
-            info!("Clearing the playlist");
-            playlist_clear(None).await?;
+        ["clear"] => {
+            Commands::clear_playlist().await?;
             Ok("Cleared playlist\n".to_string())
         }
 
-        "list" => {
-            info!("Listing playlist items");
-            match get_property("playlist", None).await {
-                Ok(Some(data)) => {
+        ["list"] => {
+            // For server response, we need to capture the output differently
+            // since Commands::list_playlist() prints to stdout
+            match mrc::get_property("playlist", None).await? {
+                Some(data) => {
                     let pretty_json =
                         serde_json::to_string_pretty(&data).map_err(MrcError::ParseError)?;
-                    Ok(format!("Playlist: {}", pretty_json))
+                    Ok(format!("Playlist: {}\n", pretty_json))
                 }
-                Ok(None) => Err(MrcError::PropertyNotFound("playlist".to_string())),
-                Err(e) => Err(e),
+                None => Ok("Playlist is empty\n".to_string()),
             }
         }
+
         _ => Err(MrcError::InvalidInput(format!(
-            "Unknown command: {}",
-            command
+            "Unknown command: {}. Available commands: pause, play [index], stop, next, prev, seek <seconds>, clear, list",
+            command.trim()
         ))),
     }
 }