From: David Bremner <david@tethera.net>
To: Matt <mattator@gmail.com>
Subject: Re: Python bindings and Xapian exceptions
In-Reply-To:
 <CADHp1NyiuG-V2b68fsie_Fhn0kigUM88bPinhE9yZPCsWNW6QA@mail.gmail.com>
References: <87k4q7s43v.fsf@ut.hh.sledj.net> <878w6gis5g.fsf@SSpaeth.de>
	<871v70ril7.fsf@yoom.home.cworth.org> <871v70z0y5.fsf@SSpaeth.de>
	<loom.20141215T121709-488@post.gmane.org>
	<87iohc7iiz.fsf@maritornes.cs.unb.ca>
	<CADHp1NyiuG-V2b68fsie_Fhn0kigUM88bPinhE9yZPCsWNW6QA@mail.gmail.com>
User-Agent: Notmuch/0.19+7~g5d7f7a6 (http://notmuchmail.org) Emacs/24.4.1
	(x86_64-pc-linux-gnu)
Date: Tue, 16 Dec 2014 09:02:17 +0100
Message-ID: <87fvcg6n06.fsf@maritornes.cs.unb.ca>
MIME-Version: 1.0
Content-Type: text/plain
Cc: notmuch@notmuchmail.org
Precedence: list

Matt <mattator@gmail.com> writes:

> 2014-12-15 21:41 GMT+01:00 David Bremner <david@tethera.net>:
>> Matt <mattator@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;
     }