Feat: improve mpd error handling and reporting #1028

docs/references/translating_status.md

@@ -1,12 +1,12 @@
-- es-AR: 10 missing phrases
-- es-ES: 689 missing phrases
-- es-VE: 594 missing phrases
-- fi-FI: 590 missing phrases
-- fr-FR: 11 missing phrases
-- it-IT: 11 missing phrases
-- ja-JP: 11 missing phrases
-- ko-KR: 10 missing phrases
-- nl-NL: 11 missing phrases
+- es-AR: 11 missing phrases
+- es-ES: 690 missing phrases
+- es-VE: 595 missing phrases
+- fi-FI: 591 missing phrases
+- fr-FR: 12 missing phrases
+- it-IT: 12 missing phrases
+- ja-JP: 12 missing phrases
+- ko-KR: 11 missing phrases
+- nl-NL: 12 missing phrases
 - pl-PL: 782 missing phrases
-- ru-RU: 11 missing phrases
-- zh-Hans: 10 missing phrases
+- ru-RU: 12 missing phrases
+- zh-Hans: 11 missing phrases

src/i18n/json/i18n.json

@@ -1,13 +1,13 @@
 {
     "default": {"desc":"Browser default", "missingPhrases": 0},
-    "de-DE": {"desc":"Deutsch (de-DE)", "missingPhrases": 0},
+    "de-DE": {"desc":"Deutsch (de-DE)", "missingPhrases": 1},
     "en-US": {"desc":"English (en-US)", "missingPhrases": 0},
-    "es-AR": {"desc":"Español (es-AR)", "missingPhrases": 10},
-    "fr-FR": {"desc":"Français (fr-FR)", "missingPhrases": 11},
-    "it-IT": {"desc":"Italiano (it-IT)", "missingPhrases": 11},
-    "ja-JP": {"desc":"日本語 (ja-JP)", "missingPhrases": 11},
-    "ko-KR": {"desc":"한국어 (ko-KR)", "missingPhrases": 10},
-    "nl-NL": {"desc":"Nederlands (nl-NL)", "missingPhrases": 11},
-    "ru-RU": {"desc":"Russian (ru-RU)", "missingPhrases": 11},
-    "zh-Hans": {"desc":"简体中文 (zh-Hans)", "missingPhrases": 10}
+    "es-AR": {"desc":"Español (es-AR)", "missingPhrases": 11},
+    "fr-FR": {"desc":"Français (fr-FR)", "missingPhrases": 12},
+    "it-IT": {"desc":"Italiano (it-IT)", "missingPhrases": 12},
+    "ja-JP": {"desc":"日本語 (ja-JP)", "missingPhrases": 12},
+    "ko-KR": {"desc":"한국어 (ko-KR)", "missingPhrases": 11},
+    "nl-NL": {"desc":"Nederlands (nl-NL)", "missingPhrases": 12},
+    "ru-RU": {"desc":"Russian (ru-RU)", "missingPhrases": 12},
+    "zh-Hans": {"desc":"简体中文 (zh-Hans)", "missingPhrases": 11}
 }

src/lib/jsonrpc.c

@@ -239,6 +239,25 @@ sds jsonrpc_respond_ok(sds buffer, enum mympd_cmd_ids cmd_id, long request_id, e
     return jsonrpc_respond_message_phrase(buffer, cmd_id, request_id, facility, JSONRPC_SEVERITY_INFO, "ok", 0);
 }
 
+/**
+ * Checks rc and responses with ok or the message
+ * @param buffer already allocated sds string for the jsonrpc response
+ * @param cmd_id enum mympd_cmd_ids
+ * @param request_id jsonrpc request id to respond
+ * @param rc return code to check
+ * @param facility one of enum jsonrpc_facilities
+ * @param severity one of enum jsonrpc_severities
+ * @param message the response message, if rc == false
+ * @return pointer to buffer
+ */
+sds jsonrpc_respond_with_message_or_ok(sds buffer, enum mympd_cmd_ids cmd_id, long request_id,
+        bool rc, enum jsonrpc_facilities facility, enum jsonrpc_severities severity, sds message)
+{
+    return rc == true
+        ? jsonrpc_respond_ok(buffer, cmd_id, request_id, JSONRPC_FACILITY_MPD)
+        : jsonrpc_respond_message(buffer, cmd_id, request_id, facility, severity, message);
+}
+
 /**
  * Creates a simple jsonrpc response with a custom message
  * @param buffer pointer to already allocated sds string

src/lib/jsonrpc.h

@@ -74,6 +74,8 @@ sds jsonrpc_notify_start(sds buffer, enum jsonrpc_events event);
 sds jsonrpc_respond_start(sds buffer, enum mympd_cmd_ids cmd_id, long request_id);
 sds jsonrpc_end(sds buffer);
 sds jsonrpc_respond_ok(sds buffer, enum mympd_cmd_ids cmd_id, long request_id, enum jsonrpc_facilities facility);
+sds jsonrpc_respond_with_message_or_ok(sds buffer, enum mympd_cmd_ids cmd_id, long request_id,
+        bool rc, enum jsonrpc_facilities facility, enum jsonrpc_severities severity, sds message);
 sds jsonrpc_respond_message(sds buffer, enum mympd_cmd_ids cmd_id, long request_id,
         enum jsonrpc_facilities facility, enum jsonrpc_severities severity, const char *message);
 sds jsonrpc_respond_message_phrase(sds buffer, enum mympd_cmd_ids cmd_id, long request_id,

src/lib/sticker_cache.c

@@ -429,7 +429,7 @@ static bool sticker_inc(struct t_cache *sticker_cache, struct t_partition_state
     MYMPD_LOG_INFO("Setting sticker: \"%s\" -> %s: %s", uri, sticker_str, value_str);
     bool rc = mpd_run_sticker_set(partition_state->conn, "song", uri, sticker_str, value_str);
     FREE_SDS(value_str);
-    return mympd_check_rc_error_and_recover(partition_state, rc, "mpd_run_sticker_set");
+    return mympd_check_rc_error_and_recover(partition_state, NULL, rc, "mpd_run_sticker_set");
 }
 
 /**
@@ -474,7 +474,7 @@ static bool sticker_set(struct t_cache *sticker_cache, struct t_partition_state
     MYMPD_LOG_INFO("Setting sticker: \"%s\" -> %s: %s", uri, sticker_str, value_str);
     bool rc = mpd_run_sticker_set(partition_state->conn, "song", uri, sticker_str, value_str);
     FREE_SDS(value_str);
-    return mympd_check_rc_error_and_recover(partition_state, rc, "mpd_run_sticker_set");
+    return mympd_check_rc_error_and_recover(partition_state, NULL, rc, "mpd_run_sticker_set");
 }
 
 /**

src/mpd_client/errorhandler.c

@@ -30,7 +30,7 @@ static bool check_rc_error_and_recover(struct t_partition_state *partition_state
         enum mympd_cmd_ids cmd_id, long request_id, enum response_types response_type, bool rc,
         const char *command);
 static bool check_error_and_recover(struct t_partition_state *partition_state, sds *buffer, enum mympd_cmd_ids cmd_id,
-        long request_id, enum response_types response_type);
+        long request_id, enum response_types response_type, const char *command);
 
 /**
  * Public functions
@@ -39,21 +39,24 @@ static bool check_error_and_recover(struct t_partition_state *partition_state, s
 /**
  * Checks for mpd protocol error and tries to recover it
  * @param partition_state pointer to partition specific states
+ * @param buffer pointer to an already allocated sds string for the error message
+ * @param command last mpd command
  * @return true on success else false
  */
-bool mympd_check_error_and_recover(struct t_partition_state *partition_state) {
-    return check_error_and_recover(partition_state, NULL, GENERAL_API_UNKNOWN, 0, RESPONSE_TYPE_NONE);
+bool mympd_check_error_and_recover(struct t_partition_state *partition_state, sds *buffer, const char *command) {
+    return check_error_and_recover(partition_state, buffer, GENERAL_API_UNKNOWN, 0, RESPONSE_TYPE_NONE, command);
 }
 
 /**
  * Checks for mpd protocol error and return code of last mpd command and tries to recover it
  * @param partition_state pointer to partition specific states
+ * @param buffer pointer to an already allocated sds string for the error message
  * @param rc return code of last mpd command to check
  * @param command last mpd command
  * @return true on success else false
  */
-bool mympd_check_rc_error_and_recover(struct t_partition_state *partition_state, bool rc, const char *command) {
-    return check_rc_error_and_recover(partition_state, NULL, GENERAL_API_UNKNOWN, 0, RESPONSE_TYPE_NONE, rc, command);
+bool mympd_check_rc_error_and_recover(struct t_partition_state *partition_state, sds *buffer, bool rc, const char *command) {
+    return check_rc_error_and_recover(partition_state, buffer, GENERAL_API_UNKNOWN, 0, RESPONSE_TYPE_NONE, rc, command);
 }
 
 /**
@@ -66,9 +69,9 @@ bool mympd_check_rc_error_and_recover(struct t_partition_state *partition_state,
  * @return true on success else false
  */
 bool mympd_check_error_and_recover_respond(struct t_partition_state *partition_state, sds *buffer,
-        enum mympd_cmd_ids cmd_id, long request_id)
+        enum mympd_cmd_ids cmd_id, long request_id, const char *command)
 {
-    return check_error_and_recover(partition_state, buffer, cmd_id, request_id, RESPONSE_TYPE_JSONRPC_RESPONSE);
+    return check_error_and_recover(partition_state, buffer, cmd_id, request_id, RESPONSE_TYPE_JSONRPC_RESPONSE, command);
 }
 
 /**
@@ -93,10 +96,11 @@ bool mympd_check_rc_error_and_recover_respond(struct t_partition_state *partitio
  * Creates a jsonrpc notification on error.
  * @param partition_state pointer to partition specific states
  * @param buffer already allocated sds string for the jsonrpc response
+ * @param command last mpd command
  * @return true on success else false
  */
-bool mympd_check_error_and_recover_notify(struct t_partition_state *partition_state, sds *buffer) {
-    return check_error_and_recover(partition_state, buffer, GENERAL_API_UNKNOWN, 0, RESPONSE_TYPE_JSONRPC_NOTIFY);
+bool mympd_check_error_and_recover_notify(struct t_partition_state *partition_state, sds *buffer, const char *command) {
+    return check_error_and_recover(partition_state, buffer, GENERAL_API_UNKNOWN, 0, RESPONSE_TYPE_JSONRPC_NOTIFY, command);
 }
 
 /**
@@ -119,10 +123,11 @@ bool mympd_check_rc_error_and_recover_notify(struct t_partition_state *partition
  * Returns the plain mpd error message.
  * @param partition_state pointer to partition specific states
  * @param buffer already allocated sds string for the mpd error message
+ * @param command last mpd command
  * @return true on success else false
  */
-bool mympd_check_error_and_recover_plain(struct t_partition_state *partition_state, sds *buffer) {
-    return check_error_and_recover(partition_state, buffer, GENERAL_API_UNKNOWN, 0, RESPONSE_TYPE_PLAIN);
+bool mympd_check_error_and_recover_plain(struct t_partition_state *partition_state, sds *buffer, const char *command) {
+    return check_error_and_recover(partition_state, buffer, GENERAL_API_UNKNOWN, 0, RESPONSE_TYPE_PLAIN, command);
 }
 
 /**
@@ -157,7 +162,7 @@ sds mympd_respond_with_error_or_ok(struct t_partition_state *partition_state, sd
         long request_id, bool rc, const char *command, bool *result)
 {
     sdsclear(buffer);
-    *result = check_rc_error_and_recover(partition_state, &buffer, cmd_id, request_id, false, rc, command);
+    *result = check_rc_error_and_recover(partition_state, &buffer, cmd_id, request_id, RESPONSE_TYPE_JSONRPC_RESPONSE, rc, command);
     if (*result == false) {
         return buffer;
     }
@@ -185,8 +190,7 @@ static bool check_rc_error_and_recover(struct t_partition_state *partition_state
         const char *command)
 {
     //first do normal mpd error checking
-    if (check_error_and_recover(partition_state, buffer, cmd_id, request_id, response_type) == false) {
-        MYMPD_LOG_ERROR("\"%s\": Error in response to command %s", partition_state->name, command);
+    if (check_error_and_recover(partition_state, buffer, cmd_id, request_id, response_type, command) == false) {
         return false;
     }
     //there is no mpd error at connection level but the command failed, return command error
@@ -220,25 +224,27 @@ static bool check_rc_error_and_recover(struct t_partition_state *partition_state
  * @param cmd_id enum mympd_cmd_ids
  * @param request_id jsonrpc request id to respond
  * @param response_type response message type
+ * @param command command to check for the error
  * @return true on success else false
  */
 static bool check_error_and_recover(struct t_partition_state *partition_state, sds *buffer, enum mympd_cmd_ids cmd_id,
-        long request_id, enum response_types response_type)
+        long request_id, enum response_types response_type, const char *command)
 {
     enum mpd_error error = mpd_connection_get_error(partition_state->conn);
     if (error != MPD_ERROR_SUCCESS) {
         const char *error_msg = mpd_connection_get_error_message(partition_state->conn);
-        MYMPD_LOG_ERROR("\"%s\": MPD error: %s (%d)", partition_state->name, error_msg , error);
+        MYMPD_LOG_ERROR("\"%s\": MPD error for command %s: %s (%d)", partition_state->name, command, error_msg , error);
         if (buffer != NULL &&
             *buffer != NULL)
         {
             switch(response_type) {
                 case RESPONSE_TYPE_JSONRPC_RESPONSE:
-                    *buffer = jsonrpc_respond_message(*buffer, cmd_id, request_id,
-                        JSONRPC_FACILITY_MPD, JSONRPC_SEVERITY_ERROR, error_msg);
+                    *buffer = jsonrpc_respond_message_phrase(*buffer, cmd_id, request_id,
+                        JSONRPC_FACILITY_MPD, JSONRPC_SEVERITY_ERROR, "MPD error for command %{cmd}: %{msg}", 4, "cmd", command, "msg", error_msg);
                     break;
                 case RESPONSE_TYPE_JSONRPC_NOTIFY:
-                    *buffer = jsonrpc_notify(*buffer, JSONRPC_FACILITY_MPD, JSONRPC_SEVERITY_ERROR, error_msg);
+                    *buffer = jsonrpc_notify_phrase(*buffer,
+                        JSONRPC_FACILITY_MPD, JSONRPC_SEVERITY_ERROR, "MPD error for command %{cmd}: %{msg}", 4, "cmd", command, "msg", error_msg);
                     break;
                 default:
                     *buffer = sdscat(*buffer, error_msg);

src/mpd_client/errorhandler.h

@@ -11,19 +11,19 @@
 #include "src/lib/api.h"
 #include "src/lib/mympd_state.h"
 
-bool mympd_check_error_and_recover(struct t_partition_state *partition_state);
-bool mympd_check_rc_error_and_recover(struct t_partition_state *partition_state, bool rc, const char *command);
+bool mympd_check_error_and_recover(struct t_partition_state *partition_state, sds *buffer, const char *command);
+bool mympd_check_rc_error_and_recover(struct t_partition_state *partition_state, sds *buffer, bool rc, const char *command);
 
 bool mympd_check_error_and_recover_respond(struct t_partition_state *partition_state, sds *buffer,
-        enum mympd_cmd_ids cmd_id, long request_id);
+        enum mympd_cmd_ids cmd_id, long request_id, const char *command);
 bool mympd_check_rc_error_and_recover_respond(struct t_partition_state *partition_state, sds *buffer,
         enum mympd_cmd_ids cmd_id, long request_id, bool rc, const char *command);
 
-bool mympd_check_error_and_recover_notify(struct t_partition_state *partition_state, sds *buffer);
+bool mympd_check_error_and_recover_notify(struct t_partition_state *partition_state, sds *buffer, const char *command);
 bool mympd_check_rc_error_and_recover_notify(struct t_partition_state *partition_state, sds *buffer, bool rc,
         const char *command);
 
-bool mympd_check_error_and_recover_plain(struct t_partition_state *partition_state, sds *buffer);
+bool mympd_check_error_and_recover_plain(struct t_partition_state *partition_state, sds *buffer, const char *command);
 bool mympd_check_rc_error_and_recover_plain(struct t_partition_state *partition_state, sds *buffer, bool rc,
         const char *command);
 

src/mpd_client/features.c

@@ -157,7 +157,7 @@ static void features_commands(struct t_partition_state *partition_state) {
         MYMPD_LOG_ERROR("Error in response to command: mpd_send_allowed_commands");
     }
     mpd_response_finish(partition_state->conn);
-    mympd_check_error_and_recover(partition_state);
+    mympd_check_error_and_recover(partition_state, NULL, "mpd_send_allowed_commands");
 }
 
 /**
@@ -220,7 +220,8 @@ static void features_mpd_tags(struct t_partition_state *partition_state) {
     enable_all_mpd_tags(partition_state);
 
     sds logline = sdsnew("MPD supported tags: ");
-    if (mpd_send_list_tag_types(partition_state->conn) == true) {
+    bool rc = mpd_send_list_tag_types(partition_state->conn);
+    if (rc == true) {
         struct mpd_pair *pair;
         while ((pair = mpd_recv_tag_type_pair(partition_state->conn)) != NULL) {
             enum mpd_tag_type tag = mpd_tag_name_parse(pair->value);
@@ -234,11 +235,8 @@ static void features_mpd_tags(struct t_partition_state *partition_state) {
             mpd_return_pair(partition_state->conn, pair);
         }
     }
-    else {
-        MYMPD_LOG_ERROR("Error in response to command: mpd_send_list_tag_types");
-    }
     mpd_response_finish(partition_state->conn);
-    mympd_check_error_and_recover(partition_state);
+    mympd_check_rc_error_and_recover(partition_state, NULL, rc, "mpd_send_list_tag_types");
 
     if (partition_state->mpd_state->tags_mpd.len == 0) {
         logline = sdscatlen(logline, "none", 4);
@@ -313,7 +311,7 @@ static void features_config(struct t_partition_state *partition_state) {
             }
         }
         mpd_response_finish(partition_state->conn);
-        mympd_check_rc_error_and_recover(partition_state, rc, "config");
+        mympd_check_rc_error_and_recover(partition_state, NULL, rc, "config");
     }
 
     partition_state->mpd_state->music_directory_value = set_directory("music", partition_state->mympd_state->music_directory,

src/mpd_client/idle.c

@@ -161,7 +161,7 @@ static void mpd_client_idle_partition(struct t_partition_state *partition_state,
                 //change partition
                 MYMPD_LOG_INFO("Switching to partition \"%s\"", partition_state->name);
                 bool rc = mpd_run_switch_partition(partition_state->conn, partition_state->name);
-                if (mympd_check_rc_error_and_recover(partition_state, rc, "mpd_run_switch_partition") == false) {
+                if (mympd_check_rc_error_and_recover(partition_state, NULL, rc, "mpd_run_switch_partition") == false) {
                     MYMPD_LOG_ERROR("Could not switch to partition \"%s\"", partition_state->name);
                     mpd_client_disconnect(partition_state, MPD_FAILURE);
                     break;
@@ -251,7 +251,7 @@ static void mpd_client_idle_partition(struct t_partition_state *partition_state,
             {
                 MYMPD_LOG_DEBUG("\"%s\": Leaving mpd idle mode", partition_state->name);
                 if (mpd_send_noidle(partition_state->conn) == false) {
-                    mympd_check_error_and_recover(partition_state);
+                    mympd_check_rc_error_and_recover(partition_state, NULL, false, "mpd_send_noidle");
                     partition_state->conn_state = MPD_FAILURE;
                     break;
                 }
@@ -313,7 +313,7 @@ static void mpd_client_idle_partition(struct t_partition_state *partition_state,
                 if (partition_state->conn_state == MPD_CONNECTED) {
                     MYMPD_LOG_DEBUG("\"%s\": Entering mpd idle mode", partition_state->name);
                     if (mpd_send_idle_mask(partition_state->conn, partition_state->idle_mask) == false) {
-                        mympd_check_error_and_recover(partition_state);
+                        mympd_check_rc_error_and_recover(partition_state, NULL, false, "mpd_send_idle_mask");
                         partition_state->conn_state = MPD_FAILURE;
                     }
                 }
@@ -382,7 +382,7 @@ static void mpd_client_parse_idle(struct t_partition_state *partition_state, uns
                         if (partition_state->play_state != MPD_STATE_PLAY) {
                             MYMPD_LOG_INFO("\"%s\": AutoPlay enabled, start playing", partition_state->name);
                             if (mpd_run_play(partition_state->conn) == false) {
-                                mympd_check_error_and_recover(partition_state);
+                                mympd_check_rc_error_and_recover(partition_state, NULL, false, "mpd_run_play");
                             }
                         }
                         else {