[gnome-software] plugin loader: Extend error message scraping to app IDs



commit 9270ddb717f510b592ceaeeab8088cfcb9968f0b
Author: Kalev Lember <klember redhat com>
Date:   Wed Nov 28 12:59:58 2018 +0100

    plugin loader: Extend error message scraping to app IDs
    
    We were already adding origin IDs to error messages; this extends the
    same mechanism to allow adding app IDs as well.
    
    The motivation for doing so is to be able to change the flatpak plugin
    from updating one app at a time to doing all apps in one transaction,
    but when we do that then the plugin loader no longer knows how to map
    errors to apps. This adds the mechanism for that.
    
    It's all pretty hacky, but not more so than the existing origin ID
    passing.
    
    This also improves things somewhat and removes the need for the UI code
    to know and handle unique ID prefixes in the error messages. Instead,
    the plugin loader now does all the work and strips any unique ID
    prefixes.

 lib/gs-plugin-loader.c                             |  42 +++++---
 lib/gs-self-test.c                                 |  28 ++++--
 lib/gs-utils.c                                     | 106 +++++++++++++++++----
 lib/gs-utils.h                                     |   7 +-
 plugins/dummy/gs-plugin-dummy.c                    |   2 +-
 .../gs-plugin-fedora-pkgdb-collections.c           |   2 +-
 plugins/flatpak/gs-flatpak.c                       |   2 +-
 plugins/fwupd/gs-plugin-fwupd.c                    |   4 +-
 plugins/odrs/gs-plugin-odrs.c                      |   4 +-
 plugins/packagekit/gs-plugin-packagekit-local.c    |   2 +-
 plugins/packagekit/gs-plugin-packagekit.c          |   6 +-
 .../shell-extensions/gs-plugin-shell-extensions.c  |   2 +-
 src/gs-updates-page.c                              |   2 -
 13 files changed, 152 insertions(+), 57 deletions(-)
---
diff --git a/lib/gs-plugin-loader.c b/lib/gs-plugin-loader.c
index 5472fd55..afd36833 100644
--- a/lib/gs-plugin-loader.c
+++ b/lib/gs-plugin-loader.c
@@ -398,8 +398,8 @@ gs_plugin_error_handle_failure (GsPluginLoaderHelper *helper,
                                const GError *error_local,
                                GError **error)
 {
-       g_auto(GStrv) split = NULL;
-       g_autoptr(GsApp) origin = NULL;
+       g_autofree gchar *app_id = NULL;
+       g_autofree gchar *origin_id = NULL;
        g_autoptr(GsPluginEvent) event = NULL;
 
        /* badly behaved plugin */
@@ -416,6 +416,14 @@ gs_plugin_error_handle_failure (GsPluginLoaderHelper *helper,
                return TRUE;
        }
 
+       /* find and strip any unique IDs from the error message */
+       for (guint i = 0; i < 2; i++) {
+               if (app_id == NULL)
+                       app_id = gs_utils_error_strip_app_id (error_local);
+               if (origin_id == NULL)
+                       origin_id = gs_utils_error_strip_origin_id (error_local);
+       }
+
        /* fatal error */
        if (gs_plugin_job_get_action (helper->plugin_job) == GS_PLUGIN_ACTION_SETUP ||
            gs_plugin_loader_is_error_fatal (error_local) ||
@@ -428,19 +436,23 @@ gs_plugin_error_handle_failure (GsPluginLoaderHelper *helper,
        /* create event which is handled by the GsShell */
        event = gs_plugin_job_to_failed_event (helper->plugin_job, error_local);
 
-       /* can we find a unique ID */
-       split = g_strsplit_set (error_local->message, "[]: ", -1);
-       for (guint i = 0; split[i] != NULL; i++) {
-               if (as_utils_unique_id_valid (split[i])) {
-                       origin = gs_plugin_cache_lookup (plugin, split[i]);
-                       if (origin != NULL) {
-                               g_debug ("found origin %s in error",
-                                        gs_app_get_unique_id (origin));
-                               gs_plugin_event_set_origin (event, origin);
-                               break;
-                       } else {
-                               g_debug ("no unique ID found for %s", split[i]);
-                       }
+       /* set the app and origin IDs if we managed to scrape them from the error above */
+       if (as_utils_unique_id_valid (app_id)) {
+               g_autoptr(GsApp) app = gs_plugin_cache_lookup (plugin, app_id);
+               if (app != NULL) {
+                       g_debug ("found app %s in error", origin_id);
+                       gs_plugin_event_set_app (event, app);
+               } else {
+                       g_debug ("no unique ID found for app %s", app_id);
+               }
+       }
+       if (as_utils_unique_id_valid (origin_id)) {
+               g_autoptr(GsApp) origin = gs_plugin_cache_lookup (plugin, origin_id);
+               if (origin != NULL) {
+                       g_debug ("found origin %s in error", origin_id);
+                       gs_plugin_event_set_origin (event, origin);
+               } else {
+                       g_debug ("no unique ID found for origin %s", origin_id);
                }
        }
 
diff --git a/lib/gs-self-test.c b/lib/gs-self-test.c
index bccec1ef..b5ba3736 100644
--- a/lib/gs-self-test.c
+++ b/lib/gs-self-test.c
@@ -144,22 +144,38 @@ gs_utils_cache_func (void)
 static void
 gs_utils_error_func (void)
 {
-       guint i;
+       g_autofree gchar *app_id = NULL;
+       g_autofree gchar *origin_id = NULL;
        g_autoptr(GError) error = NULL;
        g_autoptr(GsApp) app = gs_app_new ("gimp.desktop");
+       g_autoptr(GsApp) origin = gs_app_new ("gimp-repo");
 
-       for (i = 0; i < GS_PLUGIN_ERROR_LAST; i++)
+       for (guint i = 0; i < GS_PLUGIN_ERROR_LAST; i++)
                g_assert (gs_plugin_error_to_string (i) != NULL);
 
-       gs_utils_error_add_unique_id (&error, app);
+       /* noop */
+       gs_utils_error_add_app_id (&error, app);
+       gs_utils_error_add_origin_id (&error, origin);
+
        g_set_error (&error,
                     GS_PLUGIN_ERROR,
                     GS_PLUGIN_ERROR_DOWNLOAD_FAILED,
                     "failed");
        g_assert_cmpstr (error->message, ==, "failed");
-       gs_utils_error_add_unique_id (&error, app);
-       g_assert_cmpstr (error->message, ==, "[*/*/*/*/gimp.desktop/*] failed");
-       gs_utils_error_strip_unique_id (error);
+       gs_utils_error_add_app_id (&error, app);
+       gs_utils_error_add_origin_id (&error, origin);
+       g_assert_cmpstr (error->message, ==, "[*/*/*/*/gimp-repo/*] {*/*/*/*/gimp.desktop/*} failed");
+
+       /* find and strip any unique IDs from the error message */
+       for (guint i = 0; i < 2; i++) {
+               if (app_id == NULL)
+                       app_id = gs_utils_error_strip_app_id (error);
+               if (origin_id == NULL)
+                       origin_id = gs_utils_error_strip_origin_id (error);
+       }
+
+       g_assert_cmpstr (app_id, ==, "*/*/*/*/gimp.desktop/*");
+       g_assert_cmpstr (origin_id, ==, "*/*/*/*/gimp-repo/*");
        g_assert_cmpstr (error->message, ==, "failed");
 }
 
diff --git a/lib/gs-utils.c b/lib/gs-utils.c
index dbb7c1cc..6e206caf 100644
--- a/lib/gs-utils.c
+++ b/lib/gs-utils.c
@@ -592,47 +592,113 @@ gs_utils_get_wilson_rating (guint64 star1,
 }
 
 /**
- * gs_utils_error_add_unique_id:
+ * gs_utils_error_add_app_id:
  * @error: a #GError
  * @app: a #GsApp
  *
- * Adds a unique ID prefix to the error.
+ * Adds app unique ID prefix to the error.
  *
- * Since: 3.22
+ * Since: 3.30
  **/
 void
-gs_utils_error_add_unique_id (GError **error, GsApp *app)
+gs_utils_error_add_app_id (GError **error, GsApp *app)
 {
        g_return_if_fail (GS_APP (app));
        if (error == NULL || *error == NULL)
                return;
-       g_prefix_error (error, "[%s] ", gs_app_get_unique_id (app));
+       g_prefix_error (error, "{%s} ", gs_app_get_unique_id (app));
 }
 
 /**
- * gs_utils_error_strip_unique_id:
+ * gs_utils_error_add_origin_id:
  * @error: a #GError
+ * @origin: a #GsApp
  *
- * Removes a possible unique ID prefix from the error.
+ * Adds origin unique ID prefix to the error.
  *
- * Since: 3.22
+ * Since: 3.30
  **/
 void
-gs_utils_error_strip_unique_id (GError *error)
+gs_utils_error_add_origin_id (GError **error, GsApp *origin)
 {
-       gchar *str;
-       if (error == NULL)
-               return;
-       if (!g_str_has_prefix (error->message, "["))
-               return;
-       str = g_strstr_len (error->message, -1, " ");
-       if (str == NULL)
+       g_return_if_fail (GS_APP (origin));
+       if (error == NULL || *error == NULL)
                return;
+       g_prefix_error (error, "[%s] ", gs_app_get_unique_id (origin));
+}
+
+/**
+ * gs_utils_error_strip_app_id:
+ * @error: a #GError
+ *
+ * Removes a possible app ID prefix from the error, and returns the removed
+ * app ID.
+ *
+ * Returns: A newly allocated string with the app ID
+ *
+ * Since: 3.30
+ **/
+gchar *
+gs_utils_error_strip_app_id (GError *error)
+{
+       g_autofree gchar *app_id = NULL;
+       g_autofree gchar *msg = NULL;
+
+       if (error == NULL || error->message == NULL)
+               return FALSE;
+
+       if (g_str_has_prefix (error->message, "{")) {
+               const gchar *endp = strstr (error->message + 1, "} ");
+               if (endp != NULL) {
+                       app_id = g_strndup (error->message + 1,
+                                           endp - (error->message + 1));
+                       msg = g_strdup (endp + 2);
+               }
+       }
+
+       if (msg != NULL) {
+               g_free (error->message);
+               error->message = g_steal_pointer (&msg);
+       }
+
+       return g_steal_pointer (&app_id);
+}
+
+/**
+ * gs_utils_error_strip_origin_id:
+ * @error: a #GError
+ *
+ * Removes a possible origin ID prefix from the error, and returns the removed
+ * origin ID.
+ *
+ * Returns: A newly allocated string with the origin ID
+ *
+ * Since: 3.30
+ **/
+gchar *
+gs_utils_error_strip_origin_id (GError *error)
+{
+       g_autofree gchar *origin_id = NULL;
+       g_autofree gchar *msg = NULL;
+
+       if (error == NULL || error->message == NULL)
+               return FALSE;
+
+       if (g_str_has_prefix (error->message, "[")) {
+               const gchar *endp = strstr (error->message + 1, "] ");
+               if (endp != NULL) {
+                       origin_id = g_strndup (error->message + 1,
+                                              endp - (error->message + 1));
+                       msg = g_strdup (endp + 2);
+               }
+       }
+
+       if (msg != NULL) {
+               g_free (error->message);
+               error->message = g_steal_pointer (&msg);
+       }
 
-       /* gahh, my eyes are bleeding */
-       str = g_strdup (str + 1);
-       g_free (error->message);
-       error->message = str;
+       return g_steal_pointer (&origin_id);
 }
 
 /**
diff --git a/lib/gs-utils.h b/lib/gs-utils.h
index 6fb98ff1..792a815e 100644
--- a/lib/gs-utils.h
+++ b/lib/gs-utils.h
@@ -77,9 +77,12 @@ gint          gs_utils_get_wilson_rating     (guint64         star1,
                                                 guint64         star3,
                                                 guint64         star4,
                                                 guint64         star5);
-void            gs_utils_error_add_unique_id   (GError         **error,
+void            gs_utils_error_add_app_id      (GError         **error,
                                                 GsApp          *app);
-void            gs_utils_error_strip_unique_id (GError         *error);
+void            gs_utils_error_add_origin_id   (GError         **error,
+                                                GsApp          *origin);
+gchar          *gs_utils_error_strip_app_id    (GError         *error);
+gchar          *gs_utils_error_strip_origin_id (GError         *error);
 gboolean        gs_utils_error_convert_gio     (GError         **perror);
 gboolean        gs_utils_error_convert_gresolver (GError       **perror);
 gboolean        gs_utils_error_convert_gdbus   (GError         **perror);
diff --git a/plugins/dummy/gs-plugin-dummy.c b/plugins/dummy/gs-plugin-dummy.c
index dd657027..cb6c6ce1 100644
--- a/plugins/dummy/gs-plugin-dummy.c
+++ b/plugins/dummy/gs-plugin-dummy.c
@@ -602,7 +602,7 @@ gs_plugin_update_app (GsPlugin *plugin,
                                     GS_PLUGIN_ERROR,
                                     GS_PLUGIN_ERROR_DOWNLOAD_FAILED,
                                     "no network connection is available");
-               gs_utils_error_add_unique_id (error, priv->cached_origin);
+               gs_utils_error_add_origin_id (error, priv->cached_origin);
                return FALSE;
        }
 
diff --git a/plugins/fedora-pkgdb-collections/gs-plugin-fedora-pkgdb-collections.c 
b/plugins/fedora-pkgdb-collections/gs-plugin-fedora-pkgdb-collections.c
index c862d542..db84b9ff 100644
--- a/plugins/fedora-pkgdb-collections/gs-plugin-fedora-pkgdb-collections.c
+++ b/plugins/fedora-pkgdb-collections/gs-plugin-fedora-pkgdb-collections.c
@@ -209,7 +209,7 @@ _refresh_cache (GsPlugin *plugin,
                                      priv->cachefn,
                                      cancellable,
                                      error)) {
-               gs_utils_error_add_unique_id (error, priv->cached_origin);
+               gs_utils_error_add_origin_id (error, priv->cached_origin);
                return FALSE;
        }
 
diff --git a/plugins/flatpak/gs-flatpak.c b/plugins/flatpak/gs-flatpak.c
index 7830d3d1..5d94d09b 100644
--- a/plugins/flatpak/gs-flatpak.c
+++ b/plugins/flatpak/gs-flatpak.c
@@ -2172,7 +2172,7 @@ gs_flatpak_launch (GsFlatpak *self,
                                             GS_PLUGIN_ERROR,
                                             GS_PLUGIN_ERROR_NOT_SUPPORTED,
                                             "runtime is not installed");
-                       gs_utils_error_add_unique_id (error, runtime);
+                       gs_utils_error_add_origin_id (error, runtime);
                        gs_plugin_cache_add (self->plugin, NULL, runtime);
                        return FALSE;
                }
diff --git a/plugins/fwupd/gs-plugin-fwupd.c b/plugins/fwupd/gs-plugin-fwupd.c
index 8c5cfeec..b9d2af06 100644
--- a/plugins/fwupd/gs-plugin-fwupd.c
+++ b/plugins/fwupd/gs-plugin-fwupd.c
@@ -673,7 +673,7 @@ gs_plugin_fwupd_refresh_remote (GsPlugin *plugin,
                                    _("Downloading firmware update signature…"));
        data = gs_plugin_download_data (plugin, app_dl, url_sig, cancellable, error);
        if (data == NULL) {
-               gs_utils_error_add_unique_id (error, priv->cached_origin);
+               gs_utils_error_add_origin_id (error, priv->cached_origin);
                return FALSE;
        }
 
@@ -715,7 +715,7 @@ gs_plugin_fwupd_refresh_remote (GsPlugin *plugin,
        url = fwupd_remote_get_metadata_uri (remote);
        if (!gs_plugin_download_file (plugin, app_dl, url, filename,
                                      cancellable, error)) {
-               gs_utils_error_add_unique_id (error, priv->cached_origin);
+               gs_utils_error_add_origin_id (error, priv->cached_origin);
                return FALSE;
        }
 
diff --git a/plugins/odrs/gs-plugin-odrs.c b/plugins/odrs/gs-plugin-odrs.c
index 10e4cead..4031264d 100644
--- a/plugins/odrs/gs-plugin-odrs.c
+++ b/plugins/odrs/gs-plugin-odrs.c
@@ -650,7 +650,7 @@ gs_plugin_odrs_fetch_for_app (GsPlugin *plugin, GsApp *app, GError **error)
                                     GS_PLUGIN_ERROR,
                                     GS_PLUGIN_ERROR_DOWNLOAD_FAILED,
                                     "status code invalid");
-               gs_utils_error_add_unique_id (error, priv->cached_origin);
+               gs_utils_error_add_origin_id (error, priv->cached_origin);
                return NULL;
        }
        reviews = gs_plugin_odrs_parse_reviews (plugin,
@@ -1027,7 +1027,7 @@ gs_plugin_add_unvoted_reviews (GsPlugin *plugin,
                                     GS_PLUGIN_ERROR,
                                     GS_PLUGIN_ERROR_DOWNLOAD_FAILED,
                                     "status code invalid");
-               gs_utils_error_add_unique_id (error, priv->cached_origin);
+               gs_utils_error_add_origin_id (error, priv->cached_origin);
                return FALSE;
        }
        g_debug ("odrs returned: %s", msg->response_body->data);
diff --git a/plugins/packagekit/gs-plugin-packagekit-local.c b/plugins/packagekit/gs-plugin-packagekit-local.c
index f135289e..e82aced0 100644
--- a/plugins/packagekit/gs-plugin-packagekit-local.c
+++ b/plugins/packagekit/gs-plugin-packagekit-local.c
@@ -75,7 +75,7 @@ gs_plugin_packagekit_refresh_guess_app_id (GsPlugin *plugin,
                                             gs_packagekit_helper_cb, helper,
                                             error);
        if (!gs_plugin_packagekit_results_valid (results, error)) {
-               gs_utils_error_add_unique_id (error, app);
+               gs_utils_error_add_origin_id (error, app);
                return FALSE;
        }
        array = pk_results_get_files_array (results);
diff --git a/plugins/packagekit/gs-plugin-packagekit.c b/plugins/packagekit/gs-plugin-packagekit.c
index 096fbd0d..79d0aa56 100644
--- a/plugins/packagekit/gs-plugin-packagekit.c
+++ b/plugins/packagekit/gs-plugin-packagekit.c
@@ -197,7 +197,7 @@ gs_plugin_app_origin_repo_enable (GsPlugin *plugin,
                                         gs_packagekit_helper_cb, helper,
                                         error);
        if (!gs_plugin_packagekit_results_valid (results, error)) {
-               gs_utils_error_add_unique_id (error, app);
+               gs_utils_error_add_origin_id (error, app);
                return FALSE;
        }
 
@@ -230,7 +230,7 @@ gs_plugin_repo_enable (GsPlugin *plugin,
                                         error);
        if (!gs_plugin_packagekit_results_valid (results, error)) {
                gs_app_set_state_recover (app);
-               gs_utils_error_add_unique_id (error, app);
+               gs_utils_error_add_origin_id (error, app);
                return FALSE;
        }
 
@@ -453,7 +453,7 @@ gs_plugin_repo_disable (GsPlugin *plugin,
                                         error);
        if (!gs_plugin_packagekit_results_valid (results, error)) {
                gs_app_set_state_recover (app);
-               gs_utils_error_add_unique_id (error, app);
+               gs_utils_error_add_origin_id (error, app);
                return FALSE;
        }
 
diff --git a/plugins/shell-extensions/gs-plugin-shell-extensions.c 
b/plugins/shell-extensions/gs-plugin-shell-extensions.c
index 3d64eb51..9068e6e5 100644
--- a/plugins/shell-extensions/gs-plugin-shell-extensions.c
+++ b/plugins/shell-extensions/gs-plugin-shell-extensions.c
@@ -792,7 +792,7 @@ gs_plugin_shell_extensions_refresh (GsPlugin *plugin,
                                    /* TRANSLATORS: status text when downloading */
                                    _("Downloading shell extension metadata…"));
        if (!gs_plugin_download_file (plugin, app_dl, uri, fn, cancellable, error)) {
-               gs_utils_error_add_unique_id (error, priv->cached_origin);
+               gs_utils_error_add_origin_id (error, priv->cached_origin);
                return FALSE;
        }
 
diff --git a/src/gs-updates-page.c b/src/gs-updates-page.c
index 66abaecb..72c1c067 100644
--- a/src/gs-updates-page.c
+++ b/src/gs-updates-page.c
@@ -527,7 +527,6 @@ gs_updates_page_get_updates_cb (GsPluginLoader *plugin_loader,
                gs_updates_page_clear_flag (self, GS_UPDATES_PAGE_FLAG_HAS_UPDATES);
                if (!g_error_matches (error, GS_PLUGIN_ERROR, GS_PLUGIN_ERROR_CANCELLED))
                        g_warning ("updates-shell: failed to get updates: %s", error->message);
-               gs_utils_error_strip_unique_id (error);
                gtk_label_set_label (GTK_LABEL (self->label_updates_failed),
                                     error->message);
                gs_updates_page_set_state (self, GS_UPDATES_PAGE_STATE_FAILED);
@@ -780,7 +779,6 @@ gs_updates_page_refresh_cb (GsPluginLoader *plugin_loader,
                        return;
                }
                g_warning ("failed to refresh: %s", error->message);
-               gs_utils_error_strip_unique_id (error);
                gtk_label_set_label (GTK_LABEL (self->label_updates_failed),
                                     error->message);
                gs_updates_page_set_state (self, GS_UPDATES_PAGE_STATE_FAILED);


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]