Merge pull request #41 from SpoticordMusic/dev

Update Spoticord to v2.2.4

CHANGELOG.md

@@ -1,5 +1,12 @@
 # Changelog
 
+## 2.2.4 | TBD
+
+- Added a message for if the Spotify AP connection drops
+- Added additional timeouts to credential retrieval
+- Removed multiple points of failure in `librespot` that could shut down the bot
+- Fixed an issue where non-premium users could crash the bot for everyone (See point 3)
+
 ## 2.2.3 | September 20th 2024
 
 - Made backend changes to librespot to prevent deadlocking by not waiting for thread shutdowns

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "spoticord"
-version = "2.2.3"
+version = "2.2.4"
 edition = "2021"
 rust-version = "1.80.0"
 
@@ -39,6 +39,7 @@ poise = "0.6.1"
 serenity = "0.12.2"
 songbird = { version = "0.4.3", features = ["simd-json"] }
 tokio = { version = "1.39.3", features = ["full"] }
+rustls = { version = "0.23.13", features = ["aws-lc-rs"] }
 
 [profile.release]
 opt-level = 3

spoticord_audio/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "spoticord_audio"
-version = "2.2.3"
+version = "2.2.4"
 edition = "2021"
 
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

spoticord_audio/src/sink.rs

@@ -24,7 +24,7 @@ impl StreamSink {
 impl Sink for StreamSink {
     fn start(&mut self) -> SinkResult<()> {
         if let Err(_why) = self.sender.send(SinkEvent::Start) {
-            // WARNING: Returning an error causes librespot-playback to exit the process with status 1
+            // WARNING: Returning an error causes librespot-playback to panic
 
             // return Err(SinkError::ConnectionRefused(_why.to_string()));
         }
@@ -34,7 +34,7 @@ impl Sink for StreamSink {
 
     fn stop(&mut self) -> SinkResult<()> {
         if let Err(_why) = self.sender.send(SinkEvent::Stop) {
-            // WARNING: Returning an error causes librespot-playback to exit the process with status 1
+            // WARNING: Returning an error causes librespot-playback to panic
 
             // return Err(SinkError::ConnectionRefused(_why.to_string()));
         }

spoticord_config/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "spoticord_config"
-version = "2.2.3"
+version = "2.2.4"
 edition = "2021"
 
 [dependencies]

spoticord_config/src/env.rs

@@ -15,3 +15,7 @@ pub static SPOTIFY_CLIENT_SECRET: LazyLock<String> = LazyLock::new(|| {
     std::env::var("SPOTIFY_CLIENT_SECRET")
         .expect("missing SPOTIFY_CLIENT_SECRET environment variable")
 });
+
+// Locked behind `stats` feature
+pub static KV_URL: LazyLock<String> =
+    LazyLock::new(|| std::env::var("KV_URL").expect("missing KV_URL environment variable"));

spoticord_config/src/lib.rs

@@ -31,6 +31,10 @@ pub fn link_url() -> &'static str {
     &env::LINK_URL
 }
 
+pub fn kv_url() -> &'static str {
+    &env::KV_URL
+}
+
 pub fn get_spotify(token: Token) -> AuthCodeSpotify {
     AuthCodeSpotify::from_token_with_config(
         token,

spoticord_database/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "spoticord_database"
-version = "2.2.3"
+version = "2.2.4"
 edition = "2021"
 
 [dependencies]

spoticord_player/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "spoticord_player"
-version = "2.2.3"
+version = "2.2.4"
 edition = "2021"
 
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

spoticord_player/src/lib.rs

@@ -13,13 +13,16 @@ use librespot::{
         player::{Player as SpotifyPlayer, PlayerEvent as SpotifyPlayerEvent},
     },
 };
-use log::error;
+use log::{error, trace};
 use songbird::{input::RawAdapter, tracks::TrackHandle, Call};
 use spoticord_audio::{
     sink::{SinkEvent, StreamSink},
     stream::Stream,
 };
-use std::{io::Write, sync::Arc};
+use std::{
+    io::Write,
+    sync::{atomic::AtomicBool, Arc},
+};
 use tokio::sync::{mpsc, oneshot, Mutex};
 
 #[derive(Debug)]
@@ -41,6 +44,7 @@ pub enum PlayerEvent {
     Play,
     Stopped,
     TrackChanged(Box<PlaybackInfo>),
+    ConnectionReset,
 }
 
 pub struct Player {
@@ -57,6 +61,9 @@ pub struct Player {
     commands: mpsc::Receiver<PlayerCommand>,
     spotify_events: mpsc::UnboundedReceiver<SpotifyPlayerEvent>,
     sink_events: mpsc::UnboundedReceiver<SinkEvent>,
+
+    /// A shared boolean that reflects whether this Player has shut down
+    shutdown: Arc<AtomicBool>,
 }
 
 impl Player {
@@ -132,6 +139,7 @@ impl Player {
             }
         };
 
+        let shutdown = Arc::new(AtomicBool::new(false));
         let (tx, rx) = mpsc::channel(16);
         let player = Self {
             session,
@@ -141,15 +149,24 @@ impl Player {
 
             playback_info: None,
 
-            events: event_tx,
+            events: event_tx.clone(),
 
             commands: rx,
             spotify_events: rx_player,
             sink_events: rx_sink,
+
+            shutdown: shutdown.clone(),
         };
 
         // Launch it all!
-        tokio::spawn(spirc_task);
+        tokio::spawn(async move {
+            spirc_task.await;
+
+            // If the shutdown flag isn't set, we most likely lost connection to the Spotify AP
+            if !shutdown.load(std::sync::atomic::Ordering::SeqCst) {
+                _ = event_tx.send(PlayerEvent::ConnectionReset).await;
+            }
+        });
         tokio::spawn(player.run());
 
         Ok((PlayerHandle { commands: tx }, event_rx))
@@ -178,6 +195,11 @@ impl Player {
                 else => break,
             }
         }
+
+        self.shutdown
+            .store(true, std::sync::atomic::Ordering::SeqCst);
+
+        trace!("End of Player::run");
     }
 
     async fn handle_command(&mut self, command: PlayerCommand) {
@@ -195,6 +217,8 @@ impl Player {
     }
 
     async fn handle_spotify_event(&mut self, event: SpotifyPlayerEvent) {
+        trace!("Spotify event received: {event:#?}");
+
         match event {
             SpotifyPlayerEvent::PositionCorrection { position_ms, .. }
             | SpotifyPlayerEvent::Seeked { position_ms, .. } => {

spoticord_session/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "spoticord_session"
-version = "2.2.3"
+version = "2.2.4"
 edition = "2021"
 
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

spoticord_session/src/lib.rs

@@ -106,7 +106,7 @@ impl Session {
                 Err(why) => {
                     error!("Failed to retrieve credentials: {why}");
 
-                    return Err(why.into());
+                    return Err(why);
                 }
             };
         let device_name = match session_manager.database().get_user(owner.to_string()).await {
@@ -301,6 +301,22 @@ impl Session {
             PlayerEvent::Pause => self.start_timeout(),
             PlayerEvent::Stopped => self.shutdown_player().await,
             PlayerEvent::TrackChanged(_) => {}
+            PlayerEvent::ConnectionReset => {
+                self.disconnect().await;
+
+                _ = self
+                    .text_channel
+                    .send_message(
+                        &self.context,
+                        CreateMessage::new().embed(
+                            CreateEmbed::new()
+                                .title("Spotify connection lost")
+                                .description("The bot has lost connection to the Spotify AP servers.\nThis is most likely caused by a connection reset on Spotify's end.\n\nUse `/join` to resummon the bot to your voice channel.")
+                                .color(Colors::Error),
+                        ),
+                    )
+                    .await;
+            }
         }
 
         let force_edit = !matches!(event, PlayerEvent::TrackChanged(_));
@@ -490,13 +506,13 @@ impl SessionHandle {
     /// This playback embed will automatically update when certain events happen
     pub async fn create_playback_embed(
         &self,
-        interaction: CommandInteraction,
+        interaction: &CommandInteraction,
         behavior: playback_embed::UpdateBehavior,
     ) -> Result<()> {
         self.commands
             .send(SessionCommand::CreatePlaybackEmbed(
                 self.clone(),
-                interaction,
+                interaction.to_owned(),
                 behavior,
             ))
             .await?;
@@ -600,7 +616,7 @@ async fn retrieve_credentials(database: &Database, owner: impl AsRef<str>) -> Re
         None => {
             let access_token = database.get_access_token(&account.user_id).await?;
             let credentials = spotify::request_session_token(Credentials {
-                username: account.username.clone(),
+                username: Some(account.username.to_string()),
                 auth_type: AuthenticationType::AUTHENTICATION_SPOTIFY_TOKEN,
                 auth_data: access_token.into_bytes(),
             })
@@ -616,7 +632,7 @@ async fn retrieve_credentials(database: &Database, owner: impl AsRef<str>) -> Re
     };
 
     Ok(Credentials {
-        username: account.username,
+        username: Some(account.username),
         auth_type: AuthenticationType::AUTHENTICATION_STORED_SPOTIFY_CREDENTIALS,
         auth_data: BASE64.decode(token)?,
     })

spoticord_stats/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "spoticord_stats"
-version = "2.2.3"
+version = "2.2.4"
 edition = "2021"
 
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

spoticord_utils/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "spoticord_utils"
-version = "2.2.3"
+version = "2.2.4"
 edition = "2021"
 
 [dependencies]

spoticord_utils/src/spotify.rs

@@ -1,9 +1,9 @@
 use anyhow::Result;
 use base64::{engine::general_purpose::STANDARD as BASE64, Engine};
 use librespot::{
-    core::{connection::AuthenticationError, Session, SessionConfig},
+    core::{Session, SessionConfig},
     discovery::Credentials,
-    protocol::{authentication::AuthenticationType, keyexchange::ErrorCode},
+    protocol::authentication::AuthenticationType,
 };
 use log::debug;
 use std::time::Duration;
@@ -15,17 +15,17 @@ pub async fn validate_token(
     let auth_data = BASE64.decode(token.into())?;
 
     let credentials = Credentials {
-        username: username.into(),
+        username: Some(username.into()),
         auth_type: AuthenticationType::AUTHENTICATION_STORED_SPOTIFY_CREDENTIALS,
         auth_data,
     };
 
-    debug!("Validating session token for {}", credentials.username);
+    debug!("Validating session token for {:?}", credentials.username);
 
     let new_credentials = request_session_token(credentials.clone()).await?;
 
     if credentials.auth_data != new_credentials.auth_data {
-        debug!("New session token retrieved for {}", credentials.username);
+        debug!("New session token retrieved for {:?}", credentials.username);
 
         return Ok(Some(BASE64.encode(new_credentials.auth_data)));
     }
@@ -34,54 +34,45 @@ pub async fn validate_token(
 }
 
 pub async fn request_session_token(credentials: Credentials) -> Result<Credentials> {
-    debug!("Requesting session token for {}", credentials.username);
+    debug!("Requesting session token for {:?}", credentials.username);
 
     let session = Session::new(SessionConfig::default(), None);
     let mut tries = 0;
 
     Ok(loop {
-        let (host, port) = session.apresolver().resolve("accesspoint").await?;
-
-        let mut transport = match librespot::core::connection::connect(&host, port, None).await {
-            Ok(transport) => transport,
-            Err(why) => {
-                // Retry
-
+        match connect(&session, credentials.clone()).await {
+            Ok(creds) => break creds,
+            Err(e) => {
                 tries += 1;
                 if tries > 3 {
-                    return Err(why.into());
+                    return Err(e);
                 }
 
                 tokio::time::sleep(Duration::from_millis(100)).await;
-
-                continue;
             }
-        };
+        }
+    })
+}
 
-        match librespot::core::connection::authenticate(
+/// Wrapper around session connecting that times out if an operation is still busy after 3 seconds
+async fn connect(session: &Session, credentials: Credentials) -> Result<Credentials> {
+    const TIMEOUT: Duration = Duration::from_secs(3);
+
+    let (host, port) =
+        tokio::time::timeout(TIMEOUT, session.apresolver().resolve("accesspoint")).await??;
+
+    // `connect` already has a 3 second timeout internally
+    let mut transport = librespot::core::connection::connect(&host, port, None).await?;
+
+    let creds = tokio::time::timeout(
+        TIMEOUT,
+        librespot::core::connection::authenticate(
             &mut transport,
             credentials.clone(),
             &session.config().device_id,
-        )
-        .await
-        {
-            Ok(creds) => break creds,
-            Err(e) => {
-                if let Some(AuthenticationError::LoginFailed(ErrorCode::TryAnotherAP)) =
-                    e.error.downcast_ref::<AuthenticationError>()
-                {
-                    tries += 1;
-                    if tries > 3 {
-                        return Err(e.into());
-                    }
+        ),
+    )
+    .await??;
 
-                    tokio::time::sleep(Duration::from_millis(100)).await;
-
-                    continue;
-                } else {
-                    return Err(e.into());
-                }
-            }
-        };
-    })
+    Ok(creds)
 }

src/bot.rs

@@ -68,7 +68,7 @@ pub async fn setup(
     let manager = SessionManager::new(songbird, database);
 
     #[cfg(feature = "stats")]
-    let stats = StatsManager::new(std::env::var("KV_URL")?)?;
+    let stats = StatsManager::new(spoticord_config::kv_url())?;
 
     tokio::spawn(background_loop(
         manager.clone(),

src/commands/music/playing.rs

@@ -38,10 +38,7 @@ pub async fn playing(
     };
 
     session
-        .create_playback_embed(
-            context.interaction.clone(),
-            update_behavior.unwrap_or_default(),
-        )
+        .create_playback_embed(context.interaction, update_behavior.unwrap_or_default())
         .await?;
 
     Ok(())

src/main.rs

@@ -1,7 +1,5 @@
 mod bot;
 mod commands;
-// mod session;
-// mod utils;
 
 use log::{error, info};
 use poise::Framework;
@@ -11,6 +9,11 @@ use spoticord_database::Database;
 
 #[tokio::main]
 async fn main() {
+    // Force aws-lc-rs as default crypto provider
+    // Since multiple dependencies either enable aws_lc_rs or ring, they cause a clash, so we have to
+    // explicitly tell rustls to use the aws-lc-rs provider
+    _ = rustls::crypto::aws_lc_rs::default_provider().install_default();
+
     // Setup logging
     if std::env::var("RUST_LOG").is_err() {
         #[cfg(debug_assertions)]