classification
Title: Should we really hide unawaited coroutine warnings when an exception is pending?
Type: Stage: resolved
Components: Interpreter Core Versions: Python 3.8, Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: asvetlov, benjamin.peterson, giampaolo.rodola, njs, yselivanov
Priority: normal Keywords:

Created on 2018-01-20 05:30 by njs, last changed 2018-01-29 19:32 by yselivanov. This issue is now closed.

Messages (3)
msg310325 - (view) Author: Nathaniel Smith (njs) * (Python committer) Date: 2018-01-20 05:30
There's a curious bit of code in genobject.c:_PyGen_Finalize:

        if (!error_value) {
            PyErr_WarnFormat(PyExc_RuntimeWarning, 1,
                             "coroutine '%.50S' was never awaited",
                             gen->gi_qualname);
        }

Obviously this is the code that issues "coroutine ... was never awaited" warnings. That's not so curious. What's curious is the 'if' statement: what it does is suppress this warning if -- at the moment the coroutine object was GCed -- there is an active exception propagating.

This was added by bpo-27968, apparently as a way to hide some warnings in test_coroutine.py. This justification seems dubious to me, though, and there doesn't seem to have been much/any scrutiny of this at the time. We can certainly write those tests more carefully, so that they don't issue warnings -- e.g. by using 'with closing(corofn()) as coro: ...'. And this has a much broader effect than on just those tests. For example, say we accidentally write:

   def foo():
       len(corofn())  # should be len(await corofn())

we'll get an error that was caused by the coroutine not being awaited -- and we *won't* get the usual warning message explaining this, because the coroutine object is GCed while the exception propagates. Or, if an unawaited coroutine happens to get stuck in a reference cycle, then it's a matter of luck whether the message gets printed, depending on when exactly the cycle collector happens to trigger.

I guess in most cases where someone forgets an await and it causes errors, the coroutine happens to get stashed into a local variable, so it can't be GCed until after the exception is no longer active. And if another exception becomes active in the mean time, probably it will capture the first one as __context__, so that also keeps the coroutine alive past the time when the warning would be suppressed. So maybe this isn't a huge issue in practice? But this all feels very finicky and accidental, so I wanted to raise the issue for discussion.
msg311068 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-01-29 06:25
So in issue 32703 we consider removing the check.  I'll try to figure out what to do with "coroutine ... was never awaited" warnings in test_coroutines -- probably just silence them.
msg311159 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-01-29 19:32
With PR https://github.com/python/cpython/pull/5410 being merged I believe this issue can be now closed.
History
Date User Action Args
2018-01-29 19:32:53yselivanovsetcomponents: + Interpreter Core, - asyncio
2018-01-29 19:32:43yselivanovsetstatus: open -> closed
resolution: fixed
messages: + msg311159

stage: resolved
2018-01-29 06:25:06yselivanovsetmessages: + msg311068
2018-01-20 05:30:49njscreate