Skip to content

Commit

Permalink
result: add result.force_exception API
Browse files Browse the repository at this point in the history
Hookwrappers do not currently provide a public, proper way to override
or force an exception. Raising directly from the hookwrapper causes
further wrappers to be skipped, but this is not something we want to
change at this point, for fear of breaking things, and because we are
adding a replacement. See #244 on this issue.
  • Loading branch information
bluetech committed Jun 12, 2023
1 parent 4048f8f commit 195289b
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 7 deletions.
5 changes: 5 additions & 0 deletions changelog/394.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Added the :meth:`~pluggy._callers._Result.force_exception` method to ``_Result``.

``force_exception`` allows hookwrappers to force an exception or override/adjust an existing exception of a hook invocation,
in a properly behaving manner. Using ``force_exception`` is preferred over raising an exception from the hookwrapper,
because raising an exception causes other hookwrappers to be skipped.
1 change: 1 addition & 0 deletions docs/api_reference.rst
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ API Reference
.. autoclass:: pluggy._callers._Result
.. automethod:: pluggy._callers._Result.get_result
.. automethod:: pluggy._callers._Result.force_result
.. automethod:: pluggy._callers._Result.force_exception

.. autoclass:: pluggy._hooks._HookCaller
.. automethod:: pluggy._hooks._HookCaller.call_extra
Expand Down
27 changes: 20 additions & 7 deletions docs/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,13 @@ be implemented as generator function with a single ``yield`` in its body:
# all corresponding hookimpls are invoked here
outcome = yield
for item in outcome.get_result():
try:
result = outcome.get_result()
except BaseException as e:
outcome.force_exception(e)
return
for item in result:
print("JSON config override is {}".format(item))
if config.debug:
Expand All @@ -397,14 +403,21 @@ be implemented as generator function with a single ``yield`` in its body:
outcome.force_result(defaults)
The generator is :py:meth:`sent <python:generator.send>` a :py:class:`pluggy._callers._Result` object which can
be assigned in the ``yield`` expression and used to override or inspect
the final result(s) returned back to the caller using the
:py:meth:`~pluggy._callers._Result.force_result` or
:py:meth:`~pluggy._callers._Result.get_result` methods.
be assigned in the ``yield`` expression and used to inspect
the final result(s) or exceptions returned back to the caller using the
:py:meth:`~pluggy._callers._Result.get_result` method, override the result
using the :py:meth:`~pluggy._callers._Result.force_result`, or override
the exception using the :py:meth:`~pluggy._callers._Result.force_exception`
method.

.. note::
Hook wrappers can **not** return results (as per generator function
semantics); they can only modify them using the ``_Result`` API.
Hookwrappers can **not** return results; they can only modify them using
the :py:meth:`~pluggy._callers._Result.force_result` API.

Hookwrappers should **not** raise exceptions; this will cause further
hookwrappers to be skipped. They should use
:py:meth:`~pluggy._callers._Result.force_exception` to adjust the
exception.

Also see the :ref:`pytest:hookwrapper` section in the ``pytest`` docs.

Expand Down
12 changes: 12 additions & 0 deletions src/pluggy/_result.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,22 @@ def force_result(self, result: _T) -> None:
If the hook was marked as a ``firstresult`` a single value should
be set, otherwise set a (modified) list of results. Any exceptions
found during invocation will be deleted.
This overrides any previous result or exception.
"""
self._result = result
self._exception = None

def force_exception(self, exception: BaseException) -> None:
"""Force the result to fail with ``exception``.
This overrides any previous result or exception.
.. versionadded:: 1.1.0
"""
self._result = None
self._exception = exception

def get_result(self) -> _T:
"""Get the result(s) for this hook call.
Expand Down
49 changes: 49 additions & 0 deletions testing/test_multicall.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,3 +161,52 @@ def m2():
with pytest.raises(exc):
MC([m2, m1], {})
assert out == ["m1 init", "m1 finish"]


def test_hookwrapper_force_exception() -> None:
out = []

@hookimpl(hookwrapper=True)
def m1():
out.append("m1 init")
result = yield
try:
result.get_result()
except BaseException as exc:
result.force_exception(exc)
out.append("m1 finish")

@hookimpl(hookwrapper=True)
def m2():
out.append("m2 init")
result = yield
try:
result.get_result()
except BaseException as exc:
new_exc = OSError("m2")
new_exc.__cause__ = exc
result.force_exception(new_exc)
out.append("m2 finish")

@hookimpl(hookwrapper=True)
def m3():
out.append("m3 init")
yield
out.append("m3 finish")

@hookimpl
def m4():
raise ValueError("m4")

with pytest.raises(OSError, match="m2") as excinfo:
MC([m4, m3, m2, m1], {})
assert out == [
"m1 init",
"m2 init",
"m3 init",
"m3 finish",
"m2 finish",
"m1 finish",
]
assert excinfo.value.__cause__ is not None
assert str(excinfo.value.__cause__) == "m4"

0 comments on commit 195289b

Please sign in to comment.