Fix out of order cache updates (#3342)

Co-authored-by: Alex M. M. <[email protected]>

Author: GnomedDev

src/gateway/client/dispatch.rs

@@ -7,6 +7,7 @@ use crate::cache::{Cache, CacheUpdate};
 #[cfg(feature = "framework")]
 use crate::framework::Framework;
 use crate::internal::prelude::*;
+use crate::internal::tokio::spawn_named;
 use crate::model::channel::ChannelType;
 use crate::model::event::Event;
 use crate::model::guild::Member;
@@ -40,9 +41,6 @@ macro_rules! update_cache {
 }
 
 /// Calls the user's event handlers and the framework handler.
-///
-/// This MUST be called from a different task to the recv_event loop, to allow for
-/// intra-shard concurrency between the shard loop and event handler.
 pub(crate) async fn dispatch_model(
     event: Box<Event>,
     context: Context,
@@ -60,14 +58,16 @@ pub(crate) async fn dispatch_model(
         event,
     );
 
-    #[cfg(feature = "framework")]
-    tokio::join!(
-        dispatch_framework(&context, framework, &full_event, extra_event.as_ref()),
-        dispatch_event_handler(&context, event_handler, &full_event, extra_event.as_ref())
-    );
+    spawn_named("dispatch::user", async move {
+        #[cfg(feature = "framework")]
+        tokio::join!(
+            dispatch_framework(&context, framework, &full_event, extra_event.as_ref()),
+            dispatch_event_handler(&context, event_handler, &full_event, extra_event.as_ref())
+        );
 
-    #[cfg(not(feature = "framework"))]
-    dispatch_event_handler(&context, event_handler, &full_event, extra_event.as_ref()).await;
+        #[cfg(not(feature = "framework"))]
+        dispatch_event_handler(&context, event_handler, &full_event, extra_event.as_ref()).await;
+    });
 }
 
 #[cfg(feature = "framework")]

src/gateway/client/event_handler.rs

@@ -6,6 +6,8 @@ use async_trait::async_trait;
 use strum::{EnumCount, IntoStaticStr, VariantNames};
 
 use super::context::Context;
+#[cfg(doc)]
+use crate::gateway::ShardRunner;
 use crate::gateway::ShardStageUpdateEvent;
 use crate::http::RatelimitInfo;
 use crate::model::prelude::*;
@@ -19,12 +21,8 @@ pub trait EventHandler: Send + Sync {
     ///
     /// ## Warning
     ///
-    /// This will run synchronously on every event in the dispatch loop
-    /// of the shard that is receiving the event. If your filter code
-    /// takes too long, it may delay other events from being dispatched
-    /// in a timely manner. It is recommended to keep the runtime
-    /// complexity of the filter code low to avoid unnecessarily blocking
-    /// your bot.
+    /// Similar to [`RawEventHandler`], this method runs synchronously to the [`ShardRunner`], keep
+    /// runtime complexity low.
     fn filter_event(&self, _context: &Context, _event: &Event) -> bool {
         true
     }
@@ -415,7 +413,17 @@ full_event! {
     MessagePollVoteRemove { event: MessagePollVoteRemoveEvent };
 }
 
-/// This core trait for handling raw events
+/// An event handler that receives raw `dispatch` events.
+///
+/// ## Warning
+/// As this is a low level trait, the methods of this trait are run on the same tokio task as the
+/// [`ShardRunner`].
+///
+/// This means that if any of these methods take too long to return, the shard may drop events or be
+/// disconnected entirely.
+///
+/// It is recommended to clone the fields needed out of [`Event`], then spawn a task to run
+/// concurrently to the shard loop.
 #[async_trait]
 pub trait RawEventHandler: Send + Sync {
     /// Dispatched when any event occurs
@@ -425,15 +433,6 @@ pub trait RawEventHandler: Send + Sync {
     ///
     /// Returning `false` will drop an event and prevent it being dispatched by any frameworks and
     /// will exclude it from any collectors.
-    ///
-    /// ## Warning
-    ///
-    /// This will run synchronously on every event in the dispatch loop
-    /// of the shard that is receiving the event. If your filter code
-    /// takes too long, it may delay other events from being dispatched
-    /// in a timely manner. It is recommended to keep the runtime
-    /// complexity of the filter code low to avoid unnecessarily blocking
-    /// your bot.
     fn filter_event(&self, _context: &Context, _event: &Event) -> bool {
         // Suppress unused argument warnings
         true

src/gateway/sharding/shard_runner.rs

@@ -214,17 +214,15 @@ impl ShardRunner {
                             #[cfg(feature = "collector")]
                             self.collectors.write().retain(|callback| (callback.0)(&event));
 
-                            spawn_named(
-                                "shard_runner::dispatch",
-                                dispatch_model(
-                                    event,
-                                    context,
-                                    #[cfg(feature = "framework")]
-                                    self.framework.clone(),
-                                    self.event_handler.clone(),
-                                    self.raw_event_handler.clone(),
-                                ),
-                            );
+                            dispatch_model(
+                                event,
+                                context,
+                                #[cfg(feature = "framework")]
+                                self.framework.clone(),
+                                self.event_handler.clone(),
+                                self.raw_event_handler.clone(),
+                            )
+                            .await;
                         }
                     },
                 }