mainloop: propagate errors and implement connection retry

Fixes #1336 by returning the proper exit code and retrying connections

Author: eladyn

CHANGELOG.md

@@ -9,8 +9,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
 - revert default features to `alsa_backend` ([#1337])
 - do not require `bindgen` dependencies on supported systems ([#1340])
+- always return exitcode 1 on error ([#1338])
 
 [#1337]: https://github.com/Spotifyd/spotifyd/pull/1337
+[#1338]: https://github.com/Spotifyd/spotifyd/pull/1338
 [#1340]: https://github.com/Spotifyd/spotifyd/pull/1340
 
 ## [0.4.0]

src/main.rs

@@ -7,8 +7,6 @@ use config::ExecutionMode;
 #[cfg(unix)]
 use daemonize::Daemonize;
 use fern::colors::ColoredLevelConfig;
-#[cfg(unix)]
-use log::error;
 use log::{info, trace, LevelFilter};
 use oauth::run_oauth;
 #[cfg(target_os = "openbsd")]
@@ -172,7 +170,7 @@ fn run_daemon(mut cli_config: CliConfig) -> eyre::Result<()> {
             }
             match daemonize.start() {
                 Ok(_) => info!("Detached from shell, now running in background."),
-                Err(e) => error!("Something went wrong while daemonizing: {}", e),
+                Err(e) => return Err(e).wrap_err("something went wrong while daemonizing"),
             };
         }
         #[cfg(target_os = "windows")]
@@ -228,7 +226,6 @@ fn run_daemon(mut cli_config: CliConfig) -> eyre::Result<()> {
     let runtime = Runtime::new().unwrap();
     runtime.block_on(async {
         let initial_state = setup::initial_state(internal_config)?;
-        initial_state.run().await;
-        Ok(())
+        initial_state.run().await
     })
 }

src/main_loop.rs

@@ -3,6 +3,8 @@ use crate::config::{DBusType, MprisConfig};
 #[cfg(feature = "dbus_mpris")]
 use crate::dbus_mpris::DbusServer;
 use crate::process::spawn_program_on_event;
+use crate::utils::Backoff;
+use color_eyre::eyre::{self, Context};
 use futures::future::Either;
 #[cfg(not(feature = "dbus_mpris"))]
 use futures::future::Pending;
@@ -24,7 +26,7 @@ use librespot_playback::{
     mixer::Mixer,
     player::Player,
 };
-use log::error;
+use log::{error, info};
 use std::pin::Pin;
 use std::sync::Arc;
 
@@ -102,43 +104,63 @@ impl MainLoop {
     async fn get_connection(&mut self) -> Result<ConnectionInfo<impl Future<Output = ()>>, Error> {
         let creds = self.credentials_provider.get_credentials().await;
 
-        let session = Session::new(self.session_config.clone(), self.cache.clone());
-        let player = {
-            let audio_device = self.audio_device.clone();
-            let audio_format = self.audio_format;
-            let backend = self.backend;
-            Player::new(
-                self.player_config.clone(),
+        let mut connection_backoff = Backoff::default();
+        loop {
+            let session = Session::new(self.session_config.clone(), self.cache.clone());
+            let player = {
+                let audio_device = self.audio_device.clone();
+                let audio_format = self.audio_format;
+                let backend = self.backend;
+                Player::new(
+                    self.player_config.clone(),
+                    session.clone(),
+                    self.mixer.get_soft_volume(),
+                    move || backend(audio_device, audio_format),
+                )
+            };
+
+            // TODO: expose is_group
+            match Spirc::new(
+                ConnectConfig {
+                    name: self.device_name.clone(),
+                    device_type: self.device_type,
+                    is_group: false,
+                    initial_volume: self.initial_volume,
+                    has_volume_ctrl: self.has_volume_ctrl,
+                },
                 session.clone(),
-                self.mixer.get_soft_volume(),
-                move || backend(audio_device, audio_format),
+                creds.clone(),
+                player.clone(),
+                self.mixer.clone(),
             )
-        };
-
-        // TODO: expose is_group
-        Spirc::new(
-            ConnectConfig {
-                name: self.device_name.clone(),
-                device_type: self.device_type,
-                is_group: false,
-                initial_volume: self.initial_volume,
-                has_volume_ctrl: self.has_volume_ctrl,
-            },
-            session.clone(),
-            creds,
-            player.clone(),
-            self.mixer.clone(),
-        )
-        .await
-        .map(|(spirc, spirc_task)| ConnectionInfo {
-            spirc,
-            session,
-            player,
-            spirc_task,
-        })
+            .await
+            {
+                Ok((spirc, spirc_task)) => {
+                    break Ok(ConnectionInfo {
+                        spirc,
+                        session,
+                        player,
+                        spirc_task,
+                    })
+                }
+                Err(err) => {
+                    let Ok(backoff) = connection_backoff.next_backoff() else {
+                        break Err(err);
+                    };
+                    error!("connection to spotify failed: {err}");
+                    info!(
+                        "retrying connection in {} seconds (retry {}/{})",
+                        backoff.as_secs(),
+                        connection_backoff.retries(),
+                        connection_backoff.max_retries()
+                    );
+                    tokio::time::sleep(backoff).await;
+                }
+            }
+        }
     }
 
-    pub(crate) async fn run(mut self) {
+    pub(crate) async fn run(mut self) -> eyre::Result<()> {
         tokio::pin! {
             let ctrl_c = tokio::signal::ctrl_c();
             // we don't necessarily have a dbus server
@@ -157,18 +179,15 @@ impl MainLoop {
             None
         };
 
-        'mainloop: loop {
+        let mainloop_result: eyre::Result<()> = 'mainloop: loop {
             let connection = tokio::select!(
                 _ = &mut ctrl_c => {
-                    break 'mainloop;
+                    break 'mainloop Ok(());
                 }
                 connection = self.get_connection() => {
                     match connection {
                         Ok(connection) => connection,
-                        Err(err) => {
-                            error!("failed to connect to spotify: {}", err);
-                            break 'mainloop;
-                        }
+                        Err(err) => break 'mainloop Err(err).wrap_err("failed to connect to spotify"),
                     }
                 }
             );
@@ -184,9 +203,8 @@ impl MainLoop {
                     .as_mut()
                     .set_session(shared_spirc.clone(), connection.session)
                 {
-                    error!("failed to configure dbus server: {err}");
                     let _ = shared_spirc.shutdown();
-                    break 'mainloop;
+                    break 'mainloop Err(err).wrap_err("failed to configure dbus server");
                 }
             }
 
@@ -204,7 +222,7 @@ impl MainLoop {
                     // the program should shut down
                     _ = &mut ctrl_c => {
                         let _ = shared_spirc.shutdown();
-                        break 'mainloop;
+                        break 'mainloop Ok(());
                     }
                     // spirc was shut down by some external factor
                     _ = &mut spirc_task => {
@@ -214,12 +232,9 @@ impl MainLoop {
                     result = &mut dbus_server => {
                         #[cfg(feature = "dbus_mpris")]
                         {
-                            if let Err(err) = result {
-                                error!("DBus terminated unexpectedly: {err}");
-                            }
                             let _ = shared_spirc.shutdown();
                             *dbus_server.as_mut() = Either::Right(future::pending());
-                            break 'mainloop;
+                            break 'mainloop result.wrap_err("DBus terminated unexpectedly");
                         }
                         #[cfg(not(feature = "dbus_mpris"))]
                         result // unused variable
@@ -252,21 +267,27 @@ impl MainLoop {
             #[cfg(feature = "dbus_mpris")]
             if let Either::Left(dbus_server) = Either::as_pin_mut(dbus_server.as_mut()) {
                 if let Err(err) = dbus_server.drop_session() {
-                    error!("failed to reconfigure dbus server: {err}");
-                    break 'mainloop;
+                    break 'mainloop Err(err).wrap_err("failed to reconfigure DBus server");
                 }
             }
-        }
+        };
+
         if let CredentialsProvider::Discovery { stream, .. } = self.credentials_provider {
             let _ = stream.into_inner().shutdown().await;
         }
         #[cfg(feature = "dbus_mpris")]
         if let Either::Left(dbus_server) = Either::as_pin_mut(dbus_server.as_mut()) {
             if dbus_server.shutdown() {
                 if let Err(err) = dbus_server.await {
-                    error!("failed to shutdown the dbus server: {err}");
+                    let err = Err(err).wrap_err("failed to shutdown DBus server");
+                    if mainloop_result.is_ok() {
+                        return err;
+                    } else {
+                        error!("additional error while shutting down: {err:?}");
+                    }
                 }
             }
         }
+        mainloop_result
     }
 }

src/setup.rs

@@ -3,6 +3,7 @@ use crate::alsa_mixer;
 use crate::{
     config,
     main_loop::{self, CredentialsProvider},
+    utils::Backoff,
 };
 use color_eyre::{eyre::eyre, Section};
 use futures::StreamExt as _;
@@ -11,7 +12,7 @@ use librespot_playback::{
     mixer::{self, Mixer, MixerConfig},
 };
 use log::{debug, error, info};
-use std::{sync::Arc, thread, time::Duration};
+use std::{sync::Arc, thread};
 
 pub(crate) fn initial_state(
     config: config::SpotifydConfig,
@@ -75,9 +76,7 @@ pub(crate) fn initial_state(
     let discovery = if config.discovery {
         info!("Starting zeroconf server to advertise on local network.");
         debug!("Using device id '{}'", session_config.device_id);
-        const RETRY_MAX: u8 = 4;
-        let mut retry_counter = 0;
-        let mut backoff = Duration::from_secs(5);
+        let mut retry_backoff = Backoff::default();
         loop {
             match librespot_discovery::Discovery::builder(
                 session_config.device_id.clone(),
@@ -91,15 +90,17 @@ pub(crate) fn initial_state(
                 Ok(discovery_stream) => break Some(discovery_stream),
                 Err(err) => {
                     error!("failed to enable discovery: {err}");
-                    if retry_counter >= RETRY_MAX {
+                    let Ok(backoff) = retry_backoff.next_backoff() else {
                         error!("maximum amount of retries exceeded");
                         break None;
-                    }
+                    };
                     info!("retrying discovery in {} seconds", backoff.as_secs());
                     thread::sleep(backoff);
-                    retry_counter += 1;
-                    backoff *= 2;
-                    info!("trying to enable discovery (retry {retry_counter}/{RETRY_MAX})");
+                    info!(
+                        "trying to enable discovery (retry {}/{})",
+                        retry_backoff.retries(),
+                        retry_backoff.max_retries()
+                    );
                 }
             }
         }

src/utils.rs

@@ -1,5 +1,5 @@
 use log::trace;
-use std::env;
+use std::{env, time::Duration};
 
 #[cfg(any(
     target_os = "freebsd",
@@ -82,6 +82,51 @@ pub(crate) fn get_shell() -> Option<String> {
     shell
 }
 
+/// Implements basic exponential backoff.
+/// The used base is 2.
+pub(crate) struct Backoff {
+    retry_counter: u8,
+    max_retries: u8,
+    initial_backoff: Duration,
+}
+
+impl Backoff {
+    pub(crate) fn new(max_retries: u8, initial_backoff: Duration) -> Self {
+        Self {
+            retry_counter: 0,
+            max_retries,
+            initial_backoff,
+        }
+    }
+
+    /// Get the next backoff duration.
+    ///
+    /// Increases the retry counter and returns the next required backoff duration,
+    /// if available.
+    pub(crate) fn next_backoff(&mut self) -> Result<Duration, ()> {
+        if self.retry_counter >= self.max_retries {
+            return Err(());
+        }
+        let backoff_duration = self.initial_backoff * 2u32.pow(self.retry_counter as u32);
+        self.retry_counter += 1;
+        Ok(backoff_duration)
+    }
+
+    pub(crate) fn retries(&self) -> u8 {
+        self.retry_counter
+    }
+
+    pub(crate) fn max_retries(&self) -> u8 {
+        self.max_retries
+    }
+}
+
+impl Default for Backoff {
+    fn default() -> Self {
+        Self::new(4, Duration::from_secs(5))
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;