Exception in `.map`, `.each` did not raise

[Gr1N]

Hello!

Below you can find test which can help you to reproduce a bug:

diff --git a/tests/map_test.py b/tests/map_test.py
index 0361dbc..30b3736 100644
--- a/tests/map_test.py
+++ b/tests/map_test.py
@@ -42,6 +42,16 @@ def test_map_invalid_coro():
         run_in_loop(map(None))

+def test_map_exceptions():
+    @asyncio.coroutine
+    def coro(num):
+        raise ValueError('foo')
+
+    task = map(coro, [1, 2, 3, 4, 5], return_exceptions=False)
+    with pytest.raises(ValueError):
+        run_in_loop(task)
+
+
 def test_map_return_exceptions():
     @asyncio.coroutine
     def coro(num):

To my mind this is unexpected behaviour, what do you think?

[h2non]

The current behavior is legit and asyncio idiomatic, which does not imply a typical try-catch control flow error exception mechanism. Instead, coroutines results are wrapped as Futures which encapsulates a binary state (succeed or failed). Failed coroutines would have a Future.exception value.

Consider the following asyncio raw example:

import asyncio

async def task():
    raise RuntimeError('Oops')

async def run():
    try:
        futures = await asyncio.wait([task(), task(), task()], return_when='FIRST_EXCEPTION')
    except Exception as err:
        print('Raised exception:', err)
    else:
        print('No raised exception:', futures)

loop = asyncio.get_event_loop()
loop.run_until_complete(run())
loop.close()

It will not raise an exception even if a coroutine raises it, however you can iterate futures in order to inspect which one failed and retrieve the exception details.

We can argue if that behavior is the most convenient for most cases, but it's currently the stdlib behavior and paco is not intended to change this.

[Gr1N]

Understand and yes if we want to use .wait this is right for asyncio, but here an another option in asyncio — .gather (https://docs.python.org/3/library/asyncio-task.html#asyncio.gather). And this is why I asked this question. As you can see .gather has return_exceptions, but if value is False exceptions will be raised and you can catch them.

In [1]: import asyncio

In [2]: loop = asyncio.get_event_loop()

In [3]: async def foo():
   ...:     raise NotImplementedError()
   ...:

In [4]: try:
   ...:     loop.run_until_complete(asyncio.gather(foo(), foo()))
   ...: except:
   ...:     print('ooops')
   ...:
ooops

In [5]: try:
   ...:     results = loop.run_until_complete(asyncio.gather(foo(), foo(), return_exceptions=True))
   ...: except:
   ...:     print('ooops')
   ...:

In [6]: results
Out[6]: [NotImplementedError(), NotImplementedError()]

In [7]:

[h2non]

I agree that return_exceptions argument may be confusing in this context, since it implies behavior from asyncio.gather().

I'm open to change this behavior since both scenarios can be perfectly valid and legit.

Do you have a concrete proposal?

Perhaps something like:

if return_exceptions is False and return_when == 'FIRST_EXCEPTION':
  if err:
     raise err

[h2non]

@Gr1N if you are interested, take a look at #36. It should fix this. Any feedback is highly appreciated.

[Gr1N]

LGTM!