Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add async120, await-in-except #265

Merged
merged 8 commits into from
Jun 12, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions docs/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ Changelog

*[CalVer, YY.month.patch](https://calver.org/)*

24.5.7
======
- Add :ref:`ASYNC120 <async120>` await-in-except.

24.5.6
======
- Make :ref:`ASYNC913 <async913>` disabled by default, as originally intended.
Expand Down
10 changes: 9 additions & 1 deletion docs/rules.rst
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@ ASYNC101 : yield-in-cancel-scope
See `this thread <https://discuss.python.org/t/preventing-yield-inside-certain-context-managers/1091/23>`_ for discussion of a future PEP.
This has substantial overlap with :ref:`ASYNC119 <ASYNC119>`, which will warn on almost all instances of ASYNC101, but ASYNC101 is about a conceptually different problem that will not get resolved by `PEP 533 <https://peps.python.org/pep-0533/>`_.

ASYNC102 : await-in-finally-or-cancelled
_`ASYNC102` : await-in-finally-or-cancelled
``await`` inside ``finally`` or :ref:`cancelled-catching <cancelled>` ``except:`` must have shielded :ref:`cancel scope <cancel_scope>` with timeout.
If not, the async call will immediately raise a new cancellation, suppressing the cancellation that was caught.
See :ref:`ASYNC120 <async120>` for the general case where other exceptions might get suppressed.
This is currently not able to detect asyncio shields.

ASYNC103 : no-reraise-cancelled
Expand Down Expand Up @@ -73,6 +75,12 @@ _`ASYNC119` : yield-in-cm-in-async-gen
``yield`` in context manager in async generator is unsafe, the cleanup may be delayed until ``await`` is no longer allowed.
We strongly encourage you to read `PEP 533 <https://peps.python.org/pep-0533/>`_ and use `async with aclosing(...) <https://docs.python.org/3/library/contextlib.html#contextlib.aclosing>`_, or better yet avoid async generators entirely (see `ASYNC900`_ ) in favor of context managers which return an iterable :ref:`channel/stream/queue <channel_stream_queue>`.

_`ASYNC120` : await-in-except
``await`` inside ``except`` must have shielded :ref:`cancel scope <cancel_scope>` with timeout.
If not, the async call might get cancelled, suppressing the exception that was caught.
This will not trigger when :ref:`ASYNC102 <ASYNC102>` does, and if you don't care about losing non-cancelled exceptions you could disable this rule.
This is currently not able to detect asyncio shields.
jakkdl marked this conversation as resolved.
Show resolved Hide resolved


Blocking sync calls in async functions
======================================
Expand Down
2 changes: 1 addition & 1 deletion flake8_async/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@


# CalVer: YY.month.patch, e.g. first release of July 2022 == "22.7.1"
__version__ = "24.5.6"
__version__ = "24.5.7"
jakkdl marked this conversation as resolved.
Show resolved Hide resolved


# taken from https://github.com/Zac-HD/shed
Expand Down
24 changes: 17 additions & 7 deletions flake8_async/visitors/visitor102.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ class Visitor102(Flake8AsyncVisitor):
"await inside {0.name} on line {0.lineno} must have shielded cancel "
"scope with a timeout."
),
"ASYNC120": (
"await inside {0.name} on line {0.lineno} must have shielded cancel "
"scope with a timeout."
jakkdl marked this conversation as resolved.
Show resolved Hide resolved
),
}

class TrioScope:
Expand Down Expand Up @@ -60,7 +64,11 @@ def async_call_checker(
if self._critical_scope is not None and not any(
cm.has_timeout and cm.shielded for cm in self._trio_context_managers
):
self.error(node, self._critical_scope)
if self._critical_scope.name == "except":
error_code = "ASYNC120"
else:
error_code = "ASYNC102"
self.error(node, self._critical_scope, error_code=error_code)
Zac-HD marked this conversation as resolved.
Show resolved Hide resolved

def is_safe_aclose_call(self, node: ast.Await) -> bool:
return (
Expand Down Expand Up @@ -122,16 +130,18 @@ def visit_Try(self, node: ast.Try):
self.visit_nodes(node.finalbody)

def visit_ExceptHandler(self, node: ast.ExceptHandler):
if self.cancelled_caught:
return
res = critical_except(node)
if res is None:
# if we're inside a critical scope, a nested except should never override that
if self._critical_scope is not None and self._critical_scope.name != "except":
return

self.save_state(node, "_critical_scope", "_trio_context_managers")
self.cancelled_caught = True
self._trio_context_managers = []
self._critical_scope = res

if self.cancelled_caught or (res := critical_except(node)) is None:
self._critical_scope = Statement("except", node.lineno, node.col_offset)
else:
self._critical_scope = res
self.cancelled_caught = True

def visit_Assign(self, node: ast.Assign):
# checks for <scopename>.shield = [True/False]
Expand Down
17 changes: 9 additions & 8 deletions tests/eval_files/async102.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# type: ignore
# ARG --enable=ASYNC102,ASYNC120
# NOASYNCIO # TODO: support asyncio shields
from contextlib import asynccontextmanager

Expand Down Expand Up @@ -170,11 +171,11 @@ async def foo4():
try:
...
except ValueError:
await foo() # safe
await foo() # ASYNC120: 8, Statement("except", lineno-1)
except BaseException:
await foo() # error: 8, Statement("BaseException", lineno-1)
except:
await foo()
await foo() # ASYNC120: 8, Statement("except", lineno-1)
# safe, since BaseException will catch Cancelled
# also fully redundant and will never be executed, but that's for another linter

Expand All @@ -186,7 +187,7 @@ async def foo5():
with trio.CancelScope(deadline=30, shield=True):
await foo() # safe
except:
await foo()
await foo() # ASYNC120: 8, Statement("except", lineno-1)

try:
...
Expand Down Expand Up @@ -237,20 +238,20 @@ async def foo_nested_excepts():
try:
await foo() # error: 12, Statement("BaseException", lineno-3)
except BaseException:
await foo() # error: 12, Statement("BaseException", lineno-1)
await foo() # error: 12, Statement("BaseException", lineno-5)
except:
# unsafe, because we're waiting inside the parent baseexception
await foo() # error: 12, Statement("BaseException", lineno-8)
await foo() # error: 8, Statement("BaseException", lineno-9)
except:
await foo()
await foo() # ASYNC120: 8, Statement("except", lineno-1)
try:
await foo()
await foo() # ASYNC120: 12, Statement("except", lineno-3)
except BaseException:
await foo() # error: 12, Statement("BaseException", lineno-1)
except:
await foo()
await foo()
await foo() # ASYNC120: 12, Statement("except", lineno-1)
await foo() # ASYNC120: 8, Statement("except", lineno-8)
await foo()


Expand Down
Loading