diff --git a/changelog/463.feature.rst b/changelog/463.feature.rst new file mode 100644 index 00000000..8340cc94 --- /dev/null +++ b/changelog/463.feature.rst @@ -0,0 +1,2 @@ +A warning :class:`~pluggy.PluggyTeardownRaisedWarning` is now issued when an old-style hookwrapper raises an exception during teardown. +See the warning documentation for more details. diff --git a/docs/api_reference.rst b/docs/api_reference.rst index 40b27bfe..b14d725d 100644 --- a/docs/api_reference.rst +++ b/docs/api_reference.rst @@ -47,3 +47,15 @@ API Reference .. autoclass:: pluggy.HookimplOpts() :show-inheritance: :members: + + +Warnings +-------- + +Custom warnings generated in some situations such as improper usage or deprecated features. + +.. autoclass:: pluggy.PluggyWarning() + :show-inheritance: + +.. autoclass:: pluggy.PluggyTeardownRaisedWarning() + :show-inheritance: diff --git a/src/pluggy/__init__.py b/src/pluggy/__init__.py index 9d9e873b..2adf9454 100644 --- a/src/pluggy/__init__.py +++ b/src/pluggy/__init__.py @@ -18,6 +18,8 @@ "HookspecMarker", "HookimplMarker", "Result", + "PluggyWarning", + "PluggyTeardownRaisedWarning", ] from ._manager import PluginManager, PluginValidationError @@ -31,3 +33,7 @@ HookimplOpts, HookImpl, ) +from ._warnings import ( + PluggyWarning, + PluggyTeardownRaisedWarning, +) diff --git a/src/pluggy/_callers.py b/src/pluggy/_callers.py index 6498eaed..787f56ba 100644 --- a/src/pluggy/_callers.py +++ b/src/pluggy/_callers.py @@ -3,27 +3,52 @@ """ from __future__ import annotations +import warnings from typing import cast from typing import Generator from typing import Mapping +from typing import NoReturn from typing import Sequence from typing import Tuple from typing import Union from ._hooks import HookImpl -from ._result import _raise_wrapfail from ._result import HookCallError from ._result import Result +from ._warnings import PluggyTeardownRaisedWarning # Need to distinguish between old- and new-style hook wrappers. -# Wrapping one a singleton tuple is the fastest type-safe way I found to do it. +# Wrapping with a tuple is the fastest type-safe way I found to do it. Teardown = Union[ - Tuple[Generator[None, Result[object], None]], + Tuple[Generator[None, Result[object], None], HookImpl], Generator[None, object, object], ] +def _raise_wrapfail( + wrap_controller: ( + Generator[None, Result[object], None] | Generator[None, object, object] + ), + msg: str, +) -> NoReturn: + co = wrap_controller.gi_code + raise RuntimeError( + "wrap_controller at %r %s:%d %s" + % (co.co_name, co.co_filename, co.co_firstlineno, msg) + ) + + +def _warn_teardown_exception( + hook_name: str, hook_impl: HookImpl, e: BaseException +) -> None: + msg = "A plugin raised an exception during an old-style hookwrapper teardown.\n" + msg += f"Plugin: {hook_impl.plugin_name}, Hook: {hook_name}\n" + msg += f"{type(e).__name__}: {e}\n" + msg += "For more information see https://pluggy.readthedocs.io/en/stable/api_reference.html#pluggy.PluggyTeardownRaisedWarning" # noqa: E501 + warnings.warn(PluggyTeardownRaisedWarning(msg), stacklevel=5) + + def _multicall( hook_name: str, hook_impls: Sequence[HookImpl], @@ -60,7 +85,7 @@ def _multicall( res = hook_impl.function(*args) wrapper_gen = cast(Generator[None, Result[object], None], res) next(wrapper_gen) # first yield - teardowns.append((wrapper_gen,)) + teardowns.append((wrapper_gen, hook_impl)) except StopIteration: _raise_wrapfail(wrapper_gen, "did not yield") elif hook_impl.wrapper: @@ -128,9 +153,13 @@ def _multicall( if isinstance(teardown, tuple): try: teardown[0].send(outcome) - _raise_wrapfail(teardown[0], "has second yield") except StopIteration: pass + except BaseException as e: + _warn_teardown_exception(hook_name, teardown[1], e) + raise + else: + _raise_wrapfail(teardown[0], "has second yield") else: try: if outcome._exception is not None: diff --git a/src/pluggy/_result.py b/src/pluggy/_result.py index 29859eb9..aa21fa13 100644 --- a/src/pluggy/_result.py +++ b/src/pluggy/_result.py @@ -7,9 +7,7 @@ from typing import Callable from typing import cast from typing import final -from typing import Generator from typing import Generic -from typing import NoReturn from typing import Optional from typing import Tuple from typing import Type @@ -20,19 +18,6 @@ ResultType = TypeVar("ResultType") -def _raise_wrapfail( - wrap_controller: ( - Generator[None, Result[ResultType], None] | Generator[None, object, object] - ), - msg: str, -) -> NoReturn: - co = wrap_controller.gi_code - raise RuntimeError( - "wrap_controller at %r %s:%d %s" - % (co.co_name, co.co_filename, co.co_firstlineno, msg) - ) - - class HookCallError(Exception): """Hook was called incorrectly.""" diff --git a/src/pluggy/_warnings.py b/src/pluggy/_warnings.py new file mode 100644 index 00000000..6356c770 --- /dev/null +++ b/src/pluggy/_warnings.py @@ -0,0 +1,27 @@ +from typing import final + + +class PluggyWarning(UserWarning): + """Base class for all warnings emitted by pluggy.""" + + __module__ = "pluggy" + + +@final +class PluggyTeardownRaisedWarning(PluggyWarning): + """A plugin raised an exception during an :ref:`old-style hookwrapper + ` teardown. + + Such exceptions are not handled by pluggy, and may cause subsequent + teardowns to be executed at unexpected times, or be skipped entirely. + + This is an issue in the plugin implementation. + + If the exception is unintended, fix the underlying cause. + + If the exception is intended, switch to :ref:`new-style hook wrappers + `, or use :func:`result.force_exception() + ` to set the exception instead of raising. + """ + + __module__ = "pluggy" diff --git a/testing/test_warnings.py b/testing/test_warnings.py new file mode 100644 index 00000000..4f5454be --- /dev/null +++ b/testing/test_warnings.py @@ -0,0 +1,49 @@ +from pathlib import Path + +import pytest + +from pluggy import HookimplMarker +from pluggy import HookspecMarker +from pluggy import PluggyTeardownRaisedWarning +from pluggy import PluginManager + + +hookspec = HookspecMarker("example") +hookimpl = HookimplMarker("example") + + +def test_teardown_raised_warning(pm: PluginManager) -> None: + class Api: + @hookspec + def my_hook(self): + raise NotImplementedError() + + pm.add_hookspecs(Api) + + class Plugin1: + @hookimpl + def my_hook(self): + pass + + class Plugin2: + @hookimpl(hookwrapper=True) + def my_hook(self): + yield + 1 / 0 + + class Plugin3: + @hookimpl(hookwrapper=True) + def my_hook(self): + yield + + pm.register(Plugin1(), "plugin1") + pm.register(Plugin2(), "plugin2") + pm.register(Plugin3(), "plugin3") + with pytest.warns( + PluggyTeardownRaisedWarning, + match=r"\bplugin2\b.*\bmy_hook\b.*\n.*ZeroDivisionError", + ) as wc: + with pytest.raises(ZeroDivisionError): + pm.hook.my_hook() + assert len(wc.list) == 1 + assert Path(wc.list[0].filename).name == "test_warnings.py"