Remove Event::Unknown variant (#2708)

If deserialization fails, we should allow the specific deserialization error to
be logged so that it can be reported to the Serenity maintainers. As it stands,
any events that fail to deserialize normally will "succeed" when deserialized
into the `Unknown` variant, throwing away any extra information about the
error.

Because deserialization errors are immediately logged when they happen,
explicitly whitelist `Error::Json` in `Shard::handle_event` to prevent
duplicate logs.

src/client/dispatch.rs

@@ -1,7 +1,5 @@
 use std::sync::Arc;
 
-use tracing::debug;
-
 #[cfg(feature = "gateway")]
 use super::event_handler::{EventHandler, RawEventHandler};
 use super::{Context, FullEvent};
@@ -55,33 +53,31 @@ pub(crate) fn dispatch_model(
         tokio::spawn(async move { raw_handler.raw_event(context, event).await });
     }
 
-    let full_events = update_cache_with_event(
+    let (full_event, extra_event) = update_cache_with_event(
         #[cfg(feature = "cache")]
         &context.cache,
         event,
     );
 
-    if let Some(events) = full_events {
-        let iter = std::iter::once(events.0).chain(events.1);
-        for handler in event_handlers {
-            for event in iter.clone() {
-                let context = context.clone();
-                let handler = Arc::clone(&handler);
-                spawn_named(event.snake_case_name(), async move {
-                    event.dispatch(context, &*handler).await;
-                });
-            }
+    let iter = std::iter::once(full_event).chain(extra_event);
+    for handler in event_handlers {
+        for event in iter.clone() {
+            let context = context.clone();
+            let handler = Arc::clone(&handler);
+            spawn_named(event.snake_case_name(), async move {
+                event.dispatch(context, &*handler).await;
+            });
         }
+    }
 
-        #[cfg(feature = "framework")]
-        if let Some(framework) = framework {
-            for event in iter {
-                let context = context.clone();
-                let framework = Arc::clone(&framework);
-                spawn_named("dispatch::framework::dispatch", async move {
-                    framework.dispatch(context, event).await;
-                });
-            }
+    #[cfg(feature = "framework")]
+    if let Some(framework) = framework {
+        for event in iter {
+            let context = context.clone();
+            let framework = Arc::clone(&framework);
+            spawn_named("dispatch::framework::dispatch", async move {
+                framework.dispatch(context, event).await;
+            });
         }
     }
 }
@@ -96,7 +92,7 @@ pub(crate) fn dispatch_model(
 fn update_cache_with_event(
     #[cfg(feature = "cache")] cache: &Cache,
     event: Event,
-) -> Option<(FullEvent, Option<FullEvent>)> {
+) -> (FullEvent, Option<FullEvent>) {
     let mut extra_event = None;
     let event = match event {
         Event::CommandPermissionsUpdate(event) => FullEvent::CommandPermissionsUpdate {
@@ -369,10 +365,6 @@ fn update_cache_with_event(
         Event::TypingStart(event) => FullEvent::TypingStart {
             event,
         },
-        Event::Unknown(event) => {
-            debug!("An unknown event was received: {event:?}");
-            return None;
-        },
         Event::UserUpdate(mut event) => {
             let before = if_cache!(event.update(cache));
 
@@ -488,5 +480,5 @@ fn update_cache_with_event(
         },
     };
 
-    Some((event, extra_event))
+    (event, extra_event)
 }

src/gateway/bridge/shard_runner.rs

@@ -396,8 +396,11 @@ impl ShardRunner {
     /// successful.
     #[cfg_attr(feature = "tracing_instrument", instrument(skip(self)))]
     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),
+        let gateway_event = match self.shard.client.recv_json().await {
+            Ok(Some(inner)) => Ok(inner),
+            Ok(None) => {
+                return Ok((None, None, true));
+            },
             Err(Error::Tungstenite(tung_err)) if matches!(*tung_err, TungsteniteError::Io(_)) => {
                 debug!("Attempting to auto-reconnect");
 
@@ -420,13 +423,7 @@ impl ShardRunner {
             Err(why) => Err(why),
         };
 
-        let event = match gw_event {
-            Ok(Some(event)) => Ok(event),
-            Ok(None) => return Ok((None, None, true)),
-            Err(why) => Err(why),
-        };
-
-        let action = match self.shard.handle_event(&event) {
+        let action = match self.shard.handle_event(&gateway_event) {
             Ok(Some(action)) => Some(action),
             Ok(None) => None,
             Err(why) => {
@@ -447,18 +444,18 @@ impl ShardRunner {
             },
         };
 
-        if let Ok(GatewayEvent::HeartbeatAck) = event {
+        if let Ok(GatewayEvent::HeartbeatAck) = gateway_event {
             self.update_manager().await;
         }
 
         #[cfg(feature = "voice")]
         {
-            if let Ok(GatewayEvent::Dispatch(_, ref event)) = event {
+            if let Ok(GatewayEvent::Dispatch(_, ref event)) = gateway_event {
                 self.handle_voice_event(event).await;
             }
         }
 
-        let event = match event {
+        let event = match gateway_event {
             Ok(GatewayEvent::Dispatch(_, event)) => Some(event),
             _ => None,
         };

src/gateway/shard.rs

@@ -475,7 +475,11 @@ impl Shard {
                 Ok(Some(ShardAction::Reconnect(self.reconnection_type())))
             },
             Err(why) => {
-                warn!("[{:?}] Unhandled error: {:?}", self.shard_info, why);
+                if let Error::Json(_) = why {
+                    // Deserialization errors already get logged when the event is first received
+                } else {
+                    warn!("[{:?}] Unhandled error: {:?}", self.shard_info, why);
+                }
 
                 Ok(None)
             },

src/model/event.rs

@@ -1311,21 +1311,13 @@ pub enum Event {
     EntitlementUpdate(EntitlementUpdateEvent),
     /// A user's entitlement was deleted by Discord, or refunded.
     EntitlementDelete(EntitlementDeleteEvent),
-    /// An event type not covered by the above
-    #[serde(untagged)]
-    Unknown(UnknownEvent),
 }
 
 impl Event {
-    /// Return the event name of this event. Returns [`None`] if the event is
-    /// [`Unknown`](Event::Unknown).
+    /// Returns the event name of this event.
     #[must_use]
     pub fn name(&self) -> Option<String> {
-        if let Self::Unknown(_) = self {
-            None
-        } else {
-            let map = serde_json::to_value(self).ok()?;
-            Some(map.get("t")?.as_str()?.to_string())
-        }
+        let map = serde_json::to_value(self).ok()?;
+        Some(map.get("t")?.as_str()?.to_string())
     }
 }