refactor: implement legacy hookwrappers in terms of a modern hook"

changelog/544.bugfix.rst

@@ -0,0 +1,6 @@
+Correctly pass :class:`StopIteration` trough hook wrappers.
+
+Raising a :class:`StopIteration` in a generator triggers a :class:`RuntimeError`.
+
+If the :class:`RuntimeError` of a generator has the passed in :class:`StopIteration` as cause
+resume with that :class:`StopIteration` as normal exception instead of failing with the :class:`RuntimeError`.

src/pluggy/_callers.py

@@ -9,8 +9,6 @@
 from typing import Mapping
 from typing import NoReturn
 from typing import Sequence
-from typing import Tuple
-from typing import Union
 import warnings
 
 from ._hooks import HookImpl
@@ -21,10 +19,34 @@
 
 # Need to distinguish between old- and new-style hook wrappers.
 # Wrapping with a tuple is the fastest type-safe way I found to do it.
-Teardown = Union[
-    Tuple[Generator[None, Result[object], None], HookImpl],
-    Generator[None, object, object],
-]
+Teardown = Generator[None, object, object]
+
+
+def run_legacy_hookwrapper(
+    hook_impl: HookImpl, hook_name: str, args: Sequence[object]
+) -> Teardown:
+    teardown: Teardown = cast(Teardown, hook_impl.function(*args))
+    try:
+        next(teardown)
+    except StopIteration:
+        _raise_wrapfail(teardown, "did not yield")
+    try:
+        res = yield
+        result = Result(res, None)
+    except BaseException as exc:
+        result = Result(None, exc)
+    try:
+        teardown.send(result)
+    except StopIteration:
+        pass
+    except BaseException as e:
+        _warn_teardown_exception(hook_name, hook_impl, e)
+        raise
+    else:
+        _raise_wrapfail(teardown, "has second yield")
+    finally:
+        teardown.close()
+    return result.get_result()
 
 
 def _raise_wrapfail(
@@ -47,7 +69,7 @@
     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)
+    warnings.warn(PluggyTeardownRaisedWarning(msg), stacklevel=6)
 
 
 def _multicall(
@@ -64,7 +86,6 @@
     __tracebackhide__ = True
     results: list[object] = []
     exception = None
-    only_new_style_wrappers = True
     try:  # run impl and wrapper setup functions in a loop
         teardowns: list[Teardown] = []
         try:
@@ -79,16 +100,17 @@
                             )
 
                 if hook_impl.hookwrapper:
-                    only_new_style_wrappers = False
                     try:
                         # If this cast is not valid, a type error is raised below,
                         # which is the desired response.
-                        res = hook_impl.function(*args)
-                        wrapper_gen = cast(Generator[None, Result[object], None], res)
-                        next(wrapper_gen)  # first yield
-                        teardowns.append((wrapper_gen, hook_impl))
+                        function_gen = run_legacy_hookwrapper(
+                            hook_impl, hook_name, args
+                        )
+
+                        next(function_gen)  # first yield
+                        teardowns.append(function_gen)
                     except StopIteration:
-                        _raise_wrapfail(wrapper_gen, "did not yield")
+                        _raise_wrapfail(function_gen, "did not yield")
                 elif hook_impl.wrapper:
                     try:
                         # If this cast is not valid, a type error is raised below,
@@ -108,75 +130,44 @@
         except BaseException as exc:
             exception = exc
     finally:
-        # Fast path - only new-style wrappers, no Result.
-        if only_new_style_wrappers:
-            if firstresult:  # first result hooks return a single value
-                result = results[0] if results else None
-            else:
-                result = results
-
-            # run all wrapper post-yield blocks
-            for teardown in reversed(teardowns):
-                try:
-                    if exception is not None:
-                        teardown.throw(exception)  # type: ignore[union-attr]
-                    else:
-                        teardown.send(result)  # type: ignore[union-attr]
-                    # Following is unreachable for a well behaved hook wrapper.
-                    # Try to force finalizers otherwise postponed till GC action.
-                    # Note: close() may raise if generator handles GeneratorExit.
-                    teardown.close()  # type: ignore[union-attr]
-                except StopIteration as si:
-                    result = si.value
-                    exception = None
-                    continue
-                except BaseException as e:
-                    exception = e
-                    continue
-                _raise_wrapfail(teardown, "has second yield")  # type: ignore[arg-type]
-
-            if exception is not None:
-                raise exception
-            else:
-                return result
-
-        # Slow path - need to support old-style wrappers.
+        if firstresult:  # first result hooks return a single value
+            result = results[0] if results else None
         else:
-            if firstresult:  # first result hooks return a single value
-                outcome: Result[object | list[object]] = Result(
-                    results[0] if results else None, exception
-                )
-            else:
-                outcome = Result(results, exception)
-
-            # run all wrapper post-yield blocks
-            for teardown in reversed(teardowns):
-                if isinstance(teardown, tuple):
-                    try:
-                        teardown[0].send(outcome)
-                    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:
+            result = results
+
+        # run all wrapper post-yield blocks
+        for teardown in reversed(teardowns):
+            try:
+                if exception is not None:
                     try:
-                        if outcome._exception is not None:
-                            teardown.throw(outcome._exception)
+                        teardown.throw(exception)
+                    except RuntimeError as re:
+                        # StopIteration from generator causes RuntimeError
+                        # even for coroutine usage - see #544
+                        if (
+                            isinstance(exception, StopIteration)
+                            and re.__cause__ is exception
+                        ):
+                            teardown.close()
+                            continue
                         else:
-                            teardown.send(outcome._result)
-                        # Following is unreachable for a well behaved hook wrapper.
-                        # Try to force finalizers otherwise postponed till GC action.
-                        # Note: close() may raise if generator handles GeneratorExit.
-                        teardown.close()
-                    except StopIteration as si:
-                        outcome.force_result(si.value)
-                        continue
-                    except BaseException as e:
-                        outcome.force_exception(e)
-                        continue
-                    _raise_wrapfail(teardown, "has second yield")
-
-            return outcome.get_result()
+                            raise
+                else:
+                    teardown.send(result)
+                # Following is unreachable for a well behaved hook wrapper.
+                # Try to force finalizers otherwise postponed till GC action.
+                # Note: close() may raise if generator handles GeneratorExit.
+                teardown.close()
+            except StopIteration as si:
+                result = si.value
+                exception = None
+                continue
+            except BaseException as e:
+                exception = e
+                continue
+            _raise_wrapfail(teardown, "has second yield")
+
+    if exception is not None:
+        raise exception
+    else:
+        return result

testing/test_multicall.py

@@ -416,6 +416,36 @@ def m2():
     ]
 
 
+@pytest.mark.parametrize("has_hookwrapper", [True, False])
+def test_wrapper_stopiteration_passtrough(has_hookwrapper: bool) -> None:
+    out = []
+
+    @hookimpl(wrapper=True)
+    def wrap():
+        out.append("wrap")
+        try:
+            yield
+        finally:
+            out.append("wrap done")
+
+    @hookimpl(wrapper=not has_hookwrapper, hookwrapper=has_hookwrapper)
+    def wrap_path2():
+        yield
+
+    @hookimpl
+    def stop():
+        out.append("stop")
+        raise StopIteration
+
+    with pytest.raises(StopIteration):
+        try:
+            MC([stop, wrap, wrap_path2], {})
+        finally:
+            out.append("finally")
+
+    assert out == ["wrap", "stop", "wrap done", "finally"]
+
+
 def test_suppress_inner_wrapper_teardown_exc() -> None:
     out = []