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, 87 insertions, 31 deletions

diff --git a/azalea-protocol/src/read.rs b/azalea-protocol/src/read.rs
index 6a56ccfb..077e1297 100755
--- a/azalea-protocol/src/read.rs
+++ b/azalea-protocol/src/read.rs
@@ -1,5 +1,6 @@
 use crate::packets::ProtocolPacket;
-use azalea_buf::{read_varint_async, Readable};
+use azalea_buf::McBufVarReadable;
+use azalea_buf::{read_varint_async, BufReadError, Readable};
 use azalea_crypto::Aes128CfbDec;
 use flate2::read::ZlibDecoder;
 use std::{
@@ -8,32 +9,67 @@ use std::{
     pin::Pin,
     task::{Context, Poll},
 };
+use thiserror::Error;
 use tokio::io::{AsyncRead, AsyncReadExt};
 
-async fn frame_splitter<R: ?Sized>(mut stream: &mut R) -> Result<Vec<u8>, String>
+#[derive(Error, Debug)]
+pub enum ReadPacketError {
+    #[error("Error reading packet {packet_name} ({packet_id}): {source}")]
+    Parse {
+        packet_id: u32,
+        packet_name: String,
+        source: BufReadError,
+    },
+    #[error("Unknown packet id {id} in state {state_name}")]
+    UnknownPacketId { state_name: String, id: u32 },
+    #[error("Couldn't read packet id")]
+    ReadPacketId { source: BufReadError },
+    #[error("Couldn't decompress packet")]
+    Decompress {
+        #[from]
+        source: DecompressionError,
+    },
+    #[error("Frame splitter error")]
+    FrameSplitter {
+        #[from]
+        source: FrameSplitterError,
+    },
+    #[error("Leftover data after reading packet {packet_name}: {data:?}")]
+    LeftoverData { data: Vec<u8>, packet_name: String },
+}
+
+#[derive(Error, Debug)]
+pub enum FrameSplitterError {
+    #[error("Couldn't read VarInt length for packet. The previous packet may have been corrupted")]
+    LengthRead {
+        #[from]
+        source: BufReadError,
+    },
+    #[error("Io error")]
+    Io {
+        #[from]
+        source: std::io::Error,
+    },
+}
+
+async fn frame_splitter<R: ?Sized>(mut stream: &mut R) -> Result<Vec<u8>, FrameSplitterError>
 where
     R: AsyncRead + std::marker::Unpin + std::marker::Send,
 {
     // Packet Length
-    let length_result = read_varint_async(&mut stream).await;
-    match length_result {
-        Ok(length) => {
-            let mut buf = vec![0; length as usize];
+    let length = read_varint_async(&mut stream).await?;
 
-            stream
-                .read_exact(&mut buf)
-                .await
-                .map_err(|e| e.to_string())?;
+    let mut buf = vec![0; length as usize];
+    stream.read_exact(&mut buf).await?;
 
-            Ok(buf)
-        }
-        Err(_) => Err("length wider than 21-bit".to_string()),
-    }
+    Ok(buf)
 }
 
-fn packet_decoder<P: ProtocolPacket>(stream: &mut impl Read) -> Result<P, String> {
+fn packet_decoder<P: ProtocolPacket>(stream: &mut impl Read) -> Result<P, ReadPacketError> {
     // Packet ID
-    let packet_id = stream.read_varint()?;
+    let packet_id = stream
+        .read_varint()
+        .map_err(|e| ReadPacketError::ReadPacketId { source: e })?;
     P::read(packet_id.try_into().unwrap(), stream)
 }
 
@@ -42,39 +78,57 @@ static VALIDATE_DECOMPRESSED: bool = true;
 
 pub static MAXIMUM_UNCOMPRESSED_LENGTH: u32 = 2097152;
 
+#[derive(Error, Debug)]
+pub enum DecompressionError {
+    #[error("Couldn't read VarInt length for data")]
+    LengthReadError {
+        #[from]
+        source: BufReadError,
+    },
+    #[error("Io error")]
+    Io {
+        #[from]
+        source: std::io::Error,
+    },
+    #[error("Badly compressed packet - size of {size} is below server threshold of {threshold}")]
+    BelowCompressionThreshold { size: u32, threshold: u32 },
+    #[error(
+        "Badly compressed packet - size of {size} is larger than protocol maximum of {maximum}"
+    )]
+    AboveCompressionThreshold { size: u32, maximum: u32 },
+}
+
 fn compression_decoder(
     stream: &mut impl Read,
     compression_threshold: u32,
-) -> Result<Vec<u8>, String> {
+) -> Result<Vec<u8>, DecompressionError> {
     // Data Length
-    let n: u32 = stream.read_varint()?.try_into().unwrap();
+    let n = u32::var_read_from(stream)?;
     if n == 0 {
         // no data size, no compression
         let mut buf = vec![];
-        stream.read_to_end(&mut buf).map_err(|e| e.to_string())?;
+        stream.read_to_end(&mut buf)?;
         return Ok(buf);
     }
 
     if VALIDATE_DECOMPRESSED {
         if n < compression_threshold {
-            return Err(format!(
-                "Badly compressed packet - size of {} is below server threshold of {}",
-                n, compression_threshold
-            ));
+            return Err(DecompressionError::BelowCompressionThreshold {
+                size: n,
+                threshold: compression_threshold,
+            });
         }
         if n > MAXIMUM_UNCOMPRESSED_LENGTH {
-            return Err(format!(
-                "Badly compressed packet - size of {} is larger than protocol maximum of {}",
-                n, MAXIMUM_UNCOMPRESSED_LENGTH
-            ));
+            return Err(DecompressionError::AboveCompressionThreshold {
+                size: n,
+                maximum: MAXIMUM_UNCOMPRESSED_LENGTH,
+            });
         }
     }
 
     let mut decoded_buf = vec![];
     let mut decoder = ZlibDecoder::new(stream);
-    decoder
-        .read_to_end(&mut decoded_buf)
-        .map_err(|e| e.to_string())?;
+    decoder.read_to_end(&mut decoded_buf)?;
 
     Ok(decoded_buf)
 }
@@ -121,7 +175,7 @@ pub async fn read_packet<'a, P: ProtocolPacket, R>(
     stream: &'a mut R,
     compression_threshold: Option<u32>,
     cipher: &mut Option<Aes128CfbDec>,
-) -> Result<P, String>
+) -> Result<P, ReadPacketError>
 where
     R: AsyncRead + std::marker::Unpin + std::marker::Send + std::marker::Sync,
 {
@@ -149,5 +203,7 @@ where
     let packet = packet_decoder(&mut buf.as_slice())?;
     // println!("decoded packet ({}ms)", start_time.elapsed().as_millis());
 
+    if !buf.is_empty() {}
+
     Ok(packet)
 }