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.