Shrink size and clean up Error (#2700)

Shrinks `size_of::<Error>` from 136 bytes to 64 bytes, while removing unused
variants. This will improve performance for any method returning
`Result<T>` where `T` is less than the size of `Error` as both `Result`'s
`Ok` and `Err` have to be allocated stack space.

Author: GnomedDev

examples/testing/src/main.rs

@@ -57,7 +57,8 @@ async fn message(ctx: &Context, msg: Message) -> Result<(), serenity::Error> {
         let mut msg = channel_id
             .send_message(
                 &ctx,
-                CreateMessage::new().add_file(CreateAttachment::url(ctx, IMAGE_URL).await?),
+                CreateMessage::new()
+                    .add_file(CreateAttachment::url(ctx, IMAGE_URL, "testing.png").await?),
             )
             .await?;
         // Pre-PR, this falsely triggered a MODEL_TYPE_CONVERT Discord error
@@ -69,13 +70,15 @@ async fn message(ctx: &Context, msg: Message) -> Result<(), serenity::Error> {
         let mut msg = channel_id
             .send_message(
                 ctx,
-                CreateMessage::new().add_file(CreateAttachment::url(ctx, IMAGE_URL).await?),
+                CreateMessage::new()
+                    .add_file(CreateAttachment::url(ctx, IMAGE_URL, "testing.png").await?),
             )
             .await?;
         msg.edit(
             ctx,
             EditMessage::new().attachments(
-                EditAttachments::keep_all(&msg).add(CreateAttachment::url(ctx, IMAGE_URL_2).await?),
+                EditAttachments::keep_all(&msg)
+                    .add(CreateAttachment::url(ctx, IMAGE_URL_2, "testing1.png").await?),
             ),
         )
         .await?;
@@ -196,7 +199,7 @@ async fn message(ctx: &Context, msg: Message) -> Result<(), serenity::Error> {
                 ctx,
                 CreateMessage::new()
                     .flags(MessageFlags::IS_VOICE_MESSAGE)
-                    .add_file(CreateAttachment::url(ctx, audio_url).await?),
+                    .add_file(CreateAttachment::url(ctx, audio_url, "testing.ogg").await?),
             )
             .await?;
     } else if let Some(channel) = msg.content.strip_prefix("movetorootandback") {
@@ -231,7 +234,7 @@ async fn interaction(
                 &ctx,
                 CreateInteractionResponse::Message(
                     CreateInteractionResponseMessage::new()
-                        .add_file(CreateAttachment::url(ctx, IMAGE_URL).await?),
+                        .add_file(CreateAttachment::url(ctx, IMAGE_URL, "testing.png").await?),
                 ),
             )
             .await?;
@@ -245,7 +248,7 @@ async fn interaction(
                 &ctx,
                 EditInteractionResponse::new().attachments(
                     EditAttachments::keep_all(&msg)
-                        .add(CreateAttachment::url(ctx, IMAGE_URL_2).await?),
+                        .add(CreateAttachment::url(ctx, IMAGE_URL_2, "testing1.png").await?),
                 ),
             )
             .await?;
@@ -286,7 +289,7 @@ async fn interaction(
                 ctx,
                 CreateInteractionResponse::Message(
                     CreateInteractionResponseMessage::new()
-                        .add_file(CreateAttachment::url(ctx, IMAGE_URL).await?),
+                        .add_file(CreateAttachment::url(ctx, IMAGE_URL, "testing.png").await?),
                 ),
             )
             .await?;
@@ -295,15 +298,15 @@ async fn interaction(
             .edit_response(
                 ctx,
                 EditInteractionResponse::new()
-                    .new_attachment(CreateAttachment::url(ctx, IMAGE_URL_2).await?),
+                    .new_attachment(CreateAttachment::url(ctx, IMAGE_URL_2, "testing1.png").await?),
             )
             .await?;
 
         interaction
             .create_followup(
                 ctx,
                 CreateInteractionResponseFollowup::new()
-                    .add_file(CreateAttachment::url(ctx, IMAGE_URL).await?),
+                    .add_file(CreateAttachment::url(ctx, IMAGE_URL, "testing.png").await?),
             )
             .await?;
     } else if interaction.data.name == "editembeds" {

src/builder/create_attachment.rs

@@ -3,15 +3,12 @@ use std::path::Path;
 
 use tokio::fs::File;
 use tokio::io::AsyncReadExt;
-#[cfg(feature = "http")]
-use url::Url;
 
-use crate::all::Message;
-#[cfg(feature = "http")]
-use crate::error::Error;
-use crate::error::Result;
+#[allow(unused)] // Error is used in docs
+use crate::error::{Error, Result};
 #[cfg(feature = "http")]
 use crate::http::Http;
+use crate::model::channel::Message;
 use crate::model::id::AttachmentId;
 
 /// Enum that allows a user to pass a [`Path`] or a [`File`] type to [`send_files`]
@@ -81,20 +78,17 @@ impl<'a> CreateAttachment<'a> {
     ///
     /// # Errors
     ///
-    /// [`Error::Url`] if the URL is invalid, [`Error::Http`] if downloading the data fails.
+    /// Returns [`Error::Http`] if downloading the data fails.
     #[cfg(feature = "http")]
-    pub async fn url(http: impl AsRef<Http>, url: &str) -> Result<Self> {
-        let url = Url::parse(url).map_err(|_| Error::Url(url.to_string()))?;
-
-        let response = http.as_ref().client.get(url.clone()).send().await?;
+    pub async fn url(
+        http: impl AsRef<Http>,
+        url: impl reqwest::IntoUrl,
+        filename: impl Into<Cow<'static, str>>,
+    ) -> Result<Self> {
+        let response = http.as_ref().client.get(url).send().await?;
         let data = response.bytes().await?.to_vec();
 
-        let filename = url
-            .path_segments()
-            .and_then(Iterator::last)
-            .ok_or_else(|| Error::Url(url.to_string()))?;
-
-        Ok(CreateAttachment::bytes(data, filename.to_owned()))
+        Ok(CreateAttachment::bytes(data, filename))
     }
 
     /// Converts the stored data to the base64 representation.

src/client/error.rs

@@ -12,15 +12,12 @@ use std::fmt;
 pub enum Error {
     /// When a shard has completely failed to reboot after resume and/or reconnect attempts.
     ShardBootFailure,
-    /// When all shards that the client is responsible for have shutdown with an error.
-    Shutdown,
 }
 
 impl fmt::Display for Error {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
             Self::ShardBootFailure => f.write_str("Failed to (re-)boot a shard"),
-            Self::Shutdown => f.write_str("The clients shards shutdown"),
         }
     }
 }

src/client/mod.rs

@@ -57,6 +57,8 @@ use crate::internal::prelude::*;
 #[cfg(feature = "gateway")]
 use crate::model::gateway::GatewayIntents;
 use crate::model::id::ApplicationId;
+#[cfg(feature = "voice")]
+use crate::model::id::UserId;
 use crate::model::user::OnlineStatus;
 use crate::utils::check_shard_total;
 
@@ -633,7 +635,9 @@ impl Client {
     ///
     /// # Errors
     ///
-    /// Returns a [`ClientError::Shutdown`] when all shards have shutdown due to an error.
+    /// Returns [`Error::Gateway`] when all shards have shutdown due to an error.
+    /// Returns [`Error::Http`] if fetching the current User fails when initialising a voice
+    /// manager.
     ///
     /// [gateway docs]: crate::gateway#sharding
     #[cfg_attr(feature = "tracing_instrument", instrument(skip(self)))]
@@ -674,7 +678,9 @@ impl Client {
     ///
     /// # Errors
     ///
-    /// Returns a [`ClientError::Shutdown`] when all shards have shutdown due to an error.
+    /// Returns [`Error::Gateway`] when all shards have shutdown due to an error.
+    /// Returns [`Error::Http`] if fetching the current User fails when initialising a voice
+    /// manager.
     ///
     /// [gateway docs]: crate::gateway#sharding
     #[cfg_attr(feature = "tracing_instrument", instrument(skip(self)))]
@@ -738,7 +744,9 @@ impl Client {
     ///
     /// # Errors
     ///
-    /// Returns a [`ClientError::Shutdown`] when all shards have shutdown due to an error.
+    /// Returns [`Error::Gateway`] when all shards have shutdown due to an error.
+    /// Returns [`Error::Http`] if fetching the current User fails when initialising a voice
+    /// manager.
     ///
     /// [gateway docs]: crate::gateway#sharding
     #[cfg_attr(feature = "tracing_instrument", instrument(skip(self)))]
@@ -779,7 +787,9 @@ impl Client {
     ///
     /// # Errors
     ///
-    /// Returns a [`ClientError::Shutdown`] when all shards have shutdown due to an error.
+    /// Returns [`Error::Gateway`] when all shards have shutdown due to an error.
+    /// Returns [`Error::Http`] if fetching the current User fails when initialising a voice
+    /// manager.
     ///
     /// [Gateway docs]: crate::gateway#sharding
     #[cfg_attr(feature = "tracing_instrument", instrument(skip(self)))]
@@ -820,7 +830,9 @@ impl Client {
     ///
     /// # Errors
     ///
-    /// Returns a [`ClientError::Shutdown`] when all shards have shutdown due to an error.
+    /// Returns [`Error::Gateway`] when all shards have shutdown due to an error.
+    /// Returns [`Error::Http`] if fetching the current User fails when initialising a voice
+    /// manager.
     ///
     /// [Gateway docs]: crate::gateway#sharding
     #[cfg_attr(feature = "tracing_instrument", instrument(skip(self)))]
@@ -837,9 +849,25 @@ impl Client {
     ) -> Result<()> {
         #[cfg(feature = "voice")]
         if let Some(voice_manager) = &self.voice_manager {
-            let user = self.http.get_current_user().await?;
+            #[cfg(feature = "cache")]
+            let cache_user_id = {
+                let cache_user = self.cache.current_user();
+                if cache_user.id == UserId::default() {
+                    Some(cache_user.id)
+                } else {
+                    None
+                }
+            };
+
+            #[cfg(not(feature = "cache"))]
+            let cache_user_id: Option<UserId> = None;
+
+            let user_id = match cache_user_id {
+                Some(u) => u,
+                None => self.http.get_current_user().await?.id,
+            };
 
-            voice_manager.initialise(total_shards, user.id).await;
+            voice_manager.initialise(total_shards, user_id).await;
         }
 
         let init = end_shard - start_shard + 1;

src/error.rs

@@ -1,5 +1,5 @@
 use std::error::Error as StdError;
-use std::fmt::{self, Error as FormatError};
+use std::fmt;
 use std::io::Error as IoError;
 
 #[cfg(feature = "http")]
@@ -31,10 +31,6 @@ pub type Result<T, E = Error> = StdResult<T, E>;
 #[derive(Debug)]
 #[non_exhaustive]
 pub enum Error {
-    /// An error while decoding a payload.
-    Decode(&'static str, Value),
-    /// There was an error with a format.
-    Format(FormatError),
     /// An [`std::io`] error.
     Io(IoError),
     #[cfg_attr(not(feature = "simd_json"), doc = "An error from the [`serde_json`] crate.")]
@@ -44,20 +40,6 @@ pub enum Error {
     ///
     /// [`model`]: crate::model
     Model(ModelError),
-    /// The input is not in the specified range. Returned by [`GuildId::members`],
-    /// [`Guild::members`] and [`PartialGuild::members`]
-    ///
-    /// (param_name, value, range_min, range_max)
-    ///
-    /// [`GuildId::members`]: crate::model::id::GuildId::members
-    /// [`Guild::members`]: crate::model::guild::Guild::members
-    /// [`PartialGuild::members`]: crate::model::guild::PartialGuild::members
-    NotInRange(&'static str, u64, u64, u64),
-    /// Some other error. This is only used for "Expected value \<TYPE\>" errors, when a more
-    /// detailed error can not be easily provided via the [`Error::Decode`] variant.
-    Other(&'static str),
-    /// An error from the [`url`] crate.
-    Url(String),
     /// A [client] error.
     ///
     /// [client]: crate::client
@@ -75,13 +57,7 @@ pub enum Error {
     Http(HttpError),
     /// An error from the `tungstenite` crate.
     #[cfg(feature = "gateway")]
-    Tungstenite(TungsteniteError),
-}
-
-impl From<FormatError> for Error {
-    fn from(e: FormatError) -> Error {
-        Error::Format(e)
-    }
+    Tungstenite(Box<TungsteniteError>),
 }
 
 #[cfg(feature = "gateway")]
@@ -112,7 +88,7 @@ impl From<ModelError> for Error {
 #[cfg(feature = "gateway")]
 impl From<TungsteniteError> for Error {
     fn from(e: TungsteniteError) -> Error {
-        Error::Tungstenite(e)
+        Error::Tungstenite(Box::new(e))
     }
 }
 
@@ -140,13 +116,9 @@ impl From<ReqwestError> for Error {
 impl fmt::Display for Error {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
-            Self::Decode(msg, _) | Self::Other(msg) => f.write_str(msg),
-            Self::NotInRange(..) => f.write_str("Input is not in the specified range"),
-            Self::Format(inner) => fmt::Display::fmt(&inner, f),
             Self::Io(inner) => fmt::Display::fmt(&inner, f),
             Self::Json(inner) => fmt::Display::fmt(&inner, f),
             Self::Model(inner) => fmt::Display::fmt(&inner, f),
-            Self::Url(msg) => f.write_str(msg),
             #[cfg(feature = "client")]
             Self::Client(inner) => fmt::Display::fmt(&inner, f),
             #[cfg(feature = "gateway")]
@@ -163,7 +135,6 @@ impl StdError for Error {
     #[cfg_attr(feature = "tracing_instrument", instrument)]
     fn source(&self) -> Option<&(dyn StdError + 'static)> {
         match self {
-            Self::Format(inner) => Some(inner),
             Self::Io(inner) => Some(inner),
             Self::Json(inner) => Some(inner),
             Self::Model(inner) => Some(inner),

src/framework/standard/help_commands.rs

@@ -897,7 +897,7 @@ fn flatten_group_to_string(
     group: &GroupCommandsPair,
     nest_level: usize,
     help_options: &HelpOptions,
-) -> Result<(), Error> {
+) -> Result<(), std::fmt::Error> {
     let repeated_indent_str = help_options.indention_prefix.repeat(nest_level);
 
     if nest_level > 0 {
@@ -1002,7 +1002,8 @@ async fn send_grouped_commands_embed(
     for group in groups {
         let mut embed_text = String::default();
 
-        flatten_group_to_string(&mut embed_text, group, 0, help_options)?;
+        flatten_group_to_string(&mut embed_text, group, 0, help_options)
+            .expect("String::write should never fail");
 
         embed = embed.field(group.name, embed_text, true);
     }

src/gateway/bridge/shard_runner.rs

@@ -398,7 +398,7 @@ impl ShardRunner {
     async fn recv_event(&mut self) -> Result<(Option<Event>, Option<ShardAction>, bool)> {
         let gw_event = match self.shard.client.recv_json().await {
             Ok(inner) => Ok(inner),
-            Err(Error::Tungstenite(TungsteniteError::Io(_))) => {
+            Err(Error::Tungstenite(tung_err)) if matches!(*tung_err, TungsteniteError::Io(_)) => {
                 debug!("Attempting to auto-reconnect");
 
                 match self.shard.reconnection_type() {

src/gateway/shard.rs

@@ -201,17 +201,16 @@ impl Shard {
                 Ok(())
             },
             Err(why) => {
-                match why {
-                    Error::Tungstenite(TungsteniteError::Io(err)) => {
+                if let Error::Tungstenite(err) = &why {
+                    if let TungsteniteError::Io(err) = &**err {
                         if err.raw_os_error() != Some(32) {
                             debug!("[{:?}] Err heartbeating: {:?}", self.shard_info, err);
+                            return Err(Error::Gateway(GatewayError::HeartbeatFailed));
                         }
-                    },
-                    other => {
-                        warn!("[{:?}] Other err w/ keepalive: {:?}", self.shard_info, other);
-                    },
+                    }
                 }
 
+                warn!("[{:?}] Other err w/ keepalive: {:?}", self.shard_info, why);
                 Err(Error::Gateway(GatewayError::HeartbeatFailed))
             },
         }