Feature Suggestion: Collect multiple exceptions from hook implementations

[MrGreenTea]

Currently, if one hook implementation raises an exception the rest of the implementations of that hook won't be called. This is fine in most cases, but we have some hooks for teardowns of operations and these should always be called and pytest could also have a use for this (see pytest#8217)

Our current workaround is a patch on the internal pluggy implementation to make sure all hook implementations are called and gathering all exceptions, then raising them together. This hack could form a basis for a more correct implementation and I'd be willing to work on a PR for this feature, should you accept the suggestion.

I believe it could work as a mode in the hookspec, so look something like this:

@hookspec(force_all_impls=True)  # I don't have a good name for this yet
def example_teardown():
    ...

With the addition of exception groups to python I believe this can work quite nicely.

I'm open to all feedback and think this feature needs a more in-depth discussion then I provided as of yet, but wanted to get this discussion started.

[hoefling]

If the general idea is approved by pluggy maintainers, my humble suggestion on the naming is firsterror, similar to firstresult, with defaulting to True to not break existing plugins.

[RonnyPfannschmidt]

Im under the impression you want signal,not Hook

[RonnyPfannschmidt]

I'm going to go further and declare that's Out off scope for the Design and usage of pluggy

So from My pov it's not Worth the added complexity

[MrGreenTea]

I'm going to go further and declare that's Out off scope for the Design and usage of pluggy

So from My pov it's not Worth the added complexity

Well this is unfortunate as I hoped we could have a discussion about this as it is a feature I've needed for quite some time and the workarounds we have break with most pluggy releases.

I was also personally surprised that this is not yet a feature of pluggy, as I expected the hooks to not be dependent on the correct implementation of other plugins.

Im under the impression you want signal,not Hook

What signals do you mean exactly, the signal module? How would signals solve the problem over hooks?

For some more context:

Think of the pytest_runtest_teardown hook. If any plugin' hook implementation would raise an exception other teardown implementations might not be called. pytest makes sure all fixtures are torn down by using a similar approach as I suggest here. Using atexit handlers would leave all the teardown until all tests have run through, which might be wasteful if a lot of data can't be garbace collected.

If you're not at all interested in this feature and actively against it I will close this discussion and issue, but I hope I could make my case a bit more clear.

[RonnyPfannschmidt]

From the pluggy design perspective,a Hook that's raising a error is a hard failure to propagate asap

[hoefling]

That's unfortunate, we hoped for smth like

pm = pluggy.PluginManager("myapp")
...
try:
    pm.hook.myhook()
except* Exception as e:
    ...  # grab all errors raised by plugins

so the first faulty plugin does not abort the multicall.

[nicoddemus]

I'm not so convinced that this out of scope for pluggy TBH.

@RonnyPfannschmidt can you detail why this is a no-go/out of scope for pluggy? Detailing it would also serve as documentation why this is undersirable/out of scope for the future.

[RonnyPfannschmidt]

Currently hooks are doing direct, linear extension with direct error propagations, the request is basically for the addition of a scatter/gathering ,

This in essence adds concurrency as a entirely new patterns as well as native exception groups

In turn this will also create an eventual need to provide both results and exception

As of now id consider it quite a error prone burden to add concurrency to pluggy

[RonnyPfannschmidt]

closing this to be solved as part of #321

[MrGreenTea]

Currently hooks are doing direct, linear extension with direct error propagations, the request is basically for the addition of a scatter/gathering ,

This in essence adds concurrency as a entirely new patterns as well as native exception groups

In turn this will also create an eventual need to provide both results and exception

As of now id consider it quite a error prone burden to add concurrency to pluggy

Another way to consider this is how first_result works. If we look at errors/exceptions as a type of return value similar to the way some type systems do (Haskell, Rust, ...) it seems to me a very straight-forward extension of the current feature set where we might call all hooks and gather their "normal" return values.

From a design perspective it would not have to be exception groups, it could just use them as they've been added for cases like this where multiple errors might occur.

I see what you mean by the eventual need for mixed results and exceptions. Right now it works without this as well. If I have three hooks, the first two return some value and the third fails with an exception, I also do not get the results but just an exception.

Thank you for listening to this and integrating it into #321. I'll keep a close look at that and thanks again for all your work on this very helpful project :)