Skip to content

Commit 55214f0

Browse files
Replace complex _multicall wrapper logic with streamlined completion hook pattern
The _multicall function had intricate generator-based wrapper teardown logic that was difficult to understand and maintain. This refactoring introduces a completion hook pattern that simplifies the wrapper invocation process. Key changes: - Add CompletionHook type alias with (result, exception) -> (result, exception) signature - Implement setup_and_get_completion_hook() method in WrapperImpl class - Unify old-style and new-style wrapper handling using run_old_style_hookwrapper adapter - Replace complex teardown loop with simple completion hook calls - Adjust warning stacklevel from 6 to 7 for correct source attribution The completion hook pattern makes wrapper teardown explicit and easier to debug while maintaining full backward compatibility with both hookwrapper and wrapper implementations. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
1 parent f8b44b2 commit 55214f0

File tree

2 files changed

+114
-71
lines changed

2 files changed

+114
-71
lines changed

src/pluggy/_callers.py

Lines changed: 35 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ def _warn_teardown_exception(
7070
msg += f"Plugin: {hook_impl.plugin_name}, Hook: {hook_name}\n"
7171
msg += f"{type(e).__name__}: {e}\n"
7272
msg += "For more information see https://pluggy.readthedocs.io/en/stable/api_reference.html#pluggy.PluggyTeardownRaisedWarning" # noqa: E501
73-
warnings.warn(PluggyTeardownRaisedWarning(msg), stacklevel=6)
73+
warnings.warn(PluggyTeardownRaisedWarning(msg), stacklevel=7)
7474

7575

7676
def _multicall(
@@ -89,76 +89,40 @@ def _multicall(
8989
results: list[object] = []
9090
exception = None
9191

92-
try: # run impl and wrapper setup functions in a loop
93-
teardowns: list[Teardown] = []
94-
try:
95-
for hook_impl in reversed(wrapper_impls):
96-
args = hook_impl._get_call_args(caller_kwargs)
97-
98-
if hook_impl.hookwrapper:
99-
function_gen = run_old_style_hookwrapper(hook_impl, hook_name, args)
100-
next(function_gen) # first yield
101-
teardowns.append(function_gen)
102-
elif hook_impl.wrapper:
103-
try:
104-
# If this cast is not valid, a type error is raised below,
105-
# which is the desired response.
106-
res = hook_impl.function(*args)
107-
function_gen = cast(Generator[None, object, object], res)
108-
next(function_gen) # first yield
109-
teardowns.append(function_gen)
110-
except StopIteration:
111-
_raise_wrapfail(function_gen, "did not yield")
112-
113-
# Process normal implementations (in reverse order for correct execution)
114-
# Caller ensures normal_impls contains only non-wrapper implementations
115-
for normal_impl in reversed(normal_impls):
116-
args = normal_impl._get_call_args(caller_kwargs)
117-
118-
res = normal_impl.function(*args)
119-
if res is not None:
120-
results.append(res)
121-
if firstresult: # halt further impl calls
122-
break
123-
except BaseException as exc:
124-
exception = exc
125-
finally:
126-
if firstresult: # first result hooks return a single value
127-
result = results[0] if results else None
128-
else:
129-
result = results
130-
131-
# run all wrapper post-yield blocks
132-
for teardown in reversed(teardowns):
133-
try:
134-
if exception is not None:
135-
try:
136-
teardown.throw(exception)
137-
except RuntimeError as re:
138-
# StopIteration from generator causes RuntimeError
139-
# even for coroutine usage - see #544
140-
if (
141-
isinstance(exception, StopIteration)
142-
and re.__cause__ is exception
143-
):
144-
teardown.close()
145-
continue
146-
else:
147-
raise
148-
else:
149-
teardown.send(result)
150-
# Following is unreachable for a well behaved hook wrapper.
151-
# Try to force finalizers otherwise postponed till GC action.
152-
# Note: close() may raise if generator handles GeneratorExit.
153-
teardown.close()
154-
except StopIteration as si:
155-
result = si.value
156-
exception = None
157-
continue
158-
except BaseException as e:
159-
exception = e
160-
continue
161-
_raise_wrapfail(teardown, "has second yield")
92+
# Set up wrapper completion hooks
93+
from ._hook_callers import CompletionHook
94+
95+
completion_hooks: list[CompletionHook] = []
96+
97+
try:
98+
# Set up all wrappers and collect their completion hooks
99+
for wrapper_impl in reversed(wrapper_impls):
100+
completion_hook = wrapper_impl.setup_and_get_completion_hook(
101+
hook_name, caller_kwargs
102+
)
103+
completion_hooks.append(completion_hook)
104+
105+
# Process normal implementations (in reverse order for correct execution)
106+
# Caller ensures normal_impls contains only non-wrapper implementations
107+
for normal_impl in reversed(normal_impls):
108+
args = normal_impl._get_call_args(caller_kwargs)
109+
res = normal_impl.function(*args)
110+
if res is not None:
111+
results.append(res)
112+
if firstresult: # halt further impl calls
113+
break
114+
except BaseException as exc:
115+
exception = exc
116+
117+
# Determine final result before teardown
118+
if firstresult: # first result hooks return a single value
119+
result = results[0] if results else None
120+
else:
121+
result = results
122+
123+
# Run completion hooks in reverse order (LIFO - Last In, First Out)
124+
for completion_hook in reversed(completion_hooks):
125+
result, exception = completion_hook(result, exception)
162126

163127
if exception is not None:
164128
raise exception

src/pluggy/_hook_callers.py

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
from __future__ import annotations
66

7+
from collections.abc import Generator
78
from collections.abc import Mapping
89
from collections.abc import MutableSequence
910
from collections.abc import Sequence
@@ -33,6 +34,12 @@
3334

3435
_T_HookImpl = TypeVar("_T_HookImpl", bound="HookImpl")
3536

37+
# Type alias for completion hook functions
38+
CompletionHook = Callable[
39+
[object | list[object] | None, BaseException | None],
40+
tuple[object | list[object] | None, BaseException | None],
41+
]
42+
3643

3744
def _insert_hookimpl_into_list(
3845
hookimpl: _T_HookImpl, target_list: MutableSequence[_T_HookImpl]
@@ -738,3 +745,75 @@ def __init__(
738745
"Use HookImpl for normal implementations."
739746
)
740747
super().__init__(plugin, plugin_name, function, hook_impl_config)
748+
749+
def setup_and_get_completion_hook(
750+
self, hook_name: str, caller_kwargs: Mapping[str, object]
751+
) -> CompletionHook:
752+
"""Set up wrapper and return a completion hook for teardown processing.
753+
754+
This method provides a streamlined way to handle wrapper setup and teardown.
755+
Both old-style hookwrappers and new-style wrappers are handled uniformly
756+
by converting old-style wrappers to the new protocol using
757+
run_old_style_hookwrapper.
758+
759+
Args:
760+
hook_name: Name of the hook being called
761+
caller_kwargs: Keyword arguments passed to the hook call
762+
763+
Returns:
764+
A completion hook function that handles the teardown process
765+
"""
766+
args = self._get_call_args(caller_kwargs)
767+
768+
# Use run_old_style_hookwrapper for old-style, direct generator for new-style
769+
if self.hookwrapper:
770+
from ._callers import run_old_style_hookwrapper
771+
772+
wrapper_gen = run_old_style_hookwrapper(self, hook_name, args)
773+
else:
774+
# New-style wrapper handling
775+
res = self.function(*args)
776+
wrapper_gen = cast(Generator[None, object, object], res)
777+
778+
# Start the wrapper generator - this is where "did not yield" is checked
779+
try:
780+
next(wrapper_gen) # first yield
781+
except StopIteration:
782+
from ._callers import _raise_wrapfail
783+
784+
_raise_wrapfail(wrapper_gen, "did not yield")
785+
786+
def completion_hook(
787+
result: object | list[object] | None, exception: BaseException | None
788+
) -> tuple[object | list[object] | None, BaseException | None]:
789+
"""Unified completion hook for both old-style and new-style wrappers."""
790+
try:
791+
if exception is not None:
792+
try:
793+
wrapper_gen.throw(exception)
794+
except RuntimeError as re:
795+
# StopIteration from generator causes RuntimeError
796+
# even for coroutine usage - see #544
797+
if (
798+
isinstance(exception, StopIteration)
799+
and re.__cause__ is exception
800+
):
801+
wrapper_gen.close()
802+
return result, exception
803+
else:
804+
raise
805+
else:
806+
wrapper_gen.send(result)
807+
# Following is unreachable for a well behaved hook wrapper.
808+
# Try to force finalizers otherwise postponed till GC action.
809+
# Note: close() may raise if generator handles GeneratorExit.
810+
wrapper_gen.close()
811+
from ._callers import _raise_wrapfail
812+
813+
_raise_wrapfail(wrapper_gen, "has second yield")
814+
except StopIteration as si:
815+
return si.value, None
816+
except BaseException as e:
817+
return result, e
818+
819+
return completion_hook

0 commit comments

Comments
 (0)