Better errors (#14)

* make reading use thiserror

* finish implementing all the error things

* clippy warnings related to ok_or

* fix some errors in other places

* thiserror in more places

* don't use closures in a couple places

* errors in writing packet

* rip backtraces

* change some BufReadError::Custom to UnexpectedEnumVariant

* Errors say what packet is bad

* error on leftover data and fix

it wasn't reading the properties for gameprofile

4 files changed, 97 insertions, 45 deletions

diff --git a/azalea-client/src/account.rs b/azalea-client/src/account.rs
index 32a46406..c554908f 100644
--- a/azalea-client/src/account.rs
+++ b/azalea-client/src/account.rs
@@ -1,6 +1,6 @@
 //! Connect to Minecraft servers.
 
-use crate::{Client, Event};
+use crate::{client::JoinError, Client, Event};
 use azalea_protocol::ServerAddress;
 use tokio::sync::mpsc::UnboundedReceiver;
 
@@ -19,7 +19,7 @@ impl Account {
     pub async fn join(
         &self,
         address: &ServerAddress,
-    ) -> Result<(Client, UnboundedReceiver<Event>), String> {
+    ) -> Result<(Client, UnboundedReceiver<Event>), JoinError> {
         Client::join(self, address).await
     }
 }
diff --git a/azalea-client/src/client.rs b/azalea-client/src/client.rs
index 28154bce..38e36b63 100644
--- a/azalea-client/src/client.rs
+++ b/azalea-client/src/client.rs
@@ -3,7 +3,7 @@ use azalea_auth::game_profile::GameProfile;
 use azalea_core::{ChunkPos, EntityPos, PositionDelta, PositionDeltaTrait, ResourceLocation};
 use azalea_entity::Entity;
 use azalea_protocol::{
-    connect::Connection,
+    connect::{Connection, ConnectionError},
     packets::{
         game::{
             clientbound_player_chat_packet::ClientboundPlayerChatPacket,
@@ -22,13 +22,16 @@ use azalea_protocol::{
         },
         ConnectionProtocol, PROTOCOL_VERSION,
     },
+    read::ReadPacketError,
     resolver, ServerAddress,
 };
 use azalea_world::Dimension;
 use std::{
     fmt::Debug,
+    io,
     sync::{Arc, Mutex},
 };
+use thiserror::Error;
 use tokio::{
     sync::mpsc::{self, UnboundedReceiver, UnboundedSender},
     time::{self},
@@ -70,15 +73,34 @@ pub struct Client {
 /// Whether we should ignore errors when decoding packets.
 const IGNORE_ERRORS: bool = !cfg!(debug_assertions);
 
-#[derive(Debug)]
-struct HandleError(String);
+#[derive(Error, Debug)]
+pub enum JoinError {
+    #[error("{0}")]
+    Resolver(#[from] resolver::ResolverError),
+    #[error("{0}")]
+    Connection(#[from] ConnectionError),
+    #[error("{0}")]
+    ReadPacket(#[from] azalea_protocol::read::ReadPacketError),
+    #[error("{0}")]
+    Io(#[from] io::Error),
+}
+
+#[derive(Error, Debug)]
+pub enum HandleError {
+    #[error("{0}")]
+    Poison(String),
+    #[error("{0}")]
+    Io(#[from] io::Error),
+    #[error(transparent)]
+    Other(#[from] anyhow::Error),
+}
 
 impl Client {
     /// Connect to a Minecraft server with an account.
     pub async fn join(
         account: &Account,
         address: &ServerAddress,
-    ) -> Result<(Self, UnboundedReceiver<Event>), String> {
+    ) -> Result<(Self, UnboundedReceiver<Event>), JoinError> {
         let resolved_address = resolver::resolve_address(address).await?;
 
         let mut conn = Connection::new(&resolved_address).await?;
@@ -93,7 +115,7 @@ impl Client {
             }
             .get(),
         )
-        .await;
+        .await?;
         let mut conn = conn.login();
 
         // login
@@ -105,7 +127,7 @@ impl Client {
             }
             .get(),
         )
-        .await;
+        .await?;
 
         let (conn, game_profile) = loop {
             let packet_result = conn.read().await;
@@ -126,7 +148,7 @@ impl Client {
                             }
                             .get(),
                         )
-                        .await;
+                        .await?;
                         conn.set_encryption_key(e.secret_key);
                     }
                     ClientboundLoginPacket::ClientboundLoginCompressionPacket(p) => {
@@ -180,22 +202,23 @@ impl Client {
                 Ok(packet) => match Self::handle(&packet, &client, &tx).await {
                     Ok(_) => {}
                     Err(e) => {
-                        println!("Error handling packet: {:?}", e);
+                        println!("Error handling packet: {}", e);
                         if IGNORE_ERRORS {
                             continue;
                         } else {
-                            panic!("Error handling packet: {:?}", e);
+                            panic!("Error handling packet: {}", e);
                         }
                     }
                 },
                 Err(e) => {
                     if IGNORE_ERRORS {
-                        println!("Error: {:?}", e);
-                        if e == "length wider than 21-bit" {
-                            panic!();
+                        println!("{}", e);
+                        match e {
+                            ReadPacketError::FrameSplitter { .. } => panic!("Error: {:?}", e),
+                            _ => continue,
                         }
                     } else {
-                        panic!("Error: {:?}", e);
+                        panic!("{}", e);
                     }
                 }
             };
@@ -303,7 +326,7 @@ impl Client {
                         }
                         .get(),
                     )
-                    .await;
+                    .await?;
 
                 tx.send(Event::Login).unwrap();
             }
@@ -416,7 +439,7 @@ impl Client {
                 let mut conn_lock = client.conn.lock().await;
                 conn_lock
                     .write(ServerboundAcceptTeleportationPacket { id: p.id }.get())
-                    .await;
+                    .await?;
                 conn_lock
                     .write(
                         ServerboundMovePlayerPacketPosRot {
@@ -430,7 +453,7 @@ impl Client {
                         }
                         .get(),
                     )
-                    .await;
+                    .await?;
             }
             ClientboundGamePacket::ClientboundPlayerInfoPacket(p) => {
                 println!("Got player info packet {:?}", p);
@@ -514,14 +537,16 @@ impl Client {
                 let mut dimension_lock = client.dimension.lock()?;
                 let dimension = dimension_lock.as_mut().unwrap();
 
-                dimension.move_entity(
-                    p.id,
-                    EntityPos {
-                        x: p.x,
-                        y: p.y,
-                        z: p.z,
-                    },
-                )?;
+                dimension
+                    .move_entity(
+                        p.id,
+                        EntityPos {
+                            x: p.x,
+                            y: p.y,
+                            z: p.z,
+                        },
+                    )
+                    .map_err(|e| HandleError::Other(e.into()))?;
             }
             ClientboundGamePacket::ClientboundUpdateAdvancementsPacket(p) => {
                 println!("Got update advancements packet {:?}", p);
@@ -533,13 +558,17 @@ impl Client {
                 let mut dimension_lock = client.dimension.lock()?;
                 let dimension = dimension_lock.as_mut().unwrap();
 
-                dimension.move_entity_with_delta(p.entity_id, &p.delta)?;
+                dimension
+                    .move_entity_with_delta(p.entity_id, &p.delta)
+                    .map_err(|e| HandleError::Other(e.into()))?;
             }
             ClientboundGamePacket::ClientboundMoveEntityPosrotPacket(p) => {
                 let mut dimension_lock = client.dimension.lock()?;
                 let dimension = dimension_lock.as_mut().unwrap();
 
-                dimension.move_entity_with_delta(p.entity_id, &p.delta)?;
+                dimension
+                    .move_entity_with_delta(p.entity_id, &p.delta)
+                    .map_err(|e| HandleError::Other(e.into()))?;
             }
             ClientboundGamePacket::ClientboundMoveEntityRotPacket(p) => {
                 println!("Got move entity rot packet {:?}", p);
@@ -551,7 +580,7 @@ impl Client {
                     .lock()
                     .await
                     .write(ServerboundKeepAlivePacket { id: p.id }.get())
-                    .await;
+                    .await?;
             }
             ClientboundGamePacket::ClientboundRemoveEntitiesPacket(p) => {
                 println!("Got remove entities packet {:?}", p);
@@ -625,12 +654,6 @@ impl Client {
 
 impl<T> From<std::sync::PoisonError<T>> for HandleError {
     fn from(e: std::sync::PoisonError<T>) -> Self {
-        HandleError(e.to_string())
-    }
-}
-
-impl From<String> for HandleError {
-    fn from(e: String) -> Self {
-        HandleError(e)
+        HandleError::Poison(e.to_string())
     }
 }
diff --git a/azalea-client/src/movement.rs b/azalea-client/src/movement.rs
index 4f99984f..bc48e1b2 100644
--- a/azalea-client/src/movement.rs
+++ b/azalea-client/src/movement.rs
@@ -1,10 +1,20 @@
 use crate::Client;
 use azalea_core::EntityPos;
 use azalea_protocol::packets::game::serverbound_move_player_packet_pos_rot::ServerboundMovePlayerPacketPosRot;
+use azalea_world::MoveEntityError;
+use thiserror::Error;
+
+#[derive(Error, Debug)]
+pub enum MovePlayerError {
+    #[error("Player is not in world")]
+    PlayerNotInWorld,
+    #[error("{0}")]
+    Io(#[from] std::io::Error),
+}
 
 impl Client {
     /// Set the client's position to the given coordinates.
-    pub async fn move_to(&mut self, new_pos: EntityPos) -> Result<(), String> {
+    pub async fn move_to(&mut self, new_pos: EntityPos) -> Result<(), MovePlayerError> {
         {
             let mut dimension_lock = self.dimension.lock().unwrap();
             let dimension = dimension_lock.as_mut().unwrap();
@@ -14,10 +24,15 @@ impl Client {
             let player_id = if let Some(player_lock) = player_lock.entity(dimension) {
                 player_lock.id
             } else {
-                return Err("Player entity not found".to_string());
+                return Err(MovePlayerError::PlayerNotInWorld);
             };
 
-            dimension.move_entity(player_id, new_pos)?;
+            match dimension.move_entity(player_id, new_pos) {
+                Ok(_) => Ok(()),
+                Err(e) => match e {
+                    MoveEntityError::EntityDoesNotExist => Err(MovePlayerError::PlayerNotInWorld),
+                },
+            }?;
         }
 
         self.conn
@@ -34,7 +49,7 @@ impl Client {
                 }
                 .get(),
             )
-            .await;
+            .await?;
 
         Ok(())
     }
diff --git a/azalea-client/src/ping.rs b/azalea-client/src/ping.rs
index e4acfc34..303fae74 100755
--- a/azalea-client/src/ping.rs
+++ b/azalea-client/src/ping.rs
@@ -1,6 +1,6 @@
 ///! Ping Minecraft servers.
 use azalea_protocol::{
-    connect::Connection,
+    connect::{Connection, ConnectionError},
     packets::{
         handshake::client_intention_packet::ClientIntentionPacket,
         status::{
@@ -12,10 +12,24 @@ use azalea_protocol::{
     },
     resolver, ServerAddress,
 };
+use std::io;
+use thiserror::Error;
+
+#[derive(Error, Debug)]
+pub enum PingError {
+    #[error("{0}")]
+    Resolver(#[from] resolver::ResolverError),
+    #[error("{0}")]
+    Connection(#[from] ConnectionError),
+    #[error("{0}")]
+    ReadPacket(#[from] azalea_protocol::read::ReadPacketError),
+    #[error("{0}")]
+    WritePacket(#[from] io::Error),
+}
 
 pub async fn ping_server(
     address: &ServerAddress,
-) -> Result<ClientboundStatusResponsePacket, String> {
+) -> Result<ClientboundStatusResponsePacket, PingError> {
     let resolved_address = resolver::resolve_address(address).await?;
 
     let mut conn = Connection::new(&resolved_address).await?;
@@ -30,13 +44,13 @@ pub async fn ping_server(
         }
         .get(),
     )
-    .await;
+    .await?;
     let mut conn = conn.status();
 
     // send the empty status request packet
-    conn.write(ServerboundStatusRequestPacket {}.get()).await;
+    conn.write(ServerboundStatusRequestPacket {}.get()).await?;
 
-    let packet = conn.read().await.unwrap();
+    let packet = conn.read().await?;
 
     match packet {
         ClientboundStatusPacket::ClientboundStatusResponsePacket(p) => Ok(p),