xapian exceptions not caught in python bindings?

xapian exceptions not caught in python bindings?

[Patrick Totzke]

[-- Attachment #1: Type: text/plain, Size: 2313 bytes --]

Hi all!
It's me again, with some strange python behaviour :/

When I iterate over threads or messages only partially,
then make changes to the index and continue the iteration,
then I don't necessarily get NotmuchError's but the underlying
libnotmuch seems to call terminate. I get the following two lines on
stderr:
  terminate called after throwing an instance of 'Xapian::DatabaseModifiedError'
  Aborted
then python aborts without an exception.
This happens on current master with current python bindings. To reproduce
use this to iterate over messages:

------------------%<-------------------
#it_read.py
from notmuch import Query,Database
msgs = Database().create_query('*').search_messages()

i=0
for m in msgs:
    i=i+1
    if i%50==0:
        raw_input()
    readit = str(m)
------------------>%-------------------

start it, then use this to change the index:

------------------%<-------------------
#it_write.py 
from notmuch import Query,Database
db = Database(mode = Database.MODE.READ_WRITE)
msgs = Query(db,'*').search_messages()

i=0
for m in msgs:
    i=i+1
    if i%20==0:
        m.add_tag('REMOVEME')
------------------>%-------------------

you have to alternate a bit between hitting enter in the
read code and rerunning the write code.
Read will fail at some point.

The strange thing is that its not consistent:
I stumbled upon this while reading a list of threads from notmuch.threads on
demand. If reading from the iterator catches a NotmuchError, i create a new one
and update my list threads read so far. This does work, but: on every
NotmuchError, I get this on stderr:
 A Xapian exception occurred performing query: The revision being read has been discarded - you should call Xapian::Database::reopen() and retry the operation
 Query string was: thread:0000000000001876

After a couple of resets this does not happen again but the next time i read
from an outdated index, the whole UI terminates with the "Aborted" message
stated above.

To sum up, I think there are two bad things happening here:
1) should libnotmuch really print to stderr?
2) the python bindings are either not relaying all exceptions or libnotmuch is not relaying 
all xapian exceptions in order for the bindings to intercept them

cheers,
/p

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: xapian exceptions not caught in python bindings?

[Patrick Totzke]

[-- Attachment #1: Type: text/plain, Size: 4017 bytes --]

Hi all,

I know this issue is not easy to fix and that it's partly a problem
with xapians bad error handling. I just want to state another problem we run
into with xapians exceptions in combination with the python bindings.
If you run the following snippet, you notice that not only do we get
xapian-garbage on stderr but we don't really get any exceptions at the
position where it would make sense:

--------------------------------
from notmuch import Database
bad_querystring = "test AND"

# This should raise a NotmuchError and query should be NoneType
query = Database().create_query(bad_querystring)

# this prints to stderr but returns a Threads object.
threads = query.search_threads()

# this finally raises a NotmuchError(STATUS.NOT_INITIALIZED)
for t in threads: print t
--------------------------------

Although the querystring is invalid syntactically, the query object gets 
instantiated and can of course not behave as it should later on.
From a user point of view it is really quite strange that only later
(if one iterates over threads) an exception is raised.

One solution that comes to mind here is simply bending sys.sdterr to something internal,
interpreting its output and raising the correct exceptions accordingly.
The downside is, this would have to happen every time the library gets called.
Also, a fix like this would be python specific, so everybody else would still get
garbage printed to stderr occasionally.

Thoughts anyone?
/p



On Sun, Jun 26, 2011 at 09:27:33PM +0100, Patrick Totzke wrote:
> Hi all!
> It's me again, with some strange python behaviour :/
> 
> When I iterate over threads or messages only partially,
> then make changes to the index and continue the iteration,
> then I don't necessarily get NotmuchError's but the underlying
> libnotmuch seems to call terminate. I get the following two lines on
> stderr:
>   terminate called after throwing an instance of 'Xapian::DatabaseModifiedError'
>   Aborted
> then python aborts without an exception.
> This happens on current master with current python bindings. To reproduce
> use this to iterate over messages:
> 
> ------------------%<-------------------
> #it_read.py
> from notmuch import Query,Database
> msgs = Database().create_query('*').search_messages()
> 
> i=0
> for m in msgs:
>     i=i+1
>     if i%50==0:
>         raw_input()
>     readit = str(m)
> ------------------>%-------------------
> 
> start it, then use this to change the index:
> 
> ------------------%<-------------------
> #it_write.py 
> from notmuch import Query,Database
> db = Database(mode = Database.MODE.READ_WRITE)
> msgs = Query(db,'*').search_messages()
> 
> i=0
> for m in msgs:
>     i=i+1
>     if i%20==0:
>         m.add_tag('REMOVEME')
> ------------------>%-------------------
> 
> you have to alternate a bit between hitting enter in the
> read code and rerunning the write code.
> Read will fail at some point.
> 
> The strange thing is that its not consistent:
> I stumbled upon this while reading a list of threads from notmuch.threads on
> demand. If reading from the iterator catches a NotmuchError, i create a new one
> and update my list threads read so far. This does work, but: on every
> NotmuchError, I get this on stderr:
>  A Xapian exception occurred performing query: The revision being read has been discarded - you should call Xapian::Database::reopen() and retry the operation
>  Query string was: thread:0000000000001876
> 
> After a couple of resets this does not happen again but the next time i read
> from an outdated index, the whole UI terminates with the "Aborted" message
> stated above.
> 
> To sum up, I think there are two bad things happening here:
> 1) should libnotmuch really print to stderr?
> 2) the python bindings are either not relaying all exceptions or libnotmuch is not relaying 
> all xapian exceptions in order for the bindings to intercept them
> 
> cheers,
> /p



[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: xapian exceptions not caught in python bindings?

[David Bremner]

On Sun, 17 Jul 2011 20:35:38 +0100, Patrick Totzke <patricktotzke@googlemail.com> wrote:
> If you run the following snippet, you notice that not only do we get
> xapian-garbage on stderr but we don't really get any exceptions at the
> position where it would make sense:

I wouldn't call that "xapian-garbage" since it is output from
libnotmuch.

d

[PATCH] interpret Xapian errors from sdterr as exceptions

[pazz]

This introduces globals.RaiseStderrErrors, a ContextManager
that raises error messages printed by libnotmuch to stderr
as NotmuchError(STATUS.XAPIAN_EXCEPTION, message=err).
---
 bindings/python/notmuch/database.py |    5 +++++
 bindings/python/notmuch/globals.py  |   24 ++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/bindings/python/notmuch/database.py b/bindings/python/notmuch/database.py
index 874087e..443980b 100644
--- a/bindings/python/notmuch/database.py
+++ b/bindings/python/notmuch/database.py
@@ -18,8 +18,10 @@ Copyright 2010 Sebastian Spaeth <Sebastian@SSpaeth.de>'
 """
 
 import os
+
 from ctypes import c_int, c_char_p, c_void_p, c_uint, c_long, byref
 from notmuch.globals import nmlib, STATUS, NotmuchError, Enum
+from notmuch.globals import RaiseStderrErrors
 from notmuch.thread import Threads
 from notmuch.message import Messages, Message
 from notmuch.tag import Tags
@@ -540,6 +542,9 @@ class Query(object):
         if query_p is None:
             NotmuchError(STATUS.NULL_POINTER)
         self._query = query_p
+        # ensure Xapian errors from stderr get raised if query syntax is bad
+        with RaiseStderrErrors():
+            Query._count_messages(self._query)
 
     def set_sort(self, sort):
         """Set the sort order future results will be delivered in
diff --git a/bindings/python/notmuch/globals.py b/bindings/python/notmuch/globals.py
index 77f2905..5e527ca 100644
--- a/bindings/python/notmuch/globals.py
+++ b/bindings/python/notmuch/globals.py
@@ -17,6 +17,10 @@ along with notmuch.  If not, see <http://www.gnu.org/licenses/>.
 Copyright 2010 Sebastian Spaeth <Sebastian@SSpaeth.de>'
 """
 
+import tempfile
+import sys
+import os
+
 from ctypes import CDLL, c_char_p, c_int
 from ctypes.util import find_library
 
@@ -98,3 +102,23 @@ class NotmuchError(Exception):
             return self.args[0]
         else:
             return STATUS.status2str(self.args[1])
+
+
+class RaiseStderrErrors:
+    def __enter__(self):
+        sys.stderr.flush()
+        (self.errfd, fn) = tempfile.mkstemp()
+        self.ferr = os.fdopen(self.errfd, 'r')
+        os.unlink(fn)
+        self.oldstderr = os.dup(sys.stderr.fileno())
+        os.dup2(self.errfd, sys.stderr.fileno())
+
+    def __exit__(self, *args):
+        sys.stderr.flush()
+        os.dup2(self.oldstderr, sys.stderr.fileno())
+        os.close(self.oldstderr)
+        os.lseek(self.errfd, 0, 0)
+        err = self.ferr.read()
+        if err:
+            raise NotmuchError(STATUS.XAPIAN_EXCEPTION, message=err)
+        self.ferr.close()
-- 
1.7.4.1

Re: xapian exceptions not caught in python bindings?

[Patrick Totzke]

[-- Attachment #1: Type: text/plain, Size: 3340 bytes --]

Hi all,

I hope the patch I send is correctly formated, I'm still fumbling with
git send-email and the --in-reply-to option.
Anyhow, forgive my language, of course I didn't mean to be condescending in any
way by calling these prints garbage! It's just that it's highly unusual and very
'non-pythonic' that a module directly prints to stderr instead of raising exceptions
and if you work directly with a curseslike interface on a terminal these
errormessages litter my screen.

The patch I send is a suggestion how to fix the behaviour described in my last post.
It introduces a ContextManager class that can be used to raise messages from stderr
as Xapian exceptions like this:

> from notmuch.globals import RaiseStderrErrors
> with RaiseStderrErrors():
>  do_stuff()

Now, if one executes: 
------------
> from notmuch import Database
> bad_querystring = "test AND"
> query = Database().create_query(bad_querystring
---------------
one gets a nice Xapian exception
-----------------------
  File "syntax.py", line 4, in <module>
    query = Database().create_query(bad_querystring)
  File "/usr/local/lib/python2.7/dist-packages/notmuch/database.py", line 432, in create_query
    return Query(self, querystring)
  File "/usr/local/lib/python2.7/dist-packages/notmuch/database.py", line 514, in __init__
    self.create(db, querystr)
  File "/usr/local/lib/python2.7/dist-packages/notmuch/database.py", line 547, in create
    Query._count_messages(self._query)
  File "/usr/local/lib/python2.7/dist-packages/notmuch/globals.py", line 123, in __exit__
    raise NotmuchError(STATUS.XAPIAN_EXCEPTION, message=err)
notmuch.globals.NotmuchError: A Xapian exception occurred: Syntax: <expression> AND <expression>
Query string was: test AND
----------------------------

There are two problems with this suggestion however.
First, one needs to redirect sdterr to a tempfile: Using StringIO doesn't work since
just replacing sys.stderr with something else is not "low level" enough, the messages will
still get printed. An alternative is using os.dup2, which replaces the file descriptor
of stderr directly, but cStringIO objects dont have .fileno()..
see [0,1].
The second problem is, that creating a query object with a malformed querystring doesn't
print anything to stderr, only if you use that object errors get printed (notmuch/lib/query.cc).
Thus, I call count_messages() once after initialising a new notmuch.databas.Query object (at the end of  Query.create). This ensures that Query.__init__() raises an exception when given bad 
querystrings, but at the cost of triggering an unnecessary count_messages().

best,
/p



----
[0]: http://stackoverflow.com/questions/5903501/attributeerror-stringio-instance-has-no-attribute-fileno
[1]: http://code.activestate.com/recipes/577564-context-manager-for-low-level-redirection-of-stdou/


On Sun, Jul 17, 2011 at 04:51:41PM -0300, David Bremner wrote:
> On Sun, 17 Jul 2011 20:35:38 +0100, Patrick Totzke <patricktotzke@googlemail.com> wrote:
> > If you run the following snippet, you notice that not only do we get
> > xapian-garbage on stderr but we don't really get any exceptions at the
> > position where it would make sense:
> 
> I wouldn't call that "xapian-garbage" since it is output from
> libnotmuch.
> 
> d

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: xapian exceptions not caught in python bindings?

[Sebastian Spaeth]

[-- Attachment #1: Type: text/plain, Size: 1157 bytes --]

On Sat, 23 Jul 2011 14:36:02 +0100, Patrick Totzke wrote:
> I hope the patch I send is correctly formated, I'm still fumbling with
> git send-email and the --in-reply-to option.
> Anyhow, forgive my language, of course I didn't mean to be condescending in any
> way by calling these prints garbage! It's just that it's highly unusual and very
> 'non-pythonic' that a module directly prints to stderr instead of raising exceptions
> and if you work directly with a curseslike interface on a terminal these
> errormessages litter my screen.

Hi,
1) I fixed the lack of throwing a NotmuchError when search_threads()
failed. This was a bug.

2) Taking over stderr and capturing it in a temporary file whenever we
create a query sounds incredibly hackish to me. It also potentially has
side-effects that I cannot even judge (we might be using stderr for
something completely different).

IMHO, libnotmuch should be modified to not directly print to stderr
but to provide a string with a detailed error message together with it's
status value. This would be the proper way to deal with it, even if it
means a bit more complexity in the notmuch binary.

Sebastian

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]