Take Arc<Http> in ClientBuilder::new_with_http (#2749)

This allows the user to make a http, pass it off to a background task,
then use it in serenity, instead of having to initialize two.

Cargo.toml

@@ -91,7 +91,7 @@ framework = ["client", "model", "utils"]
 # Enables gateway support, which allows bots to listen for Discord events.
 gateway = ["flate2"]
 # Enables HTTP, which enables bots to execute actions on Discord.
-http = ["dashmap", "mime_guess", "percent-encoding"]
+http = ["dashmap", "parking_lot", "mime_guess", "percent-encoding"]
 # Enables wrapper methods around HTTP requests on model types.
 # Requires "builder" to configure the requests and "http" to execute them.
 # Note: the model type definitions themselves are always active, regardless of this feature.

src/client/mod.rs

@@ -65,7 +65,7 @@ use crate::utils::check_shard_total;
 #[must_use = "Builders do nothing unless they are awaited"]
 pub struct ClientBuilder {
     data: Option<Arc<dyn std::any::Any + Send + Sync>>,
-    http: Http,
+    http: Arc<Http>,
     intents: GatewayIntents,
     #[cfg(feature = "cache")]
     cache_settings: CacheSettings,
@@ -87,7 +87,7 @@ impl ClientBuilder {
     /// framework via the [`Self::framework`] method, otherwise awaiting the builder will cause a
     /// panic.
     pub fn new(token: &str, intents: GatewayIntents) -> Self {
-        Self::new_with_http(Http::new(token), intents)
+        Self::new_with_http(Arc::new(Http::new(token)), intents)
     }
 
     /// Construct a new builder with a [`Http`] instance to calls methods on for the client
@@ -96,7 +96,7 @@ impl ClientBuilder {
     /// **Panic**: If you have enabled the `framework`-feature (on by default), you must specify a
     /// framework via the [`Self::framework`] method, otherwise awaiting the builder will cause a
     /// panic.
-    pub fn new_with_http(http: Http, intents: GatewayIntents) -> Self {
+    pub fn new_with_http(http: Arc<Http>, intents: GatewayIntents) -> Self {
         Self {
             http,
             intents,
@@ -113,14 +113,8 @@ impl ClientBuilder {
         }
     }
 
-    /// Sets a token for the bot. If the token is not prefixed "Bot ", this method will
-    /// automatically do so.
-    pub fn token(mut self, token: &str) -> Self {
-        self.http = Http::new(token);
-        self
-    }
-
     /// Gets the current token used for the [`Http`] client.
+    #[must_use]
     pub fn get_token(&self) -> &str {
         self.http.token()
     }
@@ -134,6 +128,7 @@ impl ClientBuilder {
 
     /// Gets the application ID, if already initialized. See [`Self::application_id`] for more
     /// info.
+    #[must_use]
     pub fn get_application_id(&self) -> Option<ApplicationId> {
         self.http.application_id()
     }
@@ -155,6 +150,7 @@ impl ClientBuilder {
 
     /// Gets the cache settings. See [`Self::cache_settings`] for more info.
     #[cfg(feature = "cache")]
+    #[must_use]
     pub fn get_cache_settings(&self) -> &CacheSettings {
         &self.cache_settings
     }
@@ -176,6 +172,7 @@ impl ClientBuilder {
 
     /// Gets the framework, if already initialized. See [`Self::framework`] for more info.
     #[cfg(feature = "framework")]
+    #[must_use]
     pub fn get_framework(&self) -> Option<&dyn Framework> {
         self.framework.as_deref()
     }
@@ -193,6 +190,7 @@ impl ClientBuilder {
 
     /// Gets the voice manager, if already initialized. See [`Self::voice_manager`] for more info.
     #[cfg(feature = "voice")]
+    #[must_use]
     pub fn get_voice_manager(&self) -> Option<Arc<dyn VoiceGatewayManager>> {
         self.voice_manager.clone()
     }
@@ -224,6 +222,7 @@ impl ClientBuilder {
     }
 
     /// Gets the intents. See [`Self::intents`] for more info.
+    #[must_use]
     pub fn get_intents(&self) -> GatewayIntents {
         self.intents
     }
@@ -239,6 +238,7 @@ impl ClientBuilder {
     }
 
     /// Gets the added event handlers. See [`Self::event_handler`] for more info.
+    #[must_use]
     pub fn get_event_handlers(&self) -> &[Arc<dyn EventHandler>] {
         &self.event_handlers
     }
@@ -252,6 +252,7 @@ impl ClientBuilder {
     }
 
     /// Gets the added raw event handlers. See [`Self::raw_event_handler`] for more info.
+    #[must_use]
     pub fn get_raw_event_handlers(&self) -> &[Arc<dyn RawEventHandler>] {
         &self.raw_event_handlers
     }
@@ -271,6 +272,7 @@ impl ClientBuilder {
     }
 
     /// Gets the initial presence. See [`Self::activity`] and [`Self::status`] for more info.
+    #[must_use]
     pub fn get_presence(&self) -> &PresenceData {
         &self.presence
     }
@@ -291,10 +293,9 @@ impl IntoFuture for ClientBuilder {
         let raw_event_handlers = self.raw_event_handlers;
         let intents = self.intents;
         let presence = self.presence;
+        let http = self.http;
 
-        let mut http = self.http;
-
-        if let Some(ratelimiter) = &mut http.ratelimiter {
+        if let Some(ratelimiter) = &http.ratelimiter {
             let event_handlers_clone = event_handlers.clone();
             ratelimiter.set_ratelimit_callback(Box::new(move |info| {
                 for event_handler in &event_handlers_clone {
@@ -305,8 +306,6 @@ impl IntoFuture for ClientBuilder {
             }));
         }
 
-        let http = Arc::new(http);
-
         #[cfg(feature = "voice")]
         let voice_manager = self.voice_manager;
 

src/http/ratelimiting.rs

@@ -88,7 +88,7 @@ pub struct Ratelimiter {
     routes: DashMap<RatelimitingBucket, Ratelimit>,
     token: Secret<Token>,
     absolute_ratelimits: bool,
-    ratelimit_callback: Box<dyn Fn(RatelimitInfo) + Send + Sync>,
+    ratelimit_callback: parking_lot::RwLock<Box<dyn Fn(RatelimitInfo) + Send + Sync>>,
 }
 
 impl fmt::Debug for Ratelimiter {
@@ -115,17 +115,17 @@ impl Ratelimiter {
             token: Token::new(token),
             global: Mutex::default(),
             routes: DashMap::new(),
-            ratelimit_callback: Box::new(|_| {}),
             absolute_ratelimits: false,
+            ratelimit_callback: parking_lot::RwLock::new(Box::new(|_| {})),
         }
     }
 
     /// Sets a callback to be called when a route is rate limited.
     pub fn set_ratelimit_callback(
-        &mut self,
+        &self,
         ratelimit_callback: Box<dyn Fn(RatelimitInfo) + Send + Sync>,
     ) {
-        self.ratelimit_callback = ratelimit_callback;
+        *self.ratelimit_callback.write() = ratelimit_callback;
     }
 
     // Sets whether absolute ratelimits should be used.
@@ -187,7 +187,7 @@ impl Ratelimiter {
             let ratelimiting_bucket = req.route.ratelimiting_bucket();
             let delay_time = {
                 let mut bucket = self.routes.entry(ratelimiting_bucket).or_default();
-                bucket.pre_hook(&req, &self.ratelimit_callback)
+                bucket.pre_hook(&req, &*self.ratelimit_callback.read())
             };
 
             if let Some(delay_time) = delay_time {
@@ -224,7 +224,7 @@ impl Ratelimiter {
                             "Ratelimited on route {:?} for {:?}s",
                             ratelimiting_bucket, retry_after
                         );
-                        (self.ratelimit_callback)(RatelimitInfo {
+                        (self.ratelimit_callback.read())(RatelimitInfo {
                             timeout: Duration::from_secs_f64(retry_after),
                             limit: 50,
                             method: req.method,
@@ -244,7 +244,7 @@ impl Ratelimiter {
                     bucket.post_hook(
                         &response,
                         &req,
-                        &self.ratelimit_callback,
+                        &*self.ratelimit_callback.read(),
                         self.absolute_ratelimits,
                     )
                 } else {