improve(login): more precise error handling during login

client/src/error.rs

@@ -8,8 +8,8 @@ pub enum Error {
     ServerTimeout,
     ServerShutdown,
     TooManyPlayers,
-    InvalidAuth,
     AlreadyLoggedIn,
+    AuthErr(String),
     AuthClientError(AuthClientError),
     AuthServerNotTrusted,
     //TODO: InvalidAlias,

client/src/lib.rs

@@ -18,7 +18,7 @@ use common::{
     event::{EventBus, SfxEvent, SfxEventItem},
     msg::{
         validate_chat_msg, ChatMsgValidationError, ClientMsg, ClientState, PlayerListUpdate,
-        RequestStateError, ServerError, ServerInfo, ServerMsg, MAX_BYTES_CHAT_MSG,
+        RegisterError, RequestStateError, ServerInfo, ServerMsg, MAX_BYTES_CHAT_MSG,
     },
     net::PostBox,
     state::State,
@@ -87,13 +87,13 @@ impl Client {
         let mut postbox = PostBox::to(addr)?;
 
         // Wait for initial sync
-        let (state, entity, server_info, world_map) = match postbox.next_message() {
+        let (state, entity, server_info, world_map) = match postbox.next_message()? {
-            Some(ServerMsg::InitialSync {
+            ServerMsg::InitialSync {
                 entity_package,
                 server_info,
                 time_of_day,
                 world_map: (map_size, world_map),
-            }) => {
+            } => {
                 // TODO: Display that versions don't match in Voxygen
                 if server_info.git_hash != common::util::GIT_HASH.to_string() {
                     log::warn!(
@@ -132,9 +132,7 @@ impl Client {
 
                 (state, entity, server_info, (world_map, map_size))
             },
-            Some(ServerMsg::Error(ServerError::TooManyPlayers)) => {
+            ServerMsg::TooManyPlayers => return Err(Error::TooManyPlayers),
-                return Err(Error::TooManyPlayers);
-            },
             _ => return Err(Error::ServerWentMad),
         };
 
@@ -203,11 +201,15 @@ impl Client {
         self.client_state = ClientState::Pending;
 
         loop {
-            match self.postbox.next_message() {
+            match self.postbox.next_message()? {
-                Some(ServerMsg::StateAnswer(Err((RequestStateError::Denied, _)))) => {
+                ServerMsg::StateAnswer(Err((RequestStateError::RegisterDenied(err), state))) => {
-                    break Err(Error::InvalidAuth);
+                    self.client_state = state;
+                    break Err(match err {
+                        RegisterError::AlreadyLoggedIn => Error::AlreadyLoggedIn,
+                        RegisterError::AuthError(err) => Error::AuthErr(err),
+                    });
                 },
-                Some(ServerMsg::StateAnswer(Ok(ClientState::Registered))) => break Ok(()),
+                ServerMsg::StateAnswer(Ok(ClientState::Registered)) => break Ok(()),
                 _ => {},
             }
         }
@@ -572,12 +574,8 @@ impl Client {
         if new_msgs.len() > 0 {
             for msg in new_msgs {
                 match msg {
-                    ServerMsg::Error(e) => match e {
+                    ServerMsg::TooManyPlayers => {
-                        ServerError::TooManyPlayers => return Err(Error::ServerWentMad),
+                        return Err(Error::ServerWentMad);
-                        ServerError::InvalidAuth => return Err(Error::InvalidAuth),
-                        ServerError::AlreadyLoggedIn => return Err(Error::AlreadyLoggedIn),
-                        ServerError::AuthError(_) => unreachable!(),
-                        //TODO: ServerError::InvalidAlias => return Err(Error::InvalidAlias),
                     },
                     ServerMsg::Shutdown => return Err(Error::ServerShutdown),
                     ServerMsg::InitialSync { .. } => return Err(Error::ServerWentMad),
@@ -720,10 +718,6 @@ impl Client {
                         self.client_state = state;
                     },
                     ServerMsg::StateAnswer(Err((error, state))) => {
-                        if error == RequestStateError::Denied {
-                            warn!("Connection denied!");
-                            return Err(Error::InvalidAuth);
-                        }
                         warn!(
                             "StateAnswer: {:?}. Server thinks client is in state {:?}.",
                             error, state

common/src/msg/mod.rs

@@ -6,7 +6,7 @@ pub mod server;
 pub use self::{
     client::ClientMsg,
     ecs_packet::EcsCompPacket,
-    server::{PlayerListUpdate, RequestStateError, ServerError, ServerInfo, ServerMsg},
+    server::{PlayerListUpdate, RegisterError, RequestStateError, ServerInfo, ServerMsg},
 };
 
 #[derive(Debug, Clone, Copy, PartialEq, Serialize, Deserialize)]

common/src/msg/server.rs

@@ -8,14 +8,6 @@ use authc::AuthClientError;
 use hashbrown::HashMap;
 use vek::*;
 
-#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
-pub enum RequestStateError {
-    Denied,
-    Already,
-    Impossible,
-    WrongMessage,
-}
-
 #[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct ServerInfo {
     pub name: String,
@@ -79,21 +71,28 @@ pub enum ServerMsg {
         chunk: Result<Box<TerrainChunk>, ()>,
     },
     TerrainBlockUpdates(HashMap<Vec3<i32>, Block>),
-    Error(ServerError),
     Disconnect,
     Shutdown,
+    TooManyPlayers,
 }
 
-#[derive(Debug, Clone, Serialize, Deserialize)]
+#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
-pub enum ServerError {
+pub enum RequestStateError {
-    TooManyPlayers,
+    RegisterDenied(RegisterError),
-    InvalidAuth,
+    Denied,
+    Already,
+    Impossible,
+    WrongMessage,
+}
+
+#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
+pub enum RegisterError {
     AlreadyLoggedIn,
     AuthError(String),
     //TODO: InvalidAlias,
 }
 
-impl From<AuthClientError> for ServerError {
+impl From<AuthClientError> for RegisterError {
     fn from(err: AuthClientError) -> Self { Self::AuthError(err.to_string()) }
 }
 

common/src/net/post2.rs

@@ -119,16 +119,16 @@ impl<S: PostMsg, R: PostMsg> PostBox<S, R> {
 
     pub fn send_message(&mut self, msg: S) { let _ = self.send_tx.send(msg); }
 
-    pub fn next_message(&mut self) -> Option<R> {
+    pub fn next_message(&mut self) -> Result<R, Error> {
-        if self.error.is_some() {
+        if let Some(e) = self.error.clone() {
-            return None;
+            return Err(e);
         }
 
-        match self.recv_rx.recv().ok()? {
+        match self.recv_rx.recv().map_err(|_| Error::ChannelFailure)? {
-            Ok(msg) => Some(msg),
+            Ok(msg) => Ok(msg),
             Err(e) => {
-                self.error = Some(e);
+                self.error = Some(e.clone());
-                None
+                Err(e)
             },
         }
     }

server/src/auth_provider.rs

@@ -1,5 +1,5 @@
 use authc::{AuthClient, AuthToken, Uuid};
-use common::msg::ServerError;
+use common::msg::RegisterError;
 use hashbrown::HashMap;
 use std::str::FromStr;
 
@@ -42,28 +42,26 @@ impl AuthProvider {
         }
     }
 
-    pub fn query(&mut self, username_or_token: String) -> Result<bool, ServerError> {
+    pub fn query(&mut self, username_or_token: String) -> Result<(), RegisterError> {
         // Based on whether auth server is provided or not we expect an username or
         // token
         match &self.auth_server {
             // Token from auth server expected
             Some(srv) => {
                 log::info!("Validating '{}' token.", &username_or_token);
-                if let Ok(token) = AuthToken::from_str(&username_or_token) {
+                // Parse token
-                    match srv.validate(token) {
+                let token = AuthToken::from_str(&username_or_token)
-                        Ok(uuid) => {
+                    .map_err(|e| RegisterError::AuthError(e.to_string()))?;
+                // Validate token
+                let uuid = srv.validate(token)?;
+                // Check if already logged in
                 if self.accounts.contains_key(&uuid) {
-                                return Err(ServerError::AlreadyLoggedIn);
+                    return Err(RegisterError::AlreadyLoggedIn);
                 }
+                // Log in
                 let username = srv.uuid_to_username(uuid.clone())?;
                 self.accounts.insert(uuid, username);
-                            Ok(true)
+                Ok(())
-                        },
-                        Err(e) => Err(ServerError::from(e)),
-                    }
-                } else {
-                    Ok(false)
-                }
             },
             // Username is expected
             None => {
@@ -73,9 +71,9 @@ impl AuthProvider {
                 if !self.accounts.contains_key(&uuid) {
                     log::info!("New User '{}'", username);
                     self.accounts.insert(uuid, username);
-                    Ok(true)
+                    Ok(())
                 } else {
-                    Err(ServerError::AlreadyLoggedIn)
+                    Err(RegisterError::AlreadyLoggedIn)
                 }
             },
         }

server/src/lib.rs

@@ -27,7 +27,7 @@ use common::{
     assets, comp,
     effect::Effect,
     event::{EventBus, ServerEvent},
-    msg::{ClientMsg, ClientState, ServerError, ServerInfo, ServerMsg},
+    msg::{ClientMsg, ClientState, ServerInfo, ServerMsg},
     net::PostOffice,
     state::{State, TimeOfDay},
     sync::{Uid, WorldSyncExt},
@@ -505,7 +505,7 @@ impl Server {
                 <= self.state.ecs().read_storage::<Client>().join().count()
             {
                 // Note: in this case the client is dropped
-                client.notify(ServerMsg::Error(ServerError::TooManyPlayers));
+                client.notify(ServerMsg::TooManyPlayers);
             } else {
                 let entity = self
                     .state

server/src/sys/message.rs

@@ -127,10 +127,12 @@ impl<'a> System<'a> for Sys {
                         player,
                         token_or_username,
                     } if player.is_valid() => {
-                        if !accounts.query(token_or_username.clone()).unwrap_or(false) {
+                        match accounts.query(token_or_username.clone()) {
-                            // TO-DO: Set a less generic error here.
+                            Err(err) => {
-                            client.error_state(RequestStateError::Denied);
+                                client.error_state(RequestStateError::RegisterDenied(err));
                                 break;
+                            },
+                            Ok(()) => {},
                         }
                         match client.client_state {
                             ClientState::Connected => {

voxygen/src/menu/main/mod.rs

@@ -81,7 +81,7 @@ impl PlayState for MainMenuState {
                                 "Server not found".into()
                             },
                             InitError::ClientError(err) => match err {
-                                client::Error::InvalidAuth => "Invalid credentials".into(),
+                                client::Error::AuthErr(e) => format!("Auth error on server: {}", e),
                                 client::Error::TooManyPlayers => "Server is full".into(),
                                 client::Error::AuthServerNotTrusted => {
                                     "Auth server not trusted".into()