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

1 files changed, 57 insertions, 34 deletions

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())
     }
 }