From 5a9fca0ca9cdb46f4b866781f219756c89e2293a Mon Sep 17 00:00:00 2001
From: mat <27899617+mat-1@users.noreply.github.com>
Date: Sat, 6 Aug 2022 07:22:19 +0000
Subject: 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
---
 azalea-buf/src/definitions.rs       |   4 +-
 azalea-buf/src/lib.rs               |   2 +-
 azalea-buf/src/read.rs              | 196 +++++++++++++++++++-----------------
 azalea-buf/src/serializable_uuid.rs |   4 +-
 4 files changed, 107 insertions(+), 99 deletions(-)

(limited to 'azalea-buf/src')

diff --git a/azalea-buf/src/definitions.rs b/azalea-buf/src/definitions.rs
index f3452bea..cfe1bd8a 100644
--- a/azalea-buf/src/definitions.rs
+++ b/azalea-buf/src/definitions.rs
@@ -1,4 +1,4 @@
-use crate::{McBufReadable, McBufWritable};
+use crate::{read::BufReadError, McBufReadable, McBufWritable};
 use std::{
     io::{Read, Write},
     ops::Deref,
@@ -42,7 +42,7 @@ impl BitSet {
 }
 
 impl McBufReadable for BitSet {
-    fn read_from(buf: &mut impl Read) -> Result<Self, String> {
+    fn read_from(buf: &mut impl Read) -> Result<Self, BufReadError> {
         Ok(Self {
             data: Vec::<u64>::read_from(buf)?,
         })
diff --git a/azalea-buf/src/lib.rs b/azalea-buf/src/lib.rs
index 5bde833c..7860325e 100644
--- a/azalea-buf/src/lib.rs
+++ b/azalea-buf/src/lib.rs
@@ -9,7 +9,7 @@ mod write;
 
 pub use buf_macros::*;
 pub use definitions::*;
-pub use read::{read_varint_async, McBufReadable, McBufVarReadable, Readable};
+pub use read::{read_varint_async, BufReadError, McBufReadable, McBufVarReadable, Readable};
 pub use serializable_uuid::*;
 pub use write::{McBufVarWritable, McBufWritable, Writable};
 
diff --git a/azalea-buf/src/read.rs b/azalea-buf/src/read.rs
index 684404bc..8518d637 100644
--- a/azalea-buf/src/read.rs
+++ b/azalea-buf/src/read.rs
@@ -1,34 +1,62 @@
 use super::{UnsizedByteArray, MAX_STRING_LENGTH};
 use byteorder::{ReadBytesExt, BE};
 use std::{collections::HashMap, hash::Hash, io::Read};
+use thiserror::Error;
 use tokio::io::{AsyncRead, AsyncReadExt};
 
+#[derive(Error, Debug)]
+pub enum BufReadError {
+    #[error("Invalid VarInt")]
+    InvalidVarInt,
+    #[error("Invalid VarLong")]
+    InvalidVarLong,
+    #[error("Error reading bytes")]
+    CouldNotReadBytes,
+    #[error("The received encoded string buffer length is less than zero! Weird string!")]
+    StringLengthLessThanZero,
+    #[error("The received encoded string buffer length is longer than maximum allowed ({length} > {max_length})")]
+    StringLengthTooLong { length: i32, max_length: u32 },
+    #[error("{0}")]
+    Io(#[from] std::io::Error),
+    #[error("Boolean value is not 0 or 1")]
+    InvalidBoolean,
+    #[error("Invalid UTF-8")]
+    InvalidUtf8,
+    #[error("Unexpected enum variant {id}")]
+    UnexpectedEnumVariant { id: i32 },
+    #[error("{0}")]
+    Custom(String),
+    #[cfg(feature = "serde_json")]
+    #[error("{0}")]
+    Deserialization(#[from] serde_json::Error),
+}
+
 // TODO: get rid of Readable and use McBufReadable everywhere
 
 pub trait Readable {
-    fn read_int_id_list(&mut self) -> Result<Vec<i32>, String>;
-    fn read_varint(&mut self) -> Result<i32, String>;
+    fn read_int_id_list(&mut self) -> Result<Vec<i32>, BufReadError>;
+    fn read_varint(&mut self) -> Result<i32, BufReadError>;
     fn get_varint_size(&mut self, value: i32) -> u8;
     fn get_varlong_size(&mut self, value: i32) -> u8;
-    fn read_byte_array(&mut self) -> Result<Vec<u8>, String>;
-    fn read_bytes_with_len(&mut self, n: usize) -> Result<Vec<u8>, String>;
-    fn read_bytes(&mut self) -> Result<Vec<u8>, String>;
-    fn read_utf(&mut self) -> Result<String, String>;
-    fn read_utf_with_len(&mut self, max_length: u32) -> Result<String, String>;
-    fn read_byte(&mut self) -> Result<u8, String>;
-    fn read_int(&mut self) -> Result<i32, String>;
-    fn read_boolean(&mut self) -> Result<bool, String>;
-    fn read_long(&mut self) -> Result<i64, String>;
-    fn read_short(&mut self) -> Result<i16, String>;
-    fn read_float(&mut self) -> Result<f32, String>;
-    fn read_double(&mut self) -> Result<f64, String>;
+    fn read_byte_array(&mut self) -> Result<Vec<u8>, BufReadError>;
+    fn read_bytes_with_len(&mut self, n: usize) -> Result<Vec<u8>, BufReadError>;
+    fn read_bytes(&mut self) -> Result<Vec<u8>, BufReadError>;
+    fn read_utf(&mut self) -> Result<String, BufReadError>;
+    fn read_utf_with_len(&mut self, max_length: u32) -> Result<String, BufReadError>;
+    fn read_byte(&mut self) -> Result<u8, BufReadError>;
+    fn read_int(&mut self) -> Result<i32, BufReadError>;
+    fn read_boolean(&mut self) -> Result<bool, BufReadError>;
+    fn read_long(&mut self) -> Result<i64, BufReadError>;
+    fn read_short(&mut self) -> Result<i16, BufReadError>;
+    fn read_float(&mut self) -> Result<f32, BufReadError>;
+    fn read_double(&mut self) -> Result<f64, BufReadError>;
 }
 
 impl<R> Readable for R
 where
     R: Read,
 {
-    fn read_int_id_list(&mut self) -> Result<Vec<i32>, String> {
+    fn read_int_id_list(&mut self) -> Result<Vec<i32>, BufReadError> {
         let len = self.read_varint()?;
         let mut list = Vec::with_capacity(len as usize);
         for _ in 0..len {
@@ -39,12 +67,12 @@ where
 
     // fast varints modified from https://github.com/luojia65/mc-varint/blob/master/src/lib.rs#L67
     /// Read a single varint from the reader and return the value, along with the number of bytes read
-    fn read_varint(&mut self) -> Result<i32, String> {
+    fn read_varint(&mut self) -> Result<i32, BufReadError> {
         let mut buffer = [0];
         let mut ans = 0;
         for i in 0..5 {
             self.read_exact(&mut buffer)
-                .map_err(|_| "Invalid VarInt".to_string())?;
+                .map_err(|_| BufReadError::InvalidVarInt)?;
             ans |= ((buffer[0] & 0b0111_1111) as i32) << (7 * i);
             if buffer[0] & 0b1000_0000 == 0 {
                 return Ok(ans);
@@ -73,122 +101,102 @@ where
         10
     }
 
-    fn read_byte_array(&mut self) -> Result<Vec<u8>, String> {
+    fn read_byte_array(&mut self) -> Result<Vec<u8>, BufReadError> {
         let length = self.read_varint()? as usize;
         self.read_bytes_with_len(length)
     }
 
-    fn read_bytes_with_len(&mut self, n: usize) -> Result<Vec<u8>, String> {
+    fn read_bytes_with_len(&mut self, n: usize) -> Result<Vec<u8>, BufReadError> {
         let mut buffer = vec![0; n];
         self.read_exact(&mut buffer)
-            .map_err(|_| "Error reading bytes".to_string())?;
+            .map_err(|_| BufReadError::CouldNotReadBytes)?;
         Ok(buffer)
     }
 
-    fn read_bytes(&mut self) -> Result<Vec<u8>, String> {
+    fn read_bytes(&mut self) -> Result<Vec<u8>, BufReadError> {
         // read to end of the buffer
         let mut bytes = vec![];
         self.read_to_end(&mut bytes)
-            .map_err(|_| "Error reading bytes".to_string())?;
+            .map_err(|_| BufReadError::CouldNotReadBytes)?;
         Ok(bytes)
     }
 
-    fn read_utf(&mut self) -> Result<String, String> {
+    fn read_utf(&mut self) -> Result<String, BufReadError> {
         self.read_utf_with_len(MAX_STRING_LENGTH.into())
     }
 
-    fn read_utf_with_len(&mut self, max_length: u32) -> Result<String, String> {
+    fn read_utf_with_len(&mut self, max_length: u32) -> Result<String, BufReadError> {
         let length = self.read_varint()?;
         // i don't know why it's multiplied by 4 but it's like that in mojang's code so
         if length < 0 {
-            return Err(
-                "The received encoded string buffer length is less than zero! Weird string!"
-                    .to_string(),
-            );
+            return Err(BufReadError::StringLengthLessThanZero);
         }
         if length as u32 > max_length * 4 {
-            return Err(format!(
-                "The received encoded string buffer length is longer than maximum allowed ({} > {})",
+            return Err(BufReadError::StringLengthTooLong {
                 length,
-                max_length * 4
-            ));
+                max_length: max_length * 4,
+            });
         }
 
         // this is probably quite inefficient, idk how to do it better
         let mut string = String::new();
         let mut buffer = vec![0; length as usize];
         self.read_exact(&mut buffer)
-            .map_err(|_| "Invalid UTF-8".to_string())?;
+            .map_err(|_| BufReadError::InvalidUtf8)?;
         string.push_str(std::str::from_utf8(&buffer).unwrap());
         if string.len() > length as usize {
-            return Err(format!(
-                "The received string length is longer than maximum allowed ({} > {})",
-                length, max_length
-            ));
+            return Err(BufReadError::StringLengthTooLong { length, max_length });
         }
 
         Ok(string)
     }
 
     /// Read a single byte from the reader
-    fn read_byte(&mut self) -> Result<u8, String> {
-        self.read_u8().map_err(|_| "Error reading byte".to_string())
+    fn read_byte(&mut self) -> Result<u8, BufReadError> {
+        Ok(self.read_u8()?)
     }
 
-    fn read_int(&mut self) -> Result<i32, String> {
-        match self.read_i32::<BE>() {
-            Ok(r) => Ok(r),
-            Err(_) => Err("Error reading int".to_string()),
-        }
+    fn read_int(&mut self) -> Result<i32, BufReadError> {
+        Ok(self.read_i32::<BE>()?)
     }
 
-    fn read_boolean(&mut self) -> Result<bool, String> {
+    fn read_boolean(&mut self) -> Result<bool, BufReadError> {
         match self.read_byte()? {
             0 => Ok(false),
             1 => Ok(true),
-            _ => Err("Error reading boolean".to_string()),
+            _ => Err(BufReadError::InvalidBoolean),
         }
     }
 
-    fn read_long(&mut self) -> Result<i64, String> {
-        match self.read_i64::<BE>() {
-            Ok(r) => Ok(r),
-            Err(_) => Err("Error reading long".to_string()),
-        }
+    fn read_long(&mut self) -> Result<i64, BufReadError> {
+        Ok(self.read_i64::<BE>()?)
     }
 
-    fn read_short(&mut self) -> Result<i16, String> {
-        match self.read_i16::<BE>() {
-            Ok(r) => Ok(r),
-            Err(_) => Err("Error reading short".to_string()),
-        }
+    fn read_short(&mut self) -> Result<i16, BufReadError> {
+        Ok(self.read_i16::<BE>()?)
     }
 
-    fn read_float(&mut self) -> Result<f32, String> {
-        match self.read_f32::<BE>() {
-            Ok(r) => Ok(r),
-            Err(_) => Err("Error reading float".to_string()),
-        }
+    fn read_float(&mut self) -> Result<f32, BufReadError> {
+        Ok(self.read_f32::<BE>()?)
     }
 
-    fn read_double(&mut self) -> Result<f64, String> {
-        match self.read_f64::<BE>() {
-            Ok(r) => Ok(r),
-            Err(_) => Err("Error reading double".to_string()),
-        }
+    fn read_double(&mut self) -> Result<f64, BufReadError> {
+        Ok(self.read_f64::<BE>()?)
     }
 }
 
 // fast varints modified from https://github.com/luojia65/mc-varint/blob/master/src/lib.rs#L67
 /// Read a single varint from the reader and return the value, along with the number of bytes read
-pub async fn read_varint_async(reader: &mut (dyn AsyncRead + Unpin + Send)) -> Result<i32, String> {
+pub async fn read_varint_async(
+    reader: &mut (dyn AsyncRead + Unpin + Send),
+) -> Result<i32, BufReadError> {
     let mut buffer = [0];
     let mut ans = 0;
     for i in 0..5 {
         reader
             .read_exact(&mut buffer)
             .await
-            .map_err(|_| "Invalid VarInt".to_string())?;
+            .map_err(|_| BufReadError::InvalidVarInt)?;
         ans |= ((buffer[0] & 0b0111_1111) as i32) << (7 * i);
         if buffer[0] & 0b1000_0000 == 0 {
             return Ok(ans);
@@ -201,36 +209,36 @@ pub trait McBufReadable
 where
     Self: Sized,
 {
-    fn read_from(buf: &mut impl Read) -> Result<Self, String>;
+    fn read_from(buf: &mut impl Read) -> Result<Self, BufReadError>;
 }
 
 pub trait McBufVarReadable
 where
     Self: Sized,
 {
-    fn var_read_from(buf: &mut impl Read) -> Result<Self, String>;
+    fn var_read_from(buf: &mut impl Read) -> Result<Self, BufReadError>;
 }
 
 impl McBufReadable for i32 {
-    fn read_from(buf: &mut impl Read) -> Result<Self, String> {
+    fn read_from(buf: &mut impl Read) -> Result<Self, BufReadError> {
         Readable::read_int(buf)
     }
 }
 
 impl McBufVarReadable for i32 {
-    fn var_read_from(buf: &mut impl Read) -> Result<Self, String> {
+    fn var_read_from(buf: &mut impl Read) -> Result<Self, BufReadError> {
         buf.read_varint()
     }
 }
 
 impl McBufVarReadable for i64 {
     // fast varints modified from https://github.com/luojia65/mc-varint/blob/master/src/lib.rs#L54
-    fn var_read_from(buf: &mut impl Read) -> Result<Self, String> {
+    fn var_read_from(buf: &mut impl Read) -> Result<Self, BufReadError> {
         let mut buffer = [0];
         let mut ans = 0;
         for i in 0..8 {
             buf.read_exact(&mut buffer)
-                .map_err(|_| "Invalid VarLong".to_string())?;
+                .map_err(|_| BufReadError::InvalidVarLong)?;
             ans |= ((buffer[0] & 0b0111_1111) as i64) << (7 * i);
             if buffer[0] & 0b1000_0000 == 0 {
                 break;
@@ -240,19 +248,19 @@ impl McBufVarReadable for i64 {
     }
 }
 impl McBufVarReadable for u64 {
-    fn var_read_from(buf: &mut impl Read) -> Result<Self, String> {
+    fn var_read_from(buf: &mut impl Read) -> Result<Self, BufReadError> {
         i64::var_read_from(buf).map(|i| i as u64)
     }
 }
 
 impl McBufReadable for UnsizedByteArray {
-    fn read_from(buf: &mut impl Read) -> Result<Self, String> {
+    fn read_from(buf: &mut impl Read) -> Result<Self, BufReadError> {
         Ok(buf.read_bytes()?.into())
     }
 }
 
 impl<T: McBufReadable + Send> McBufReadable for Vec<T> {
-    default fn read_from(buf: &mut impl Read) -> Result<Self, String> {
+    default fn read_from(buf: &mut impl Read) -> Result<Self, BufReadError> {
         let length = buf.read_varint()? as usize;
         let mut contents = Vec::with_capacity(length);
         for _ in 0..length {
@@ -263,7 +271,7 @@ impl<T: McBufReadable + Send> McBufReadable for Vec<T> {
 }
 
 impl<K: McBufReadable + Send + Eq + Hash, V: McBufReadable + Send> McBufReadable for HashMap<K, V> {
-    default fn read_from(buf: &mut impl Read) -> Result<Self, String> {
+    default fn read_from(buf: &mut impl Read) -> Result<Self, BufReadError> {
         let length = buf.read_varint()? as usize;
         let mut contents = HashMap::with_capacity(length);
         for _ in 0..length {
@@ -274,49 +282,49 @@ impl<K: McBufReadable + Send + Eq + Hash, V: McBufReadable + Send> McBufReadable
 }
 
 impl McBufReadable for Vec<u8> {
-    fn read_from(buf: &mut impl Read) -> Result<Self, String> {
+    fn read_from(buf: &mut impl Read) -> Result<Self, BufReadError> {
         buf.read_byte_array()
     }
 }
 
 impl McBufReadable for String {
-    fn read_from(buf: &mut impl Read) -> Result<Self, String> {
+    fn read_from(buf: &mut impl Read) -> Result<Self, BufReadError> {
         buf.read_utf()
     }
 }
 
 impl McBufReadable for u32 {
-    fn read_from(buf: &mut impl Read) -> Result<Self, String> {
+    fn read_from(buf: &mut impl Read) -> Result<Self, BufReadError> {
         Readable::read_int(buf).map(|i| i as u32)
     }
 }
 
 impl McBufVarReadable for u32 {
-    fn var_read_from(buf: &mut impl Read) -> Result<Self, String> {
+    fn var_read_from(buf: &mut impl Read) -> Result<Self, BufReadError> {
         buf.read_varint().map(|i| i as u32)
     }
 }
 
 impl McBufReadable for u16 {
-    fn read_from(buf: &mut impl Read) -> Result<Self, String> {
+    fn read_from(buf: &mut impl Read) -> Result<Self, BufReadError> {
         buf.read_short().map(|i| i as u16)
     }
 }
 
 impl McBufReadable for i16 {
-    fn read_from(buf: &mut impl Read) -> Result<Self, String> {
+    fn read_from(buf: &mut impl Read) -> Result<Self, BufReadError> {
         buf.read_short()
     }
 }
 
 impl McBufVarReadable for u16 {
-    fn var_read_from(buf: &mut impl Read) -> Result<Self, String> {
+    fn var_read_from(buf: &mut impl Read) -> Result<Self, BufReadError> {
         buf.read_varint().map(|i| i as u16)
     }
 }
 
 impl<T: McBufVarReadable> McBufVarReadable for Vec<T> {
-    fn var_read_from(buf: &mut impl Read) -> Result<Self, String> {
+    fn var_read_from(buf: &mut impl Read) -> Result<Self, BufReadError> {
         let length = buf.read_varint()? as usize;
         let mut contents = Vec::with_capacity(length);
         for _ in 0..length {
@@ -327,49 +335,49 @@ impl<T: McBufVarReadable> McBufVarReadable for Vec<T> {
 }
 
 impl McBufReadable for i64 {
-    fn read_from(buf: &mut impl Read) -> Result<Self, String> {
+    fn read_from(buf: &mut impl Read) -> Result<Self, BufReadError> {
         buf.read_long()
     }
 }
 
 impl McBufReadable for u64 {
-    fn read_from(buf: &mut impl Read) -> Result<Self, String> {
+    fn read_from(buf: &mut impl Read) -> Result<Self, BufReadError> {
         i64::read_from(buf).map(|i| i as u64)
     }
 }
 
 impl McBufReadable for bool {
-    fn read_from(buf: &mut impl Read) -> Result<Self, String> {
+    fn read_from(buf: &mut impl Read) -> Result<Self, BufReadError> {
         buf.read_boolean()
     }
 }
 
 impl McBufReadable for u8 {
-    fn read_from(buf: &mut impl Read) -> Result<Self, String> {
+    fn read_from(buf: &mut impl Read) -> Result<Self, BufReadError> {
         buf.read_byte()
     }
 }
 
 impl McBufReadable for i8 {
-    fn read_from(buf: &mut impl Read) -> Result<Self, String> {
+    fn read_from(buf: &mut impl Read) -> Result<Self, BufReadError> {
         buf.read_byte().map(|i| i as i8)
     }
 }
 
 impl McBufReadable for f32 {
-    fn read_from(buf: &mut impl Read) -> Result<Self, String> {
+    fn read_from(buf: &mut impl Read) -> Result<Self, BufReadError> {
         buf.read_float()
     }
 }
 
 impl McBufReadable for f64 {
-    fn read_from(buf: &mut impl Read) -> Result<Self, String> {
+    fn read_from(buf: &mut impl Read) -> Result<Self, BufReadError> {
         buf.read_double()
     }
 }
 
 impl<T: McBufReadable> McBufReadable for Option<T> {
-    default fn read_from(buf: &mut impl Read) -> Result<Self, String> {
+    default fn read_from(buf: &mut impl Read) -> Result<Self, BufReadError> {
         let present = buf.read_boolean()?;
         Ok(if present {
             Some(T::read_from(buf)?)
diff --git a/azalea-buf/src/serializable_uuid.rs b/azalea-buf/src/serializable_uuid.rs
index eb256d90..fad5edfc 100644
--- a/azalea-buf/src/serializable_uuid.rs
+++ b/azalea-buf/src/serializable_uuid.rs
@@ -1,4 +1,4 @@
-use crate::{McBufReadable, McBufWritable, Readable};
+use crate::{read::BufReadError, McBufReadable, McBufWritable, Readable};
 use std::io::{Read, Write};
 use uuid::Uuid;
 
@@ -33,7 +33,7 @@ impl SerializableUuid for Uuid {
 }
 
 impl McBufReadable for Uuid {
-    fn read_from(buf: &mut impl Read) -> Result<Self, String> {
+    fn read_from(buf: &mut impl Read) -> Result<Self, BufReadError> {
         Ok(Uuid::from_int_array([
             Readable::read_int(buf)? as u32,
             Readable::read_int(buf)? as u32,
-- 
cgit v1.2.3