fix(mpd): Handle broken connections

include/adapters/mpd.hpp

@@ -3,6 +3,7 @@
 #include <mpd/client.h>
 #include <stdlib.h>
 #include <chrono>
+#include <csignal>
 
 #include "common.hpp"
 #include "errors.hpp"
@@ -15,6 +16,8 @@ class logger;
 namespace chrono = std::chrono;
 
 namespace mpd {
+  extern sig_atomic_t g_connection_closed;
+
   DEFINE_ERROR(mpd_exception);
   DEFINE_CHILD_ERROR(client_error, mpd_exception);
   DEFINE_CHILD_ERROR(server_error, mpd_exception);
@@ -78,6 +81,7 @@ namespace mpd {
    public:
     explicit mpdconnection(
         const logger& logger, string host, unsigned int port = 6600, string password = "", unsigned int timeout = 15);
+    ~mpdconnection();
 
     void connect();
     void disconnect();
@@ -114,6 +118,8 @@ namespace mpd {
     const logger& m_log;
     mpd_connection_t m_connection{};
 
+    struct sigaction m_signal_action {};
+
     bool m_listactive = false;
     bool m_idle = false;
     int m_fd = -1;

include/modules/mpd.hpp

@@ -10,8 +10,6 @@ POLYBAR_NS
 
 using namespace mpd;
 
-namespace chrono = std::chrono;
-
 namespace modules {
   class mpd_module : public event_module<mpd_module>, public input_handler {
    public:
@@ -70,9 +68,11 @@ namespace modules {
     chrono::system_clock::time_point m_lastsync{};
     float m_synctime{1.0f};
 
+    int m_quick_attempts{0};
+
     // This flag is used to let thru a broadcast once every time
     // the connection state changes
-    mpd::connection_state m_statebroadcasted{mpd::connection_state::NONE};
+    connection_state m_statebroadcasted{connection_state::NONE};
 
     progressbar_t m_bar_progress;
     iconset_t m_icons;

src/adapters/mpd.cpp

@@ -1,35 +1,43 @@
 #include <cassert>
+#include <csignal>
 #include <thread>
 #include <utility>
 
 #include "adapters/mpd.hpp"
 #include "components/logger.hpp"
 #include "utils/math.hpp"
+#include "utils/string.hpp"
 
 POLYBAR_NS
 
 namespace mpd {
+  sig_atomic_t g_connection_closed = 0;
+  void g_mpd_signal_handler(int signum) {
+    if (signum == SIGPIPE) {
+      g_connection_closed = 1;
+    }
+  }
+
   void check_connection(mpd_connection* conn) {
-    if (conn == nullptr) {
+    if (g_connection_closed) {
-      throw client_error("Not connected to MPD server", MPD_ERROR_STATE);
+      g_connection_closed = 0;
+      throw server_error("Connection closed (broken pipe)");
+    } else if (conn == nullptr) {
+      throw client_error("Not connected to server", MPD_ERROR_STATE);
     }
   }
 
   void check_errors(mpd_connection* conn) {
-    mpd_error code = mpd_connection_get_error(conn);
+    check_connection(conn);
-
+    switch (mpd_connection_get_error(conn)) {
-    if (code == MPD_ERROR_SUCCESS) {
+      case MPD_ERROR_SUCCESS:
-      return;
+        return;
-    }
+      case MPD_ERROR_SERVER:
-
+        mpd_connection_clear_error(conn);
-    auto msg = mpd_connection_get_error_message(conn);
+        throw server_error(mpd_connection_get_error_message(conn), mpd_connection_get_server_error(conn));
-
+      default:
-    if (code == MPD_ERROR_SERVER) {
+        mpd_connection_clear_error(conn);
-      mpd_connection_clear_error(conn);
+        throw client_error(mpd_connection_get_error_message(conn), mpd_connection_get_error(conn));
-      throw server_error(msg, mpd_connection_get_server_error(conn));
-    } else {
-      mpd_connection_clear_error(conn);
-      throw client_error(msg, code);
     }
   }
 
@@ -70,28 +78,19 @@ namespace mpd {
   string mpdsong::get_album() {
     assert(m_song);
     auto tag = mpd_song_get_tag(m_song.get(), MPD_TAG_ALBUM, 0);
-    if (tag == nullptr) {
+    return string{tag != nullptr ? tag : ""};
-      return "";
-    }
-    return string{tag};
   }
 
   string mpdsong::get_date() {
     assert(m_song);
     auto tag = mpd_song_get_tag(m_song.get(), MPD_TAG_DATE, 0);
-    if (tag == nullptr) {
+    return string{tag != nullptr ? tag : ""};
-      return "";
-    }
-    return string{tag};
   }
 
   string mpdsong::get_title() {
     assert(m_song);
     auto tag = mpd_song_get_tag(m_song.get(), MPD_TAG_TITLE, 0);
-    if (tag == nullptr) {
+    return string{tag != nullptr ? tag : ""};
-      return "";
-    }
-    return string{tag};
   }
 
   unsigned mpdsong::get_duration() {
@@ -104,7 +103,18 @@ namespace mpd {
 
   mpdconnection::mpdconnection(
       const logger& logger, string host, unsigned int port, string password, unsigned int timeout)
-      : m_log(logger), m_host(move(host)), m_port(port), m_password(move(password)), m_timeout(timeout) {}
+      : m_log(logger), m_host(move(host)), m_port(port), m_password(move(password)), m_timeout(timeout) {
+    memset(&m_signal_action, 0, sizeof(m_signal_action));
+    m_signal_action.sa_handler = &g_mpd_signal_handler;
+    if (sigaction(SIGPIPE, &m_signal_action, nullptr) == -1) {
+      throw mpd_exception("Could not setup signal handler: "s + std::strerror(errno));
+    }
+  }
+
+  mpdconnection::~mpdconnection() {
+    m_signal_action.sa_handler = SIG_DFL;
+    sigaction(SIGPIPE, &m_signal_action, nullptr);
+  }
 
   void mpdconnection::connect() {
     try {
@@ -134,10 +144,7 @@ namespace mpd {
   }
 
   bool mpdconnection::connected() {
-    if (!m_connection) {
+    return m_connection && m_connection != nullptr;
-      return false;
-    }
-    return m_connection != nullptr;
   }
 
   bool mpdconnection::retry_connection(int interval) {
@@ -150,9 +157,8 @@ namespace mpd {
         connect();
         return true;
       } catch (const mpd_exception& e) {
+        std::this_thread::sleep_for(chrono::duration<double>(interval));
       }
-
-      std::this_thread::sleep_for(chrono::duration<double>(interval));
     }
 
     return false;
@@ -164,12 +170,11 @@ namespace mpd {
 
   void mpdconnection::idle() {
     check_connection(m_connection.get());
-    if (m_idle) {
+    if (!m_idle) {
-      return;
+      mpd_send_idle(m_connection.get());
+      check_errors(m_connection.get());
+      m_idle = true;
     }
-    mpd_send_idle(m_connection.get());
-    check_errors(m_connection.get());
-    m_idle = true;
   }
 
   int mpdconnection::noidle() {
@@ -188,8 +193,6 @@ namespace mpd {
     check_prerequisites();
     auto status = make_unique<mpdstatus>(this);
     check_errors(m_connection.get());
-    // if (update)
-    //   status->update(-1, this);
     return status;
   }
 
@@ -440,7 +443,7 @@ namespace mpd {
       return 0;
     }
     math_util::cap<int>(0, 100, percentage);
-    return float(m_total_time) * percentage / 100.0f + 0.5f;
+    return math_util::percentage_to_value<double>(percentage, m_total_time);
   }
 
   // }}}

src/modules/mpd.cpp

@@ -1,16 +1,15 @@
-#include "modules/mpd.hpp"
+#include <csignal>
 
 #include "drawtypes/iconset.hpp"
 #include "drawtypes/label.hpp"
 #include "drawtypes/progressbar.hpp"
+#include "modules/mpd.hpp"
 #include "utils/factory.hpp"
 
 #include "modules/meta/base.inl"
 
 POLYBAR_NS
 
-using namespace mpd;
-
 namespace modules {
   template class module<mpd_module>;
 
@@ -104,9 +103,10 @@ namespace modules {
 
   void mpd_module::idle() {
     if (connected()) {
+      m_quick_attempts = 0;
       sleep(80ms);
     } else {
-      sleep(2s);
+      sleep(m_quick_attempts++ < 5 ? 0.5s : 2s);
     }
   }
 
@@ -144,7 +144,6 @@ namespace modules {
       m_mpd->idle();
 
       int idle_flags = 0;
-
       if ((idle_flags = m_mpd->noidle()) != 0) {
         m_status->update(idle_flags, m_mpd.get());
         return true;
@@ -154,7 +153,7 @@ namespace modules {
         m_status->update_timer();
       }
     } catch (const mpd_exception& err) {
-      m_log.err(err.what());
+      m_log.err("%s: %s", name(), err.what());
       m_mpd.reset();
       return def;
     }
@@ -211,7 +210,7 @@ namespace modules {
         }
       }
     } catch (const mpd_exception& err) {
-      m_log.err(err.what());
+      m_log.err("%s: %s", name(), err.what());
       m_mpd.reset();
     }
 
@@ -328,8 +327,7 @@ namespace modules {
         int percentage = 0;
         if (s.empty()) {
           return false;
-        }
+        } else if (s[0] == '+') {
-        if (s[0] == '+') {
           percentage = status->get_elapsed_percentage() + std::atoi(s.substr(1).c_str());
         } else if (s[0] == '-') {
           percentage = status->get_elapsed_percentage() - std::atoi(s.substr(1).c_str());