Merge pull request #2984 from alebastr/too-much-logs-mpd

mpd: less aggressive logging and reconnections

include/modules/mpd/state.hpp

@@ -148,6 +148,7 @@ class Stopped : public State {
 class Disconnected : public State {
   Context* const ctx_;
   sigc::connection timer_connection_;
+  int last_interval_;
 
  public:
   Disconnected(Context* const ctx) : ctx_{ctx} {}
@@ -162,7 +163,7 @@ class Disconnected : public State {
   Disconnected(Disconnected const&) = delete;
   Disconnected& operator=(Disconnected const&) = delete;
 
-  void arm_timer(int interval) noexcept;
+  bool arm_timer(int interval) noexcept;
   void disarm_timer() noexcept;
 
   bool on_timer();

resources/config.jsonc

@@ -72,7 +72,7 @@
         "format-disconnected": "Disconnected ",
         "format-stopped": "{consumeIcon}{randomIcon}{repeatIcon}{singleIcon}Stopped ",
         "unknown-tag": "N/A",
-        "interval": 2,
+        "interval": 5,
         "consume-icons": {
             "on": " "
         },

src/modules/mpd/mpd.cpp

@@ -4,6 +4,7 @@
 #include <glibmm/ustring.h>
 #include <spdlog/spdlog.h>
 
+#include <system_error>
 #include <util/sanitize_str.hpp>
 using namespace waybar::util;
 
@@ -52,10 +53,10 @@ auto waybar::modules::MPD::update() -> void {
 
 void waybar::modules::MPD::queryMPD() {
   if (connection_ != nullptr) {
-    spdlog::debug("{}: fetching state information", module_name_);
+    spdlog::trace("{}: fetching state information", module_name_);
     try {
       fetchState();
-      spdlog::debug("{}: fetch complete", module_name_);
+      spdlog::trace("{}: fetch complete", module_name_);
     } catch (std::exception const& e) {
       spdlog::error("{}: {}", module_name_, e.what());
       state_ = MPD_STATE_UNKNOWN;
@@ -254,6 +255,21 @@ std::string waybar::modules::MPD::getOptionIcon(std::string optionName, bool act
   }
 }
 
+static bool isServerUnavailable(const std::error_code& ec) {
+  if (ec.category() == std::system_category()) {
+    switch (ec.value()) {
+      case ECONNREFUSED:
+      case ECONNRESET:
+      case ENETDOWN:
+      case ENETUNREACH:
+      case EHOSTDOWN:
+      case ENOENT:
+        return true;
+    }
+  }
+  return false;
+}
+
 void waybar::modules::MPD::tryConnect() {
   if (connection_ != nullptr) {
     return;
@@ -281,6 +297,11 @@ void waybar::modules::MPD::tryConnect() {
       }
       checkErrors(connection_.get());
     }
+  } catch (std::system_error& e) {
+    /* Tone down logs if it's likely that the mpd server is not running */
+    auto level = isServerUnavailable(e.code()) ? spdlog::level::debug : spdlog::level::err;
+    spdlog::log(level, "{}: Failed to connect to MPD: {}", module_name_, e.what());
+    connection_.reset();
   } catch (std::runtime_error& e) {
     spdlog::error("{}: Failed to connect to MPD: {}", module_name_, e.what());
     connection_.reset();
@@ -298,6 +319,12 @@ void waybar::modules::MPD::checkErrors(mpd_connection* conn) {
       connection_.reset();
       state_ = MPD_STATE_UNKNOWN;
       throw std::runtime_error("Connection to MPD closed");
+    case MPD_ERROR_SYSTEM:
+      if (auto ec = mpd_connection_get_system_error(conn); ec != 0) {
+        mpd_connection_clear_error(conn);
+        throw std::system_error(ec, std::system_category());
+      }
+      G_GNUC_FALLTHROUGH;
     default:
       if (conn) {
         auto error_message = mpd_connection_get_error_message(conn);

src/modules/mpd/state.cpp

@@ -119,7 +119,7 @@ bool Idle::on_io(Glib::IOCondition const&) {
 
 void Playing::entry() noexcept {
   sigc::slot<bool> timer_slot = sigc::mem_fun(*this, &Playing::on_timer);
-  timer_connection_ = Glib::signal_timeout().connect(timer_slot, /* milliseconds */ 1'000);
+  timer_connection_ = Glib::signal_timeout().connect_seconds(timer_slot, 1);
   spdlog::debug("mpd: Playing: enabled 1 second periodic timer.");
 }
 
@@ -327,14 +327,20 @@ void Stopped::pause() {
 
 void Stopped::update() noexcept { ctx_->do_update(); }
 
-void Disconnected::arm_timer(int interval) noexcept {
+bool Disconnected::arm_timer(int interval) noexcept {
+  // check if it's necessary to modify the timer
+  if (timer_connection_ && last_interval_ == interval) {
+    return true;
+  }
   // unregister timer, if present
   disarm_timer();
 
   // register timer
+  last_interval_ = interval;
   sigc::slot<bool> timer_slot = sigc::mem_fun(*this, &Disconnected::on_timer);
-  timer_connection_ = Glib::signal_timeout().connect(timer_slot, interval);
+  timer_connection_ = Glib::signal_timeout().connect_seconds(timer_slot, interval);
-  spdlog::debug("mpd: Disconnected: enabled interval timer.");
+  spdlog::debug("mpd: Disconnected: enabled {}s interval timer.", interval);
+  return false;
 }
 
 void Disconnected::disarm_timer() noexcept {
@@ -347,7 +353,7 @@ void Disconnected::disarm_timer() noexcept {
 
 void Disconnected::entry() noexcept {
   ctx_->emit();
-  arm_timer(1'000);
+  arm_timer(1 /* second */);
 }
 
 void Disconnected::exit() noexcept { disarm_timer(); }
@@ -376,9 +382,7 @@ bool Disconnected::on_timer() {
     spdlog::warn("mpd: Disconnected: error: {}", e.what());
   }
 
-  arm_timer(ctx_->interval() * 1'000);
+  return arm_timer(ctx_->interval());
-
-  return false;
 }
 
 void Disconnected::update() noexcept { ctx_->do_update(); }