Fixed unhelpful error message in from_thread_run functions.

trio/_core/_run.py

@@ -17,7 +17,6 @@
 from sniffio import current_async_library_cvar
 
 import attr
-from async_generator import isasyncgen
 from sortedcontainers import SortedDict
 from outcome import Error, Value, capture
 
@@ -36,7 +35,7 @@
 )
 from .. import _core
 from .._deprecate import deprecated
-from .._util import Final, NoPublicConstructor
+from .._util import Final, NoPublicConstructor, coroutine_or_error
 
 _NO_SEND = object()
 
@@ -1243,86 +1242,8 @@ def spawn_impl(self, async_fn, args, nursery, name, *, system_task=False):
         if nursery is None:
             assert self.init_task is None
 
-        ######
-        # Call the function and get the coroutine object, while giving helpful
-        # errors for common mistakes.
-        ######
-
-        def _return_value_looks_like_wrong_library(value):
-            # Returned by legacy @asyncio.coroutine functions, which includes
-            # a surprising proportion of asyncio builtins.
-            if isinstance(value, collections.abc.Generator):
-                return True
-            # The protocol for detecting an asyncio Future-like object
-            if getattr(value, "_asyncio_future_blocking", None) is not None:
-                return True
-            # This janky check catches tornado Futures and twisted Deferreds.
-            # By the time we're calling this function, we already know
-            # something has gone wrong, so a heuristic is pretty safe.
-            if value.__class__.__name__ in ("Future", "Deferred"):
-                return True
-            return False
-
-        try:
-            coro = async_fn(*args)
-        except TypeError:
-            # Give good error for: nursery.start_soon(trio.sleep(1))
-            if isinstance(async_fn, collections.abc.Coroutine):
-                raise TypeError(
-                    "Trio was expecting an async function, but instead it got "
-                    "a coroutine object {async_fn!r}\n"
-                    "\n"
-                    "Probably you did something like:\n"
-                    "\n"
-                    "  trio.run({async_fn.__name__}(...))            # incorrect!\n"
-                    "  nursery.start_soon({async_fn.__name__}(...))  # incorrect!\n"
-                    "\n"
-                    "Instead, you want (notice the parentheses!):\n"
-                    "\n"
-                    "  trio.run({async_fn.__name__}, ...)            # correct!\n"
-                    "  nursery.start_soon({async_fn.__name__}, ...)  # correct!"
-                    .format(async_fn=async_fn)
-                ) from None
-
-            # Give good error for: nursery.start_soon(future)
-            if _return_value_looks_like_wrong_library(async_fn):
-                raise TypeError(
-                    "Trio was expecting an async function, but instead it got "
-                    "{!r} – are you trying to use a library written for "
-                    "asyncio/twisted/tornado or similar? That won't work "
-                    "without some sort of compatibility shim."
-                    .format(async_fn)
-                ) from None
-
-            raise
-
-        # We can't check iscoroutinefunction(async_fn), because that will fail
-        # for things like functools.partial objects wrapping an async
-        # function. So we have to just call it and then check whether the
-        # return value is a coroutine object.
-        if not isinstance(coro, collections.abc.Coroutine):
-            # Give good error for: nursery.start_soon(func_returning_future)
-            if _return_value_looks_like_wrong_library(coro):
-                raise TypeError(
-                    "start_soon got unexpected {!r} – are you trying to use a "
-                    "library written for asyncio/twisted/tornado or similar? "
-                    "That won't work without some sort of compatibility shim."
-                    .format(coro)
-                )
-
-            if isasyncgen(coro):
-                raise TypeError(
-                    "start_soon expected an async function but got an async "
-                    "generator {!r}".format(coro)
-                )
-
-            # Give good error for: nursery.start_soon(some_sync_fn)
-            raise TypeError(
-                "Trio expected an async function, but {!r} appears to be "
-                "synchronous".format(
-                    getattr(async_fn, "__qualname__", async_fn)
-                )
-            )
+        # Check if async_fn is callable coroutine
+        coro = coroutine_or_error(async_fn, args)
 
         ######
         # Set up the Task object

trio/_core/tests/test_run.py

@@ -1696,74 +1696,6 @@ async def test_current_effective_deadline(mock_clock):
     assert _core.current_effective_deadline() == inf
 
 
-# @coroutine is deprecated since python 3.8, which is fine with us.
-@pytest.mark.filterwarnings("ignore:.*@coroutine.*:DeprecationWarning")
-def test_nice_error_on_bad_calls_to_run_or_spawn():
-    def bad_call_run(*args):
-        _core.run(*args)
-
-    def bad_call_spawn(*args):
-        async def main():
-            async with _core.open_nursery() as nursery:
-                nursery.start_soon(*args)
-
-        _core.run(main)
-
-    class Deferred:
-        "Just kidding"
-
-    with ignore_coroutine_never_awaited_warnings():
-        for bad_call in bad_call_run, bad_call_spawn:
-
-            async def f():  # pragma: no cover
-                pass
-
-            with pytest.raises(TypeError) as excinfo:
-                bad_call(f())
-            assert "expecting an async function" in str(excinfo.value)
-
-            import asyncio
-
-            @asyncio.coroutine
-            def generator_based_coro():  # pragma: no cover
-                yield from asyncio.sleep(1)
-
-            with pytest.raises(TypeError) as excinfo:
-                bad_call(generator_based_coro())
-            assert "asyncio" in str(excinfo.value)
-
-            with pytest.raises(TypeError) as excinfo:
-                bad_call(asyncio.Future())
-            assert "asyncio" in str(excinfo.value)
-
-            with pytest.raises(TypeError) as excinfo:
-                bad_call(lambda: asyncio.Future())
-            assert "asyncio" in str(excinfo.value)
-
-            with pytest.raises(TypeError) as excinfo:
-                bad_call(Deferred())
-            assert "twisted" in str(excinfo.value)
-
-            with pytest.raises(TypeError) as excinfo:
-                bad_call(lambda: Deferred())
-            assert "twisted" in str(excinfo.value)
-
-            with pytest.raises(TypeError) as excinfo:
-                bad_call(len, [1, 2, 3])
-            assert "appears to be synchronous" in str(excinfo.value)
-
-            async def async_gen(arg):  # pragma: no cover
-                yield
-
-            with pytest.raises(TypeError) as excinfo:
-                bad_call(async_gen, 0)
-            msg = "expected an async function but got an async generator"
-            assert msg in str(excinfo.value)
-
-            # Make sure no references are kept around to keep anything alive
-            del excinfo
-
-
 def test_calling_asyncio_function_gives_nice_error():
     async def child_xyzzy():
         import asyncio

trio/_threads.py

@@ -3,12 +3,14 @@
 from itertools import count
 
 import attr
+import inspect
 import outcome
 
 import trio
 
 from ._sync import CapacityLimiter
 from ._core import enable_ki_protection, disable_ki_protection, RunVar, TrioToken
+from ._util import coroutine_or_error
 
 # Global due to Threading API, thread local storage for trio token
 TOKEN_LOCAL = threading.local()
@@ -365,6 +367,7 @@ def from_thread_run(afn, *args, trio_token=None):
             which would otherwise cause a deadlock.
         AttributeError: if no ``trio_token`` was provided, and we can't infer
             one from context.
+        TypeError: if ``afn`` is not an asynchronous function.
 
     **Locating a Trio Token**: There are two ways to specify which
     `trio.run` loop to reenter:
@@ -380,7 +383,8 @@ def from_thread_run(afn, *args, trio_token=None):
     def callback(q, afn, args):
         @disable_ki_protection
         async def unprotected_afn():
-            return await afn(*args)
+            coro = coroutine_or_error(afn, args)
+            return await coro
 
         async def await_in_trio_thread_task():
             q.put_nowait(await outcome.acapture(unprotected_afn))
@@ -404,12 +408,13 @@ def from_thread_run_sync(fn, *args, trio_token=None):
         RunFinishedError: if the corresponding call to `trio.run` has
             already completed.
         Cancelled: if the corresponding call to `trio.run` completes
-            while ``afn(*args)`` is running, then ``afn`` is likely to raise
+            while ``fn(*args)`` is running, then ``fn`` is likely to raise
             :exc:`trio.Cancelled`, and this will propagate out into
         RuntimeError: if you try calling this from inside the Trio thread,
             which would otherwise cause a deadlock.
         AttributeError: if no ``trio_token`` was provided, and we can't infer
-            one from context.
+            one from context. Also if ``fn`` is not a sync function.
+        TypeError: if ``fn`` is not callable or is an async function.
 
     **Locating a Trio Token**: There are two ways to specify which
     `trio.run` loop to reenter:
@@ -422,6 +427,13 @@ def from_thread_run_sync(fn, *args, trio_token=None):
           "foreign" thread, spawned using some other framework, and still want
           to enter Trio.
     """
+
+    if not callable(fn) or inspect.iscoroutinefunction(fn):
+        raise TypeError(
+            "Trio expected a sync function, but {!r} appears to not be "
+            "callable or asynchronous".format(getattr(fn, "__qualname__", fn))
+        )
+
     def callback(q, fn, args):
         @disable_ki_protection
         def unprotected_fn():

trio/_util.py

@@ -8,6 +8,9 @@
 from functools import wraps, update_wrapper
 import typing as t
 import threading
+import collections
+
+from async_generator import isasyncgen
 
 from ._deprecate import warn_deprecated
 
@@ -85,6 +88,87 @@ def is_main_thread():
         return False
 
 
+######
+# Call the function and get the coroutine object, while giving helpful
+# errors for common mistakes. Returns coroutine object.
+######
+def coroutine_or_error(async_fn, args=[]):
+    def _return_value_looks_like_wrong_library(value):
+        # Returned by legacy @asyncio.coroutine functions, which includes
+        # a surprising proportion of asyncio builtins.
+        if isinstance(value, collections.abc.Generator):
+            return True
+        # The protocol for detecting an asyncio Future-like object
+        if getattr(value, "_asyncio_future_blocking", None) is not None:
+            return True
+        # This janky check catches tornado Futures and twisted Deferreds.
+        # By the time we're calling this function, we already know
+        # something has gone wrong, so a heuristic is pretty safe.
+        if value.__class__.__name__ in ("Future", "Deferred"):
+            return True
+        return False
+
+    try:
+        coro = async_fn(*args)
+    except TypeError:
+        # Give good error for: nursery.start_soon(trio.sleep(1))
+        if isinstance(async_fn, collections.abc.Coroutine):
+            raise TypeError(
+                "Trio was expecting an async function, but instead it got "
+                "a coroutine object {async_fn!r}\n"
+                "\n"
+                "Probably you did something like:\n"
+                "\n"
+                "  trio.run({async_fn.__name__}(...))            # incorrect!\n"
+                "  nursery.start_soon({async_fn.__name__}(...))  # incorrect!\n"
+                "\n"
+                "Instead, you want (notice the parentheses!):\n"
+                "\n"
+                "  trio.run({async_fn.__name__}, ...)            # correct!\n"
+                "  nursery.start_soon({async_fn.__name__}, ...)  # correct!"
+                .format(async_fn=async_fn)
+            ) from None
+
+        # Give good error for: nursery.start_soon(future)
+        if _return_value_looks_like_wrong_library(async_fn):
+            raise TypeError(
+                "Trio was expecting an async function, but instead it got "
+                "{!r} – are you trying to use a library written for "
+                "asyncio/twisted/tornado or similar? That won't work "
+                "without some sort of compatibility shim.".format(async_fn)
+            ) from None
+
+        raise
+
+    # We can't check iscoroutinefunction(async_fn), because that will fail
+    # for things like functools.partial objects wrapping an async
+    # function. So we have to just call it and then check whether the
+    # return value is a coroutine object.
+    if not isinstance(coro, collections.abc.Coroutine):
+        # Give good error for: nursery.start_soon(func_returning_future)
+        if _return_value_looks_like_wrong_library(coro):
+            raise TypeError(
+                "start_soon got unexpected {!r} – are you trying to use a "
+                "library written for asyncio/twisted/tornado or similar? "
+                "That won't work without some sort of compatibility shim."
+                .format(coro)
+            )
+
+        if isasyncgen(coro):
+            raise TypeError(
+                "start_soon expected an async function but got an async "
+                "generator {!r}".format(coro)
+            )
+
+        # Give good error for: nursery.start_soon(some_sync_fn)
+        raise TypeError(
+            "Trio expected an async function, but {!r} appears to be "
+            "synchronous".format(getattr(async_fn, "__qualname__", async_fn))
+        )
+
+    return coro
+
+
 class ConflictDetector:
     """Detect when two tasks are about to perform operations that would
     conflict.

trio/tests/test_threads.py

@@ -471,6 +471,15 @@ def thread_fn():
     trio_time = await to_thread_run_sync(thread_fn)
     assert isinstance(trio_time, float)
 
+    # Test correct error when passed async function
+    async def async_fn():  # pragma: no cover
+        pass
+
+    with pytest.raises(TypeError) as excinfo:
+        from_thread_run_sync(async_fn)
+
+    assert "expected a sync function" in str(excinfo.value)
+
 
 async def test_trio_from_thread_run():
     # Test that to_thread_run_sync correctly "hands off" the trio token to
@@ -488,6 +497,18 @@ def thread_fn():
     await to_thread_run_sync(thread_fn)
     assert record == ["in thread", "back in trio"]
 
+    # Test correct error when passed sync function
+    def sync_fn():  # pragma: no cover
+        pass
+
+    def thread_fn():
+        from_thread_run(sync_fn)
+
+    with pytest.raises(TypeError) as excinfo:
+        await to_thread_run_sync(thread_fn)
+
+    assert "appears to be synchronous" in str(excinfo.value)
+
 
 async def test_trio_from_thread_token():
     # Test that to_thread_run_sync and spawned trio.from_thread.run_sync()