[tracker-miners/sam/file-processed-signal: 4/10] tracker-extract: Allow extractors to return error messages

[Sam Thursfield <sthursfield src gnome org>]

commit 127787155047c4b5e1702b44afaa8bcad335d9f3
Author: Sam Thursfield <sam afuera me uk>
Date:   Sat Mar 21 20:28:45 2020 +0100

    tracker-extract: Allow extractors to return error messages
    
    Previously the extractor get_metadata() function implemented by
    extract modules could only return a boolean success value. Internal
    errors could only be reported on the console.
    
    Extract modules can now return a GError, and the message will be
    propagated to the GAsyncReadyCallback passed to tracker_extract_file().
    
    The error can also be sent over D-Bus via the FileProcessed signal.
    This allows tools like `tracker index` to report errors to the user.

 src/libtracker-extract/tracker-extract-info.c   | 43 ++++++++++++++++++++++--
 src/libtracker-extract/tracker-extract-info.h   |  3 ++
 src/libtracker-miner/tracker-decorator.c        |  4 +--
 src/tracker-extract/tracker-extract-decorator.c |  3 +-
 src/tracker-extract/tracker-extract.c           | 44 ++++++++++++++++++-------
 src/tracker-extract/tracker-extract.h           |  1 +
 6 files changed, 80 insertions(+), 18 deletions(-)
---
diff --git a/src/libtracker-extract/tracker-extract-info.c b/src/libtracker-extract/tracker-extract-info.c
index 46e5fb700..620baad37 100644
--- a/src/libtracker-extract/tracker-extract-info.c
+++ b/src/libtracker-extract/tracker-extract-info.c
@@ -40,6 +40,7 @@
 struct _TrackerExtractInfo
 {
        TrackerResource *resource;
+       GError *error;
 
        GFile *file;
        gchar *mimetype;
@@ -78,6 +79,7 @@ tracker_extract_info_new (GFile       *file,
        info->graph = g_strdup (graph);
 
        info->resource = NULL;
+       info->error = NULL;
 
        info->ref_count = 1;
 
@@ -123,8 +125,8 @@ tracker_extract_info_unref (TrackerExtractInfo *info)
                g_free (info->mimetype);
                g_free (info->graph);
 
-               if (info->resource)
-                       g_object_unref (info->resource);
+               g_clear_object (&info->resource);
+               g_clear_error (&info->error);
 
                g_slice_free (TrackerExtractInfo, info);
        }
@@ -195,6 +197,24 @@ tracker_extract_info_get_resource (TrackerExtractInfo *info)
        return info->resource;
 }
 
+/**
+ * tracker_extract_info_get_error:
+ * @info: a #TrackerExtractInfo
+ *
+ * If extraction was not possible, extract modules can report
+ * the problem as a GError. This returns the error if set,
+ * or %NULL.
+ *
+ * Returns: (transfer none): a #GError instance, or %NULL
+ *
+ * Since: 3.0
+ */
+GError *
+tracker_extract_info_get_error (TrackerExtractInfo *info)
+{
+       return info->error;
+}
+
 /**
  * tracker_extract_info_set_resource:
  * @info: a #TrackerExtractInfo
@@ -231,3 +251,22 @@ tracker_extract_info_set_resource (TrackerExtractInfo *info,
        g_object_ref (resource);
        info->resource = resource;
 }
+
+/**
+ * tracker_extract_info_set_error:
+ * @info: a #TrackerExtractInfo
+ * @error: a #GError
+ *
+ * Sets the #GError to this #TrackerExtractInfo. Use this if the extract
+ * module was unable to process the file.
+ *
+ * The error message will be published over D-Bus via the FileProcessed signal.
+ *
+ * Since: 3.0
+ **/
+void
+tracker_extract_info_set_error (TrackerExtractInfo *info,
+                                GError             *error)
+{
+       info->error = g_error_copy (error);
+}
diff --git a/src/libtracker-extract/tracker-extract-info.h b/src/libtracker-extract/tracker-extract-info.h
index 3751c6c94..3d78fc390 100644
--- a/src/libtracker-extract/tracker-extract-info.h
+++ b/src/libtracker-extract/tracker-extract-info.h
@@ -45,8 +45,11 @@ const gchar *         tracker_extract_info_get_mimetype           (TrackerExtrac
 const gchar *         tracker_extract_info_get_graph              (TrackerExtractInfo *info);
 
 TrackerResource *     tracker_extract_info_get_resource           (TrackerExtractInfo *info);
+GError          *     tracker_extract_info_get_error              (TrackerExtractInfo *info);
 void                  tracker_extract_info_set_resource           (TrackerExtractInfo *info,
                                                                    TrackerResource    *resource);
+void                  tracker_extract_info_set_error              (TrackerExtractInfo *info,
+                                                                   GError             *error);
 
 G_END_DECLS
 
diff --git a/src/libtracker-miner/tracker-decorator.c b/src/libtracker-miner/tracker-decorator.c
index 7494cd635..05e02bb0d 100644
--- a/src/libtracker-miner/tracker-decorator.c
+++ b/src/libtracker-miner/tracker-decorator.c
@@ -554,8 +554,8 @@ decorator_task_done (GObject      *object,
                if (error) {
                        tracker_miner_file_processed (TRACKER_MINER (object), file, FALSE, error->message);
 
-                       g_warning ("Task for '%s' finished with error: %s\n",
-                                  info->url, error->message);
+                       g_debug ("Task for '%s' finished with error: %s\n",
+                                info->url, error->message);
                        g_error_free (error);
                } else {
                        tracker_miner_file_processed (TRACKER_MINER (object), file, FALSE, "no SPARQL was 
generated for this item");
diff --git a/src/tracker-extract/tracker-extract-decorator.c b/src/tracker-extract/tracker-extract-decorator.c
index 039882ff3..9e6cd40c7 100644
--- a/src/tracker-extract/tracker-extract-decorator.c
+++ b/src/tracker-extract/tracker-extract-decorator.c
@@ -193,7 +193,6 @@ get_metadata_cb (TrackerExtract *extract,
 
        if (error) {
                g_message ("Extraction failed: %s\n", error ? error->message : "no error given");
-               g_clear_error (&error);
 
                mime_type = tracker_decorator_info_get_mimetype (data->decorator_info);
                graph = tracker_extract_module_manager_get_graph (mime_type);
@@ -205,7 +204,7 @@ get_metadata_cb (TrackerExtract *extract,
                                          graph,
                                          tracker_decorator_info_get_url (data->decorator_info));
 
-               tracker_decorator_info_complete (data->decorator_info, sparql);
+               tracker_decorator_info_complete_error (data->decorator_info, error, sparql);
        } else {
                resource = decorator_save_info (TRACKER_EXTRACT_DECORATOR (data->decorator),
                                                data->decorator_info, info);
diff --git a/src/tracker-extract/tracker-extract.c b/src/tracker-extract/tracker-extract.c
index 9df045e56..bb2b88a51 100644
--- a/src/tracker-extract/tracker-extract.c
+++ b/src/tracker-extract/tracker-extract.c
@@ -33,6 +33,7 @@
 #include <libtracker-miners-common/tracker-common.h>
 
 #include <libtracker-extract/tracker-extract.h>
+#include <libtracker-miner/tracker-miner.h>
 
 #include "tracker-extract.h"
 #include "tracker-main.h"
@@ -278,7 +279,8 @@ notify_task_finish (TrackerExtractTask *task,
 
 static gboolean
 get_file_metadata (TrackerExtractTask  *task,
-                   TrackerExtractInfo **info_out)
+                   TrackerExtractInfo **info_out,
+                   GError             **error)
 {
        TrackerExtractInfo *info;
        GFile *file;
@@ -294,6 +296,8 @@ get_file_metadata (TrackerExtractTask  *task,
                /* We know the mime */
                mime_used = g_strdup (task->mimetype);
        } else {
+               g_set_error (error, TRACKER_EXTRACT_ERROR, TRACKER_EXTRACT_ERROR_NO_MIMETYPE,
+                            "Could not detect the MIME type of %s", task->file);
                tracker_extract_info_unref (info);
                return FALSE;
        }
@@ -315,6 +319,14 @@ get_file_metadata (TrackerExtractTask  *task,
        }
 
        if (!task->success) {
+               GError *task_error;
+
+               task_error = tracker_extract_info_get_error (info);
+
+               if (task_error && error) {
+                       *error = g_error_copy (task_error);
+               }
+
                tracker_extract_info_unref (info);
                info = NULL;
        }
@@ -475,6 +487,8 @@ static gboolean
 get_metadata (TrackerExtractTask *task)
 {
        TrackerExtractInfo *info;
+       GError *error = NULL;
+       gboolean success;
 
 #ifdef THREAD_ENABLE_TRACE
        g_debug ("Thread:%p --> '%s': Collected metadata",
@@ -487,20 +501,25 @@ get_metadata (TrackerExtractTask *task)
                return FALSE;
        }
 
-       if (!filter_module (task->extract, task->module) &&
-           get_file_metadata (task, &info)) {
-               g_task_return_pointer (G_TASK (task->res), info,
-                                      (GDestroyNotify) tracker_extract_info_unref);
-               extract_task_free (task);
-       } else {
+       if (filter_module (task->extract, task->module)) {
                g_task_return_new_error (G_TASK (task->res),
                                         tracker_extract_error_quark (),
                                         TRACKER_EXTRACT_ERROR_NO_EXTRACTOR,
                                         "Could not get any metadata for uri:'%s' and mime:'%s'",
                                         task->file, task->mimetype);
-               extract_task_free (task);
+       } else {
+               success = get_file_metadata (task, &info, &error);
+
+               if (success) {
+                       g_task_return_pointer (G_TASK (task->res), info,
+                                          (GDestroyNotify) tracker_extract_info_unref);
+               } else {
+                       g_task_return_error (G_TASK (task->res), error);
+               }
        }
 
+       extract_task_free (task);
+
        return FALSE;
 }
 
@@ -560,6 +579,7 @@ dispatch_task_cb (TrackerExtractTask *task)
        }
 
        if (error) {
+               /* Report the failure to the caller. */
                g_task_return_error (G_TASK (task->res), error);
                extract_task_free (task);
 
@@ -675,7 +695,7 @@ tracker_extract_get_metadata_by_cmdline (TrackerExtract             *object,
                                                                  &task->func);
 
        if (!filter_module (object, task->module) &&
-           get_file_metadata (task, &info)) {
+           get_file_metadata (task, &info, &error)) {
                resource = tracker_extract_info_get_resource (info);
        }
 
@@ -723,9 +743,9 @@ tracker_extract_get_metadata_by_cmdline (TrackerExtract             *object,
                        }
                }
        } else {
-               g_printerr ("%s: %s\n",
-                        uri,
-                        _("No metadata or extractor modules found to handle this file"));
+               /* Extraction failed */
+               g_printerr ("%s: %s: %s\n",
+                           uri, g_module_name (task->module), error->message);
        }
 
        extract_task_free (task);
diff --git a/src/tracker-extract/tracker-extract.h b/src/tracker-extract/tracker-extract.h
index bd88c3f9d..24e342e3c 100644
--- a/src/tracker-extract/tracker-extract.h
+++ b/src/tracker-extract/tracker-extract.h
@@ -45,6 +45,7 @@ typedef enum {
        TRACKER_EXTRACT_ERROR_NO_MIMETYPE,
        TRACKER_EXTRACT_ERROR_NO_EXTRACTOR,
        TRACKER_EXTRACT_ERROR_IO_ERROR,
+       TRACKER_EXTRACT_ERROR_PARSE_ERROR,
 } TrackerExtractError;
 
 struct TrackerExtract {