Potential Issue: Overly restrictive coroutine check rejects valid Cython awaitables

[sknigh]

I created a minimal reproducer, but the issue template asks that potential bugs start as discussions. httpcore’s coroutine check rejects valid awaitables returned by Cython-compiled async callbacks.

Environment

Python: 3.14.3
httpcore: 1.0.9
Cython: 3.2.9
setuptools: 80.9.0
OS: Linux x86_64

Expected behavior

The Cython-compiled callback should be accepted because its result implements the awaitable protocol and can be used with await.

Actual behavior

inspect.isawaitable: True
inspect.iscoroutine: False

TypeError: If you're using an asynchronous interface, the callback of the
trace extension should be an asynchronous function rather than a normal
function.

RuntimeWarning: coroutine 'trace_callback' was never awaited

The pure-Python invocation reports:

inspect.isawaitable: True
inspect.iscoroutine: True

and completes successfully.

Likely cause

Trace.atrace() currently validates the returned object with:

if not inspect.iscoroutine(coro):

Because the object is immediately used with await, would it be appropriate to validate the general awaitable protocol instead?

if not inspect.isawaitable(coro):

Demonstration

repro.py

# /// script
# requires-python = ">=3.10"
# dependencies = ["httpcore==1.0.9"]
# ///

import asyncio
import inspect
import logging
import sys

import httpcore
from httpcore._trace import Trace


async def trace_callback(name, info):
    pass


async def run() -> None:
    callback = trace_callback if sys.argv[1] == "python" else __import__("repro_cython").trace_callback

    probe = callback("probe", {})
    print(f"inspect.isawaitable: {inspect.isawaitable(probe)}")
    print(f"inspect.iscoroutine: {inspect.iscoroutine(probe)}")
    await probe

    trace = Trace(
        "demo",
        logging.getLogger("repro"),
        httpcore.Request("GET", "https://example.com", extensions={"trace": callback}),
    )
    await trace.atrace("started", {"value": 1})


if __name__ == "__main__":
    asyncio.run(run())

How to run it

# Pure-Python callback works
uv run --python 3.14 repro.py python

# Copy and compile the callback with Cython
cp repro.py repro_cython.py
uv run \
  --python 3.14 \
  --with cython==3.2.9 \
  --with setuptools==80.9.0 \
  cythonize -i repro_cython.py

# Reproduce the httpcore failure
uv run --python 3.14 repro.py cython

[MLuc24]

Your reading looks right to me, and there's a detail in the source that supports it: the guard is marked as never-executed.

coro = self.trace_extension(prefix_and_name, info)
if not inspect.iscoroutine(coro):  # pragma: no cover
    raise TypeError(
        "If you're using an asynchronous interface, "
        "the callback of the `trace` extension should "
        "be an asynchronous function rather than a normal function."
    )
await coro

That # pragma: no cover means no test exercises the branch — it's a defensive message for a user mistake, not behaviour anything depends on. Which matters, because it makes the check the only thing standing between your Cython callback and a line that would otherwise work fine.

The argument for isawaitable is that the very next statement is await coro, and await needs __await__, nothing more. inspect.iscoroutine is strictly narrower than the requirement — it's true only for native coroutine objects, so it excludes anything else implementing the protocol correctly, which is exactly the case you hit. inspect.isawaitable is the predicate that matches what the code goes on to do.

It also doesn't weaken the guard's actual purpose. The mistake the message describes — passing a normal function — returns whatever that function returned, typically None, and isawaitable(None) is False. So the sync-callback case still raises with the same message; the only behaviour that changes is that correctly-implemented non-coroutine awaitables stop being rejected.

Worth mentioning the second symptom in your report as part of the same change: the raise happens after the callback has already been called, so the returned awaitable is discarded without ever being awaited, which is where the coroutine 'trace_callback' was never awaited warning comes from. It's harmless once the check stops firing spuriously, but it does mean the failure path currently leaks whatever the callback produced.

[muhammadbalawal]

Your reading is right, and I think the fix is safe for a reason beyond the awaitable protocol itself.

await is defined in terms of __await__, not in terms of being a coroutine object. inspect.iscoroutine is specifically a check for a native coroutine created by an async def in pure Python. A Cython-compiled async def produces a distinct type that implements __await__ but is not a types.CoroutineType, so it passes isawaitable and fails iscoroutine. That is exactly the split you measured.

The current code is:

coro = self.trace_extension(prefix_and_name, info)
if not inspect.iscoroutine(coro):  # pragma: no cover
    raise TypeError(
        "If you're using an asynchronous interface, "
        "the callback of the `trace` extension should "
        "be an asynchronous function rather than a normal function."
    )
await coro

Switching to inspect.isawaitable keeps the guard doing the thing it was written for. The case it exists to catch is someone passing a plain def callback, and a plain function's return value is not awaitable either, so isawaitable still rejects it and still produces the same error message. The only behaviour that changes is that things which are genuinely awaitable stop being rejected, which is the bug you hit.

The # pragma: no cover is worth pointing out as well, since it means no test covers the branch. Nothing in the suite depends on the current semantics, so the change is unlikely to break anything downstream.

One nit on the error message if you do open a PR. If it is going to accept any awaitable, then wording it around "asynchronous function" is slightly off, since a callable returning an awaitable is now fine too. Something like "should return an awaitable" would match the new check more closely.

Worth checking whether _trace.py is the only place with this pattern before submitting, in case the sync path or another module does the same narrow check.

[Yigtwxx]

The atrace half has been covered well above, so I'll add the part nobody has looked at: the same file has the mirror-image check on the sync path, and there your Cython callback fails silently.

Trace.trace():

ret = self.trace_extension(prefix_and_name, info)
if inspect.iscoroutine(ret):  # pragma: no cover
    raise TypeError(
        "If you are using a synchronous interface, "
        "the callback of the `trace` extension should "
        "be a normal function instead of an asynchronous function."
    )

That guard exists to catch someone handing an async callback to the sync interface. It uses the same predicate, so it inherits the same blind spot — but the consequence is inverted. Your Cython callback returns something where iscoroutine is False, the guard doesn't fire, ret is dropped on the floor, and the sync path returns as if the trace succeeded. The callback body never runs. No TypeError, no traceback, just a callback that quietly does nothing. Compared to that, the atrace failure you hit is the friendly one.

So the fix goes the same direction in both places, which is a bit counterintuitive given they're checking for opposite mistakes:

# trace()   — widen, so Cython async callbacks are caught instead of ignored
if inspect.isawaitable(ret):
    raise TypeError(...)

# atrace()  — widen, so Cython async callbacks are accepted instead of rejected
if not inspect.isawaitable(coro):
    raise TypeError(...)

Both still do what the error messages promise. A plain function returning None gives isawaitable(None) is False, so atrace keeps raising and trace keeps staying quiet.

Why isawaitable picks up the Cython object at all is worth stating, since it's what makes the fix load-bearing rather than incidental. inspect.iscoroutine is a bare isinstance(object, types.CoroutineType) — a type identity check on CPython's native coroutine. inspect.isawaitable also accepts isinstance(object, collections.abc.Awaitable), and Cython registers its coroutine type with that ABC. So the widened predicate matches the actual requirement of the await on the next line, which is __await__ and nothing more.

One thing to fold in while touching these lines: in both branches the callback has already been invoked when the raise happens, so the awaitable is discarded un-awaited — that's your coroutine 'trace_callback' was never awaited warning, and after the fix trace() would start producing it on the path that currently raises. Closing it first keeps the failure clean:

if inspect.isawaitable(ret):
    if hasattr(ret, "close"):
        ret.close()
    raise TypeError(...)

Both branches carry # pragma: no cover, so neither is exercised by the suite. If you're opening a PR, a Cython-compiled callback is awkward as a test dependency, but a plain class with __await__ reproduces the exact isawaitable is True, iscoroutine is False split with no build step:

class FakeAwaitable:
    def __await__(self):
        yield from ()
        return None

That covers both branches and pins the behaviour without adding Cython to the test matrix.