From 085f9bf579bf363e175458bfa094432ba5e8fbb1 Mon Sep 17 00:00:00 2001 From: sahil Date: Mon, 27 Nov 2023 10:28:15 +0000 Subject: [PATCH] Refactored and more unwraps - which caused some channels to be dropped after panics, but not the connection on the client side --- chat_messages/src/lib.rs | 4 +- client/src/main.rs | 11 ++++-- server/src/main.rs | 41 ++++++++++---------- server/src/server_data.rs | 80 +++++++++++++++++---------------------- 4 files changed, 64 insertions(+), 72 deletions(-) diff --git a/chat_messages/src/lib.rs b/chat_messages/src/lib.rs index 518c766..30c571f 100644 --- a/chat_messages/src/lib.rs +++ b/chat_messages/src/lib.rs @@ -1,5 +1,3 @@ - - use serde::Serialize; #[derive(serde::Deserialize, Debug, Serialize)] @@ -10,7 +8,7 @@ pub struct Message { impl Message { pub fn new(message: String, name: String) -> Self { - Self { message , name } + Self { message, name } } pub fn name(&self) -> &String { diff --git a/client/src/main.rs b/client/src/main.rs index 0805032..95fcd50 100644 --- a/client/src/main.rs +++ b/client/src/main.rs @@ -4,11 +4,10 @@ use std::net::TcpStream; use std::sync::Arc; use chat_messages::Message; -use termion::color; use termion::clear; +use termion::color; fn main() { - println!("{}", clear::All); let address = "127.0.0.1:6969"; @@ -22,7 +21,13 @@ fn main() { println!("{from_utf8}"); let x = serde_json::from_str::(&from_utf8).unwrap(); // let msg = if x.name().is_some() { - let msg = format!("{}{}{}: {}", color::Red.fg_str(), &x.name().clone(), color::Reset.fg_str(), x.message()); + let msg = format!( + "{}{}{}: {}", + color::Red.fg_str(), + &x.name().clone(), + color::Reset.fg_str(), + x.message() + ); // } else { // format!("{}{}{}: {}", color::Red.fg_str(), stream.local_addr().unwrap().to_string(), color::Reset.fg_str(), x.message()) // }; diff --git a/server/src/main.rs b/server/src/main.rs index f943ea7..196dd88 100644 --- a/server/src/main.rs +++ b/server/src/main.rs @@ -13,7 +13,10 @@ fn main() -> anyhow::Result<()> { env_logger::init(); // Starting TcpListener - let listener = std::net::TcpListener::bind(address)?; + let listener = std::net::TcpListener::bind(address).map_err(|err| { + error!("Could not start the server: {err}"); + err + })?; //creates a new server and spawns it in a new thread let mut server = Server::new(); @@ -24,26 +27,24 @@ fn main() -> anyhow::Result<()> { for stream in listener.incoming() { match stream { Ok(stream) => { - debug!( - "Client Connected from {ip}", - ip = stream.peer_addr().unwrap() - ); - - // at this stage the Tcp connection is established between the client and the - // server - let connection = Arc::new(stream); - let addr = connection.as_ref().peer_addr().map_err(|err| { - error!("Could not read clients addr {err}"); - err - })?; - // Send the connection to the server with the channel from above - // the receiver should be the server object which was listing to the events - // in case of errors log it and move on - let _ = tx - .send(ServerEvent::ClientJoinRequest(connection, tx.clone(), addr)) - .map_err(|err| error!("Could not send message to server {err}")); + // should be fine to ignore the error for logging + let _ = stream + .peer_addr() + .map(|ip| { + debug!("Client Connected from {ip}",); + // at this stage the Tcp connection is established between the client and the + // server + let connection = Arc::new(stream); + // Send the connection to the server with the channel from above + // the receiver should be the server object which was listing to the events + // in case of errors log it and move on + let _ = tx + .send(ServerEvent::ClientJoinRequest(connection, tx.clone(), ip)) + .map_err(|err| error!("Could not send message to server: {err}")); + }) + .map_err(|err| error!("Could not read clients ip: {err}")); } - Err(err) => error!("Error connecting to client {err}"), + Err(err) => error!("Error connecting to client: {err}"), } } diff --git a/server/src/server_data.rs b/server/src/server_data.rs index 21a1fcc..56af8bf 100644 --- a/server/src/server_data.rs +++ b/server/src/server_data.rs @@ -2,7 +2,6 @@ use std::collections::HashMap; use std::io::Write; use std::net::SocketAddr; -use std::str::FromStr; use std::{ io::Read, net::TcpStream, @@ -15,8 +14,7 @@ use std::{ use chat_messages::Message; -use log::{debug, error}; -use serde::de::value; +use log::{debug, error, warn}; /// Buffer size of a single message const MESSAGE_SIZE: usize = 1000000; @@ -72,20 +70,16 @@ impl Server { Commands::CloseConnection => { self.disconnect_client(addr); continue; - }, + } Commands::RawMessage(val) => { for (key, value) in self.clients.iter_mut() { if key == &addr { continue; } - let _ = writeln!( - value.connection.deref(), - "{msg}", - msg = val - ) - .map_err(|err| { - error!("Could not write to client: {err}"); - }); + let _ = writeln!(value.connection.deref(), "{msg}", msg = val) + .map_err(|err| { + error!("Could not write to client: {err}"); + }); } continue; } @@ -100,18 +94,21 @@ impl Server { if key == &addr { continue; } - let msg = serde_json::to_string::(val).unwrap().trim_nulls(); - let msg = msg.trim(); - let _ = writeln!( - value.connection.deref(), - "{msg}" - ) - .map_err(|err| { - error!("Could not write to client: {err}"); - }); + let _ = serde_json::to_string(val) + .map(|val| { + let val = val.trim_nulls(); + let val = val.trim(); + let _ = writeln!(value.connection.deref(), "{val}") + .map_err(|err| { + error!("Could not write to client: {err}"); + }); + }) + .map_err(|err| { + warn!("Client did not send correct format: {err}") + }); } }) - .map_err(|_err| error!("Client did not send correct message format")); + .map_err(|err| warn!("Client did not send correct format: {err}")); } ServerEvent::ClientDisconnect(ip) => self.disconnect_client(ip), } @@ -196,34 +193,25 @@ enum Commands { impl From for Commands { fn from(value: String) -> Self { - - return match value.as_str().split_whitespace().next().unwrap() { - ":ext" => Self::CloseConnection, - ":r" => Self::RawMessage(value), - _ => Self::NotCommand, + let first = value.as_str().split_whitespace().next(); + return match first { + Some(val) => { + return match val { + ":ext" => Self::CloseConnection, + ":r" => Self::RawMessage(value), + _ => { + if val.starts_with(":") { + return Self::InvalidCommand; + } + return Self::NotCommand; + } + }; + } + None => Self::NotCommand, }; } } -impl FromStr for Commands { - type Err = Commands; - - fn from_str(s: &str) -> Result { - if !s.starts_with(':') { - return Ok(Self::NotCommand); - } - - let mut splits = s.split_whitespace(); - error!("{}", splits.next().unwrap()); - - match splits.next().unwrap() { - ":ext" => Ok(Self::CloseConnection), - ":r" => Ok(Self::RawMessage(s.to_string())), - _ => Err(Self::InvalidCommand), - } - } -} - trait TcpStreamString { fn trim_nulls(&self) -> Self; }