Merge branch 'error-handling' into 'main'

Error handling

Closes #13

See merge request gustav/mum!3

8 files changed, 88 insertions, 44 deletions

diff --git a/mumctl/Cargo.toml b/mumctl/Cargo.toml
index 1f2f727..c955afa 100644
--- a/mumctl/Cargo.toml
+++ b/mumctl/Cargo.toml
@@ -12,6 +12,7 @@ edition = "2018"
 mumlib = { path = "../mumlib" }
 
 clap = { version = "2.33", features = ["yaml"] }
+colored = "2.0"
 log = "0.4"
 ipc-channel = "0.14"
 
diff --git a/mumctl/src/main.rs b/mumctl/src/main.rs
index 124cc8c..ae4acc5 100644
--- a/mumctl/src/main.rs
+++ b/mumctl/src/main.rs
@@ -4,6 +4,15 @@ use log::*;
 use mumlib::command::{Command, CommandResponse};
 use mumlib::setup_logger;
 use std::{fs, io};
+use colored::Colorize;
+
+macro_rules! err_print {
+    ($func:expr) => {
+        if let Err(e) = $func {
+            println!("{} {}", "error:".red(), e);
+        }
+    };
+}
 
 fn main() {
     setup_logger();
@@ -41,36 +50,40 @@ fn main() {
                          .long("fish")));
 
     let matches = app.clone().get_matches();
-
-    debug!("Matching clap");
+    
     if let Some(matches) = matches.subcommand_matches("server") {
         if let Some(matches) = matches.subcommand_matches("connect") {
             let host = matches.value_of("host").unwrap();
             let username = matches.value_of("username").unwrap();
-            send_command(Command::ServerConnect {
+            err_print!(send_command(Command::ServerConnect {
                 host: host.to_string(),
                 port: 64738u16, //TODO
                 username: username.to_string(),
                 accept_invalid_cert: true, //TODO
-            }).unwrap();
+            }));
         } else if let Some(_) = matches.subcommand_matches("disconnect") {
-            send_command(Command::ServerDisconnect).unwrap();
+            err_print!(send_command(Command::ServerDisconnect));
         }
     } else if let Some(matches) = matches.subcommand_matches("channel") {
         if let Some(_matches) = matches.subcommand_matches("list") {
-            let res = send_command(Command::ChannelList).unwrap().unwrap();
-            println!("{:#?}", res);
-            /*if matches.is_present("short") {
-                None //TODO
-            } else {
-                None //TODO
-            };*/
+            match send_command(Command::ChannelList) {
+                Ok(res) => {
+                    println!("{:#?}", res.unwrap());
+                }
+                Err(e) => println!("{} {}", "error:".red(), e),
+            }
         } else if let Some(matches) = matches.subcommand_matches("connect") {
-            send_command(Command::ChannelJoin {
+            err_print!(send_command(Command::ChannelJoin {
                 channel_id: matches.value_of("channel").unwrap().parse::<u32>().unwrap()
-            }).unwrap();
+            }));
         }
     } else if let Some(_matches) = matches.subcommand_matches("status") {
+        match send_command(Command::Status) {
+            Ok(res) => {
+                println!("{:#?}", res.unwrap());
+            }
+            Err(e) => println!("{} {}", "error:".red(), e),
+        }
         let res = send_command(Command::Status).unwrap().unwrap();
         println!("{:#?}", res);
     } else if let Some(matches) = matches.subcommand_matches("completions") {
@@ -91,13 +104,13 @@ fn main() {
     };
 }
 
-fn send_command(command: Command) -> Result<Option<CommandResponse>, ()> {
-    let (tx_client, rx_client): (IpcSender<Result<Option<CommandResponse>, ()>>,
-                                 IpcReceiver<Result<Option<CommandResponse>, ()>>) = ipc::channel().unwrap();
+fn send_command(command: Command) -> mumlib::error::Result<Option<CommandResponse>> {
+    let (tx_client, rx_client) = ipc::channel::<mumlib::error::Result<Option<CommandResponse>>>().unwrap();
 
     let server_name = fs::read_to_string("/var/tmp/mumd-oneshot").unwrap(); //TODO don't panic
 
     let tx0 = IpcSender::connect(server_name).unwrap();
+
     tx0.send((command, tx_client)).unwrap();
 
     rx_client.recv().unwrap()
diff --git a/mumd/src/command.rs b/mumd/src/command.rs
index 9adf7d8..57eaaa3 100644
--- a/mumd/src/command.rs
+++ b/mumd/src/command.rs
@@ -8,12 +8,10 @@ use tokio::sync::mpsc;
 
 pub async fn handle(
     state: Arc<Mutex<State>>,
-    mut command_receiver: mpsc::UnboundedReceiver<(Command, IpcSender<Result<Option<CommandResponse>, ()>>)>,
+    mut command_receiver: mpsc::UnboundedReceiver<(Command, IpcSender<mumlib::error::Result<Option<CommandResponse>>>)>,
 ) {
     debug!("Begin listening for commands");
-    loop {
-        debug!("Enter loop");
-        let command = command_receiver.recv().await.unwrap();
+    while let Some(command) = command_receiver.recv().await {
         debug!("Received command {:?}", command.0);
         let mut state = state.lock().unwrap();
         let (wait_for_connected, command_response) = state.handle_command(command.0).await;
diff --git a/mumd/src/main.rs b/mumd/src/main.rs
index 14a43c1..3a0d7ec 100644
--- a/mumd/src/main.rs
+++ b/mumd/src/main.rs
@@ -27,7 +27,7 @@ async fn main() {
     // For simplicity we don't deal with re-syncing, real applications would have to.
     let (crypt_state_sender, crypt_state_receiver) = mpsc::channel::<ClientCryptState>(1); // crypt state should always be consumed before sending a new one
     let (packet_sender, packet_receiver) = mpsc::unbounded_channel::<ControlPacket<Serverbound>>();
-    let (command_sender, command_receiver) = mpsc::unbounded_channel::<(Command, IpcSender<Result<Option<CommandResponse>, ()>>)>();
+    let (command_sender, command_receiver) = mpsc::unbounded_channel::<(Command, IpcSender<mumlib::error::Result<Option<CommandResponse>>>)>();
     let (connection_info_sender, connection_info_receiver) =
         watch::channel::<Option<ConnectionInfo>>(None);
 
@@ -61,17 +61,17 @@ async fn main() {
 }
 
 fn receive_oneshot_commands(
-    command_sender: mpsc::UnboundedSender<(Command, IpcSender<Result<Option<CommandResponse>, ()>>)>,
+    command_sender: mpsc::UnboundedSender<(Command, IpcSender<mumlib::error::Result<Option<CommandResponse>>>)>,
 ) {
     loop {
         // create listener
-        let (server, server_name): (IpcOneShotServer<(Command, IpcSender<Result<Option<CommandResponse>, ()>>)>, String) = IpcOneShotServer::new().unwrap();
+        let (server, server_name): (IpcOneShotServer<(Command, IpcSender<mumlib::error::Result<Option<CommandResponse>>>)>, String) = IpcOneShotServer::new().unwrap();
         fs::write("/var/tmp/mumd-oneshot", &server_name).unwrap();
         debug!("Listening to {}", server_name);
 
         // receive command and response channel
-        let (_, conn): (_, (Command, IpcSender<Result<Option<CommandResponse>, ()>>)) = server.accept().unwrap();
-        debug!("Sending to command handler: {:#?}", conn.0);
+        let (_, conn): (_, (Command, IpcSender<mumlib::error::Result<Option<CommandResponse>>>)) = server.accept().unwrap();
+        debug!("Sending command {:?} to command handler", conn.0);
         command_sender.send(conn).unwrap();
     }
 }
diff --git a/mumd/src/network/mod.rs b/mumd/src/network.rs
index 1a31ee2..1a31ee2 100644
--- a/mumd/src/network/mod.rs
+++ b/mumd/src/network.rs
diff --git a/mumd/src/state.rs b/mumd/src/state.rs
index 72197f6..82d671e 100644
--- a/mumd/src/state.rs
+++ b/mumd/src/state.rs
@@ -9,6 +9,7 @@ use mumlib::command::{Command, CommandResponse};
 use mumlib::state::Server;
 use std::net::ToSocketAddrs;
 use tokio::sync::{mpsc, watch};
+use mumlib::error::Error;
 
 #[derive(Clone, Debug, Eq, PartialEq)]
 pub enum StatePhase {
@@ -50,12 +51,16 @@ impl State {
     pub async fn handle_command(
         &mut self,
         command: Command,
-    ) -> (bool, Result<Option<CommandResponse>, ()>) {
+    ) -> (bool, mumlib::error::Result<Option<CommandResponse>>) {
         match command {
             Command::ChannelJoin { channel_id } => {
                 if !matches!(*self.phase_receiver().borrow(), StatePhase::Connected) {
-                    warn!("Not connected");
-                    return (false, Err(()));
+                    return (false, Err(Error::DisconnectedError));
+                }
+                if let Some(server) = &self.server {
+                    if !server.channels().contains_key(&channel_id) {
+                        return (false, Err(Error::InvalidChannelIdError(channel_id)));
+                    }
                 }
                 let mut msg = msgs::UserState::new();
                 msg.set_session(self.session_id.unwrap());
@@ -65,12 +70,9 @@ impl State {
             }
             Command::ChannelList => {
                 if !matches!(*self.phase_receiver().borrow(), StatePhase::Connected) {
-                    warn!("Not connected");
-                    return (false, Err(()));
+                    return (false, Err(Error::DisconnectedError));
                 }
-                (
-                    false,
-                    Ok(Some(CommandResponse::ChannelList {
+                (false, Ok(Some(CommandResponse::ChannelList {
                         channels: self.server.as_ref().unwrap().channels().clone(),
                     })),
                 )
@@ -82,8 +84,7 @@ impl State {
                 accept_invalid_cert,
             } => {
                 if !matches!(*self.phase_receiver().borrow(), StatePhase::Disconnected) {
-                    warn!("Tried to connect to a server while already connected");
-                    return (false, Err(()));
+                    return (false, Err(Error::AlreadyConnectedError));
                 }
                 self.server = Some(Server::new());
                 self.username = Some(username);
@@ -91,11 +92,14 @@ impl State {
                     .0
                     .broadcast(StatePhase::Connecting)
                     .unwrap();
-                let socket_addr = (host.as_ref(), port)
-                    .to_socket_addrs()
-                    .expect("Failed to parse server address")
-                    .next()
-                    .expect("Failed to resolve server address");
+
+                let socket_addr = match (host.as_ref(), port).to_socket_addrs().map(|mut e| e.next()) {
+                    Ok(Some(v)) => v,
+                    _ => {
+                        warn!("Error parsing server addr");
+                        return (false, Err(Error::InvalidServerAddrError(host, port)));
+                    }
+                };
                 self.connection_info_sender
                     .broadcast(Some(ConnectionInfo::new(
                         socket_addr,
@@ -107,18 +111,21 @@ impl State {
             }
             Command::Status => {
                 if !matches!(*self.phase_receiver().borrow(), StatePhase::Connected) {
-                    warn!("Not connected");
-                    return (false, Err(()));
+                    return (false, Err(Error::DisconnectedError));
                 }
                 (
                     false,
                     Ok(Some(CommandResponse::Status {
                         username: self.username.clone(),
-                        server_state: self.server.clone().unwrap(),
+                        server_state: self.server.clone().unwrap(), //guaranteed not to panic because if we are connected, server is guaranteed to be Some
                     })),
                 )
             }
             Command::ServerDisconnect => {
+                if !matches!(*self.phase_receiver().borrow(), StatePhase::Connected) {
+                    return (false, Err(Error::DisconnectedError));
+                }
+
                 self.session_id = None;
                 self.username = None;
                 self.server = None;
diff --git a/mumlib/src/error.rs b/mumlib/src/error.rs
new file mode 100644
index 0000000..cb88aa7
--- /dev/null
+++ b/mumlib/src/error.rs
@@ -0,0 +1,24 @@
+use serde::{Serialize, Deserialize};
+use std::fmt::Display;
+use serde::export::Formatter;
+
+pub type Result<T> = std::result::Result<T, Error>;
+
+#[derive(Debug, Serialize, Deserialize)]
+pub enum Error {
+    DisconnectedError,
+    AlreadyConnectedError,
+    InvalidChannelIdError(u32),
+    InvalidServerAddrError(String, u16),
+}
+
+impl Display for Error {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        match self {
+            Error::DisconnectedError => write!(f, "Not connected to a server"),
+            Error::AlreadyConnectedError => write!(f, "Already connected to a server"),
+            Error::InvalidChannelIdError(id) => write!(f, "Invalid channel id: {}", id),
+            Error::InvalidServerAddrError(addr, port) => write!(f, "Invalid server address: {}:{}", addr, port),
+        }
+    }
+}
\ No newline at end of file
diff --git a/mumlib/src/lib.rs b/mumlib/src/lib.rs
index ebf2019..b77653c 100644
--- a/mumlib/src/lib.rs
+++ b/mumlib/src/lib.rs
@@ -1,5 +1,6 @@
 pub mod command;
 pub mod state;
+pub mod error;
 
 use colored::*;
 use log::*;