Python bindings and Xapian exceptions

[Matt]

For now, as a workaround, I open the db, do my request, close it and
repeat the process 5/10 sec later. I hope it does not add to much
overhead ?! Anyway, it owuld be nice to have this fixed in one way or
another.

Thanks for your answer David.
Best regards

2014-12-16 9:02 GMT+01:00 David Bremner <david at tethera.net>:
> Matt <mattator at gmail.com> writes:
>
>> 2014-12-15 21:41 GMT+01:00 David Bremner <david at tethera.net>:
>>> Matt <mattator at gmail.com> writes:
>>>
>>>>> > But for exceptions in general, yes the notmuch library does need to be
>>>>> > fixed to allow the caller of functions to distinguish between things
>>>>> > like "no matches found" and "an exception occurred, so it's unknown if
>>>>> > any messages match the search". That's a general class of library
>>>>> > interface bugs that all need to be fixed.
>>>>
>>>> I 've also hit this *API bug* and was wondering if a fix had been done since
>>>> then (I use notmuch 0.17) ? I found nothing on http://notmuchmail.org/news/
>>>
>>> Can you be more specific? I'd say in general no thorough overhaul of
>>> error handling has happened, but if you can tell us what particular
>>> libnotmuch function (or the equivalient python binding) you are having
>>> trouble with, we may be able to give a more informative answer.
>>>
>>
>> For instance when using the python bindings:
>> In constructor I do
>> self.db = notmuch.Database(self.db_path)
>> and there I have a method called periodically that returns:
>> returns notmuch.Query(self.db, "tag:unread and tag:inbox").count_messages()
>>
>> When it fails the previous method returns 0 and displays on stdout/stderr;
>> "A Xapian exception occurred: The revision being read has been
>> discarded - you should call Xapian::Database::reopen() and retry the
>> operation
>> Query string was: tag:unread and tag:inbox"
>
> Right, this seems to be a particularly heinous example.
>
> Any objections (or better ideas) from fellow developers to something
> along the lines of the following? It isn't a huge improvement, and I
> didn't update the other 3 places it's called (or the bindings), but it
> seems like a step forward.  I guess something similar should be done for
> notmuch_query_count_threads.
>
> Alternatively, we could follow unix tradition and return -1 on error.
> The only argument I can see either way at the moment is that fewer error
> return styles is better than more.
>
> diff --git a/lib/notmuch.h b/lib/notmuch.h
> index 220839b..06228bc 100644
> --- a/lib/notmuch.h
> +++ b/lib/notmuch.h
> @@ -901,8 +901,8 @@ notmuch_threads_destroy (notmuch_threads_t *threads);
>   * If a Xapian exception occurs, this function may return 0 (after
>   * printing a message).
>   */
> -unsigned
> -notmuch_query_count_messages (notmuch_query_t *query);
> +notmuch_status_t
> +notmuch_query_count_messages (notmuch_query_t *query, unsigned *count);
>
>  /**
>   * Return the number of threads matching a search.
> diff --git a/lib/query.cc b/lib/query.cc
> index 60ff8bd..a623ea8 100644
> --- a/lib/query.cc
> +++ b/lib/query.cc
> @@ -508,8 +508,8 @@ notmuch_threads_destroy (notmuch_threads_t *threads)
>      talloc_free (threads);
>  }
>
> -unsigned
> -notmuch_query_count_messages (notmuch_query_t *query)
> +notmuch_status_t
> +notmuch_query_count_messages (notmuch_query_t *query, unsigned *count_out)
>  {
>      notmuch_database_t *notmuch = query->notmuch;
>      const char *query_string = query->query_string;
> @@ -562,12 +562,11 @@ notmuch_query_count_messages (notmuch_query_t *query)
>         count = mset.get_matches_estimated();
>
>      } catch (const Xapian::Error &error) {
> -       fprintf (stderr, "A Xapian exception occurred: %s\n",
> -                error.get_msg().c_str());
> -       fprintf (stderr, "Query string was: %s\n", query->query_string);
> +       return NOTMUCH_STATUS_XAPIAN_EXCEPTION;
>      }
> -
> -    return count;
> +
> +    *count_out=count;
> +    return NOTMUCH_STATUS_SUCCESS;
>  }
>
>  unsigned
> diff --git a/notmuch-count.c b/notmuch-count.c
> index 6058f7c..db43959 100644
> --- a/notmuch-count.c
> +++ b/notmuch-count.c
> @@ -71,6 +71,7 @@ print_count (notmuch_database_t *notmuch, const char *query_str,
>  {
>      notmuch_query_t *query;
>      size_t i;
> +    unsigned count;
>
>      query = notmuch_query_create (notmuch, query_str);
>      if (query == NULL) {
> @@ -83,7 +84,9 @@ print_count (notmuch_database_t *notmuch, const char *query_str,
>
>      switch (output) {
>      case OUTPUT_MESSAGES:
> -       printf ("%u\n", notmuch_query_count_messages (query));
> +       if (notmuch_query_count_messages (query, &count))
> +           return 1;
> +       printf ("%u\n", count);
>         break;
>      case OUTPUT_THREADS:
>         printf ("%u\n", notmuch_query_count_threads (query));
> diff --git a/notmuch-reply.c b/notmuch-reply.c
> index 7c1c809..5b7c0e1 100644
> --- a/notmuch-reply.c
> +++ b/notmuch-reply.c
> @@ -650,8 +650,13 @@ notmuch_reply_format_sprinter(void *ctx,
>      notmuch_messages_t *messages;
>      notmuch_message_t *message;
>      mime_node_t *node;
> +    unsigned count;
>
> -    if (notmuch_query_count_messages (query) != 1) {
> +    if (notmuch_query_count_messages (query, &count)) {
> +       fprintf (stderr, "Error: Xapian exception counting messages.\n");
> +       return 1;
> +    }
> +    if (count != 1) {
>         fprintf (stderr, "Error: search term did not match precisely one message.\n");
>         return 1;
>      }