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



commit 12db6dc5f5b76c95286ba6cb96cb85338e4ab7fc
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 93e39490..b811d66d 100644
--- a/lib/gs-plugin-loader.c
+++ b/lib/gs-plugin-loader.c
@@ -429,8 +429,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 */
@@ -447,6 +447,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) ||
@@ -459,19 +467,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 868d5a50..e8f52307 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 23d24995..f95e7fba 100644
--- a/lib/gs-utils.c
+++ b/lib/gs-utils.c
@@ -586,47 +586,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 6f739ff7..623092c7 100644
--- a/plugins/dummy/gs-plugin-dummy.c
+++ b/plugins/dummy/gs-plugin-dummy.c
@@ -588,7 +588,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 292d9253..1660655e 100644
--- a/plugins/fedora-pkgdb-collections/gs-plugin-fedora-pkgdb-collections.c
+++ b/plugins/fedora-pkgdb-collections/gs-plugin-fedora-pkgdb-collections.c
@@ -220,7 +220,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 ffd8927a..0afcaaa4 100644
--- a/plugins/flatpak/gs-flatpak.c
+++ b/plugins/flatpak/gs-flatpak.c
@@ -2056,7 +2056,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 e7b86778..8c685792 100644
--- a/plugins/fwupd/gs-plugin-fwupd.c
+++ b/plugins/fwupd/gs-plugin-fwupd.c
@@ -672,7 +672,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;
        }
 
@@ -714,7 +714,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 c6fc479b..3d03364e 100644
--- a/plugins/odrs/gs-plugin-odrs.c
+++ b/plugins/odrs/gs-plugin-odrs.c
@@ -653,7 +653,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,
@@ -1031,7 +1031,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 50359b30..bf34ac7f 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 8d7eb551..fb240fdc 100644
--- a/plugins/packagekit/gs-plugin-packagekit.c
+++ b/plugins/packagekit/gs-plugin-packagekit.c
@@ -196,7 +196,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;
        }
 
@@ -229,7 +229,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;
        }
 
@@ -452,7 +452,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 582b7aaf..a8a283a8 100644
--- a/plugins/shell-extensions/gs-plugin-shell-extensions.c
+++ b/plugins/shell-extensions/gs-plugin-shell-extensions.c
@@ -710,7 +710,7 @@ gs_plugin_shell_extensions_get_apps (GsPlugin *plugin,
                                    _("Downloading shell extension metadata…"));
        data = gs_plugin_download_data (plugin, app_dl, uri, 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 NULL;
        }
        apps = gs_plugin_shell_extensions_parse_apps (plugin,
diff --git a/src/gs-updates-page.c b/src/gs-updates-page.c
index f19f7dfb..947c187b 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);
@@ -778,7 +777,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]