Skip to content

Commit

Permalink
Catch errors and log in _dispose in PluginContext (#325)
Browse files Browse the repository at this point in the history
When running the dispose functions in `_dispose` in `PluginContext`,
catch errors and log instead of raising.

Suggested by Talley in:
pyapp-kit/app-model#147 (comment)

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Nathan Clack <nclack@chanzuckerberg.com>
  • Loading branch information
3 people authored Nov 27, 2023
1 parent 6612e8c commit f9f35ea
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 1 deletion.
8 changes: 7 additions & 1 deletion src/npe2/_plugin_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from collections import Counter
from fnmatch import fnmatch
from importlib import metadata
from logging import getLogger
from pathlib import Path
from typing import (
TYPE_CHECKING,
Expand Down Expand Up @@ -52,6 +53,8 @@
InclusionSet = Union[AbstractSetIntStr, MappingIntStrAny, None]
DisposeFunction = Callable[[], None]

logger = getLogger(__name__)

__all__ = ["PluginContext", "PluginManager"]
PluginName = str # this is `PluginManifest.name`

Expand Down Expand Up @@ -710,7 +713,10 @@ def __init__(

def _dispose(self):
while self._disposables:
self._disposables.pop()()
try:
self._disposables.pop()()
except Exception as e:
logger.warning(f"Error while disposing {self.plugin_key}; {e}")

def register_command(self, id: str, command: Optional[Callable] = None):
"""Associate a callable with a command id."""
Expand Down
14 changes: 14 additions & 0 deletions tests/test_plugin_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -222,3 +222,17 @@ def test_plugin_context_dispose():
pm.get_context("test").register_disposable(mock)
pm.deactivate("test")
mock.assert_called_once()


def test_plugin_context_dispose_error(caplog):
"""Test errors when executing dispose functions caught correctly."""
pm = PluginManager()
mf = PluginManifest(name="test")
pm.register(mf)

def dummy_error():
raise ValueError("This is an error")

pm.get_context("test").register_disposable(dummy_error)
pm.deactivate("test")
assert caplog.records[0].msg == "Error while disposing test; This is an error"

0 comments on commit f9f35ea

Please sign in to comment.