Exceptions inside greenlets

[Jeff Lindsay]

It seems like always printing the traceback for exceptions in
greenlets is a little harsh, especially when testing code that throws
(and catches) exceptions. Maybe I should be doing something else? What
is the best practice for this?

On a related note, I can't get exceptions to fire when I call
join(raise_error=True) on an Pool or Group (in this case a Group)...
even when I set _exception manually.

--
Jeff Lindsay
http://progrium.com

[Ned Rockson]

We ran into this during unit testing and we simply monkey patch the error handler in order to not print stack traces. In order to use this code, simply wrap it with:

with MakeGreenletsNotLogErrors():

  <CODE>

And that should do it. Code follows:

try:

  from StringIO import cStringIO as StringIO

except ImportError:

  from StringIO import StringIO

from contextlib import contextmanager

import sys

import gevent

@contextmanager

def MakeGreenletsNotLogErrors():

  """Monkey-patch in a custom Greenlet subclass that suppresses stderr for

  the duration of this with block Greenlet printing stack traces. Upon exit

  of the block, the original greenlet class will be reloaded.

  Yields:

    A useless value to fit the contextmanager paradigm.

  """

  _OriginalGreenletClass = gevent.Greenlet

  class NoErrorLoggingGreenlet(_OriginalGreenletClass):

    def _report_error(self, exc_info):

      _stderr = sys.stderr

      try:

        sys.stderr = StringIO()

        _OriginalGreenletClass._report_error(self, exc_info)

      finally:

        sys.stderr = _stderr

  try:

    gevent.Greenlet = gevent.greenlet.Greenlet = NoErrorLoggingGreenlet

    reload(gevent)

    yield 1

  finally:

    gevent.Greenlet = gevent.greenlet.Greenlet = _OriginalGreenletClass

    reload(gevent)

[Jeff Lindsay]

Yowza. So I guess there isn't an established way. What would be an
ideal scenario? I don't see a ticket (only glanced), should I make
one? I'm also willing to write code.

-jeff

[Denis Bilenko]

On Tue, May 3, 2011 at 6:51 AM, Jeff Lindsay <prog...@gmail.com> wrote:
> It seems like always printing the traceback for exceptions in
> greenlets is a little harsh, especially when testing code that throws
> (and catches) exceptions. Maybe I should be doing something else? What
> is the best practice for this?

Wrap the raising code with:

try:
func()
except MyExceptions, ex:
return ex # or don't return 'ex' if you don't care about it.

There's a shortcut in gevent.util for that:

from gevent.util import wrap_errors
gevent.spawn(wrap_errors(MyExceptions, func))

That way the traceback won't be printed. (And the exception will be
available as 'value' attribute, not 'exception').

> On a related note, I can't get exceptions to fire when I call
> join(raise_error=True) on an Pool or Group (in this case a Group)...
> even when I set _exception manually.

Could you give a short test script that does not work?
Why would you set '_exception' manually?

On Tue, May 3, 2011 at 10:58 AM, Ned Rockson <n...@tellapart.com> wrote:
> We ran into this during unit testing and we simply monkey patch the error
> handler in order to not print stack traces. In order to use this code,
> simply wrap it with:
> with MakeGreenletsNotLogErrors():
> <CODE>
> And that should do it. Code follows:

But that's not only makes <CODE> hide the exceptions but also all
other greenlets in the program running at that time. You could be
swallowing important errors that way. I wouldn't recommend this.

Automatic printing of unhandled exceptions is life jacket that helps
you find out why the program does not work as expected. If it becomes
noisy, then catch and silent those specific exceptions that are not
logic errors in your program (e.g. if you do networking, socket errors
are probably not logic errors and thus don't let them kill the
greenlet).

BTW, if you still think monkey patching error-reporting code is the
best way to go then in 0.14 overriding Hub.handle_error would be a
cleaner way then patching stderr:

https://bitbucket.org/denis/gevent/src/23c2b5778af6/gevent/hub.py#cl-226

Hub.handle_error prints all the stacktraces in the new gevent, those
from callbacks, servers, greenlets. You get the context as the first
argument - where. For failed greenlets it will be a Greenlet instance
which you can detect with isinstance(where, Greenlet).

I still think it's best just to put try / except in the right places.

[Jeff Lindsay]

It's important to remember this is all for testing.

> There's a shortcut in gevent.util for that:
>
> from gevent.util import wrap_errors
> gevent.spawn(wrap_errors(MyExceptions, func))
>
> That way the traceback won't be printed. (And the exception will be
> available as 'value' attribute, not 'exception').

I guess the problem with this approach is that it assumes I normally
don't want exceptions to be printed. I'm actually ok with this
behavior when running my program, but not for testing. Really this is
only for testing. Maybe (since I don't have 0.14) I can monkey patch
gevent.spawn to always use wrap_errors when running tests (or maybe
just for this one test)...

>
>> On a related note, I can't get exceptions to fire when I call
>> join(raise_error=True) on an Pool or Group (in this case a Group)...
>> even when I set _exception manually.
>
> Could you give a short test script that does not work?
> Why would you set '_exception' manually?

I might punt on this if I can find a better way, but I was setting
_exception manually in an attempt to avoid actually raising and having
_report_error() called since it would print.

[Keith Gaughan]

On 03/05/11 05:52, Jeff Lindsay wrote:

> It's important to remember this is all for testing.
>
>> There's a shortcut in gevent.util for that:
>>
>> from gevent.util import wrap_errors
>> gevent.spawn(wrap_errors(MyExceptions, func))
>>
>> That way the traceback won't be printed. (And the exception will be
>> available as 'value' attribute, not 'exception').
>
> I guess the problem with this approach is that it assumes I normally
> don't want exceptions to be printed. I'm actually ok with this
> behavior when running my program, but not for testing. Really this is
> only for testing. Maybe (since I don't have 0.14) I can monkey patch
> gevent.spawn to always use wrap_errors when running tests (or maybe
> just for this one test)...

...or you could do something along the lines of this either where you're
calling gevent.spawn or in a module for handling code that changes between
tests and live:

import settings
from gevent.util import wrap_errors as real_wrap_errors

if settings.TESTING:
wrap_errors = real_wrap_errors
else:
wrap_errors = lambda _, func: func

Personally, I'd just let the traceback appear as if one appears, that's
probably a bug. I'm not a fan of monkeypatching unless absolutely inescapably
necessary.

K.

[Jeff Lindsay]

> ...or you could do something along the lines of this either where you're
> calling gevent.spawn or in a module for handling code that changes between
> tests and live:

Yeah, I considered this as well. It just doesn't sit right yet.

> Personally, I'd just let the traceback appear as if one appears, that's
> probably a bug. I'm not a fan of monkeypatching unless absolutely inescapably
> necessary.

DItto, except this whole thing seems terribly inelegant. The whole
point of testing is to test all situations, including error
conditions. Some exceptions aren't bugs, they're just ... exceptions.
They're there so you can catch that exceptional situation and handle
it. I don't agree that it should always print a traceback..
*especially* if you intend to catch it somehow.

[Jeff Lindsay]

Trying to think of a proper way to handle exceptions across
greenlets... if you can do this right, I see no reason to always print
the trace. Instead, you'd only print it when an exception handler
wasn't registered.

I feel like link_exception might be the closest way to achieving what
I'm thinking, but would I be wrong to assume it will still print the
trace?

I'm coming up with some ideas that I could implement, but they all
depend on wrap_errors, which effectively makes most of the exception
related code in Greenlet useless; since now all exceptions that I care
about are not exceptions any more from the perspective of the Greenlet
class.

-jeff

[Jeff Lindsay]

Here we go:
http://pastie.org/1865238

In my case, I'm dealing with greenlets in a group. So based on the
wrap_errors util, I extended the greenlet Group to let you register
exception handlers. It also passes the greenlet that registered the
handler so you can raise the caught exception in that greenlet (which
I needed for testing -- nose has that nice raises decorator!).

Anyway, feedback on this approach?

-jeff

[Denis Bilenko]

On Tue, May 3, 2011 at 9:32 PM, Jeff Lindsay <prog...@gmail.com> wrote:
>
> Trying to think of a proper way to handle exceptions across
> greenlets... if you can do this right, I see no reason to always print
> the trace. Instead, you'd only print it when an exception handler
> wasn't registered.
>
> I feel like link_exception might be the closest way to achieving what
> I'm thinking, but would I be wrong to assume it will still print the
> trace?

trace is printed always, yes. I actually experimented with skipping
printing the traceback if link_exception()
was called but found that situation where the exception was swallowed
but the handler from link_exception() has
not reported it (e.g. because it has a bug) is unpleasant.

In case of error reporting, better to be dumb, so that it's obvious
that error reporting code cannot fail to report error.

On Wed, May 4, 2011 at 8:57 PM, Jeff Lindsay <prog...@gmail.com> wrote:
>
> Here we go:
> http://pastie.org/1865238
>
> In my case, I'm dealing with greenlets in a group. So based on the
> wrap_errors util, I extended the greenlet Group to let you register
> exception handlers. It also passes the greenlet that registered the
> handler so you can raise the caught exception in that greenlet (which
> I needed for testing -- nose has that nice raises decorator!).
>
> Anyway, feedback on this approach?

- why do you pass the greenlet that called catch() to the handler? it
feels like you wanted to pass the greenlet that had an error.
- using throw() leaves the greenlet that called it unscheduled forever
and since it in a group it never gets removed; use kill()
- if I set 2 handlers, one for Exception and one for ValueError and
ValueError is raised, then both handlers will match and which one is
chosen depends on order in dictionary's keys().

Regarding the whole approach, it seems that you're re-implementing
Python's "try/except" mechanism and that does not feel right.

> I'm coming up with some ideas that I could implement, but they all
> depend on wrap_errors, which effectively makes most of the exception
> related code in Greenlet useless; since now all exceptions that I care
> about are not exceptions any more from the perspective of the Greenlet
> class.

If that's what you care about, then perhaps you can create a subclass
of Greenlet, that overrides
- get()
- exception property
- value property

and behaves as if greenlet has failed with an error even though
actually error instance was returned. (maybe some additional flag
if you want to distinguish from situations where exception instance
was actually returned from user code).

[Jeff Lindsay]

>> Anyway, feedback on this approach?
>
> - why do you pass the greenlet that called catch() to the handler? it
> feels like you wanted to pass the greenlet that had an error.

Because that's the greenlet/scope you'd probably want to raise an
exception in. That's what the final example is about. Throwing the
exception inside the greenlet just brings you back to the original
problem of "exceptions inside greenlets that are not handled in that
greenlet cannot be handled at all"

> - using throw() leaves the greenlet that called it unscheduled forever
> and since it in a group it never gets removed; use kill()

Yes, I noticed that, but I can't remember why I did it this way. It
seemed right at the time, but I'll defer to your knowledge about that.
But I would point out that this is more of one way to use my solution
as opposed being part of my solution.

> - if I set 2 handlers, one for Exception and one for ValueError and
> ValueError is raised, then both handlers will match and which one is
> chosen depends on order in dictionary's keys().

Good catch.

>
> Regarding the whole approach, it seems that you're re-implementing
> Python's "try/except" mechanism and that does not feel right.

Kind of. Again, the last example shows that I'm trying to make it so
you can actually use Python try/except in cases where there is a
greenlet involved that throws an exception. You cannot right now
without this approach as far as I know how.