Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No way to catch exceptions when calling coroutine from gen.Task #759

Closed
ghost opened this issue Apr 25, 2013 · 15 comments
Closed

No way to catch exceptions when calling coroutine from gen.Task #759

ghost opened this issue Apr 25, 2013 · 15 comments

Comments

@ghost
Copy link

ghost commented Apr 25, 2013

from tornado import gen
from tornado.ioloop import IOLoop

@gen.coroutine
def throw():
    10/0 # Exception here
    return 'hello'


@gen.coroutine
def test():
    print "i'm ok"
    res = yield gen.Task(throw)
    print "here too" # it is never executed


test()

IOLoop.instance().start()

There must be a way to catch exceptions.

@bdarnell
Copy link
Member

Exceptions should work as usual: just put a try/except block around the "yield gen.Task" statement.

@ajdavis
Copy link
Contributor

ajdavis commented Apr 25, 2013

@kanski you could catch the exception like so:

@gen.coroutine
def test():
    print "i'm ok"
    try:
        res = yield gen.Task(throw)
    except Exception, e:
        print 'EXCEPTION!', e
    print "here too"

This prints:

i'm ok
EXCEPTION! integer division or modulo by zero
here too

A few corrections to your code, by the way. Since all coroutines return Futures, you can omit gen.Task and have the same outcome:

@gen.coroutine
def test():
    print "i'm ok"
    try:
        res = yield throw()
    except Exception, e:
        print 'EXCEPTION!', e
    print "here too"

Also, if you ever put a yield statement in throw() then the return 'hello' will no longer be valid: you'll get a SyntaxError: 'return' with argument inside generator in Pythons older than 3.3. Better to use raise gen.Return('hello').

So, the real question you're asking is, I think, what happened to the exception? and the answer is, it's wrapped in the Future returned by test(). If you do not add a try / except in test, you can do this to catch the exception instead:

def done_callback(future):
    future.result()

future = test()
future.add_done_callback(done_callback)

IOLoop.instance().start()

If you run this code you'll get a lengthy traceback ending in ZeroDivisionError.

I agree that this is a confusing consequence of using gen.coroutine in a command-line application like the one you're writing. If you were using coroutines in RequestHandlers for a web application, Tornado would log or display the error.

@bdarnell
Copy link
Member

The method IOLoop.run_sync exists to help with this situation: IOLoop.instance().run_sync(test) is equivalent to the block above with future.add_done_callback.

@ajdavis
Copy link
Contributor

ajdavis commented Apr 25, 2013

Ah, I knew I was missing a more convenient method.

@ghost
Copy link
Author

ghost commented Apr 26, 2013

@gen.coroutine
def test():
    print "i'm ok"
    try:
        res = yield gen.Task(throw)
    except Exception, e:
        print 'EXCEPTION!', e
    print "here too"

Yes, this is working, but what if i don't know where to put try..except block? It's just stops with no errors and i must seek where exception may raise. This is very annoying.

res = yield throw()

This is exactly what i need! Thank you!

@ghost ghost closed this as completed Apr 27, 2013
@truetug
Copy link

truetug commented May 29, 2013

It was a simple case. Let's see more complicated.

@gen.coroutine
def do_not_throw():
    raise gen.Return('hello')

@gen.coroutine
def throw():
    10/0 # Exception here
    raise gen.Return('hello')

@gen.coroutine
def test():
    print "i'm ok"

    try:
        res = yield [
            do_not_throw(),
            throw(),
        ]
    except Exception, e:
        print 'EXCEPTION!', e

    print "here too"

First task will return value, but second will raise ZeroDivisionError. So we'll lost value of first task because of exception in second task.

In my case, I use gen.Callback and gen.WaitAll to avoid exception (if any url returns not 200)

@tornado.web.asynchronous
@tornado.gen.coroutine
def post(self):
    client = tornado.httpclient.AsyncHTTPClient()

    keys = []
    for i, x in enumerate(self.get_arguments('urls')):
        key = '%sk' % i
        keys.append(key)

        http_client.fetch(x, callback=(yield tornado.gen.Callback(key)))

    response = yield tornado.gen.WaitAll(keys)

Is it right?

@bdarnell
Copy link
Member

Yes, when you do "yield [do_not_throw(), throw()]", you lose the result of do_not_throw(), just like with regular tuple assignment: "a, b = do_not_throw(), throw()" with normal functions doesn't assign to either variable. You can either call the fetches one at a time, use callbacks instead of futures, or wrap the fetch operation in a helper function that will catch the exception.

@Rustem
Copy link

Rustem commented Oct 6, 2013

What have you decided about silently passing exceptions in tornado coroutines ?
http://stackoverflow.com/questions/19205828/silently-passing-exceptions-with-future-objects

@bdarnell
Copy link
Member

bdarnell commented Oct 6, 2013

What is the question that needs a decision? I don't anticipate any changes in Future-related exception handling except perhaps a __del__ hook to help warn you when you forget to wait on a Future.

@cganterh
Copy link

The silent exceptions in coroutines were fixed in 4.1. Does this mean that I no longer have to add a big try-catch block for every coroutine? Or it is safer to do it anyway?

@bdarnell
Copy link
Member

No, you shouldn't have to add try/except block around every coroutine just to log them. Just let the exceptions escape to the higher level. This was true even before 4.1; the change in 4.1 was to catch a particular kind of mistake: calling a coroutine without the yield keyword.

@cganterh
Copy link

Calling a coroutine without the yield keyword is always a mistake? What if, in a specific part of my code, I don't want to wait for that coroutine to complete?

@bdarnell
Copy link
Member

You should always do something with the result of a coroutine; it is always a mistake to call a coroutine and ignore the Future it returns.
If you don't want to wait for a coroutine to complete, you should run it with IOLoop.spawn_callback. This ensures that any exceptions it raises will A) be logged and B) not interfere with the function/coroutine that started it.

@Morriaty-The-Murderer
Copy link

Hi @bdarnell

I'm new to asynchronous programming and being messed with it.

I got confused when I decorated the RequestHandler with gen.coroutine but found the request was still blocked.

Here is a brief code, with python 2.7.11 and tornado 4.4.1

@gen.coroutine
def store_data(data):
   try:
      # parse_data
      ....
   except ParseError as e:
      logger.warning(e)
      return
   yield motor.insert_many(parsed_data)  # asynchronous mongo
   print motor.count()

class MainHandler(RequestHandler):
    @gen.coroutine
    def post(self):
        try:
            some_argument = int(self.get_argument("some", 0))
            data = self.request.body
        except Exception:
            self.write("Improper Argument")
            self.finish()
            return
        IOLoop.current().spawn_callback(lambda: store_data(data))
        self.write("Request Done")
        self.finish()

And I made a test with 10 threads. According to the response time in access log, I suppose some requests were blocked

[I 161222 15:40:22 web:1971] 200 POST /upload/ (::1) 9.00ms
[I 161222 15:40:23 web:1971] 200 POST /upload/ (::1) 8.00ms
[I 161222 15:40:23 web:1971] 200 POST /upload/ (::1) 8.00ms
[I 161222 15:40:23 web:1971] 200 POST /upload/ (::1) 7.00ms
[I 161222 15:40:23 web:1971] 200 POST /upload/ (::1) 8.00ms
[I 161222 15:40:23 web:1971] 200 POST /upload/ (::1) 9.00ms
[I 161222 15:40:23 web:1971] 200 POST /upload/ (::1) 8.00ms
[I 161222 15:40:23 web:1971] 200 POST /upload/ (::1) 9.00ms
[I 161222 15:40:23 web:1971] 200 POST /upload/ (::1) 701.00ms # Seem blocked
[I 161222 15:40:23 web:1971] 200 POST /upload/ (::1) 696.00ms # Seem blocked

@bdarnell
Copy link
Member

@Morriaty-The-Murderer The issue tracker is for reporting issues in tornado itself, not problems using tornado. Questions like yours are better suited to the mailing list or stack overflow. (and this particular issue was closed three years ago and doesn't look relevant to your question)

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants