Skip to content

Commit 9edf05e

Browse files
committed
feat: allow configure behavior on temporary check errors
Instead of blindly treating temporarily failing checks as activity, let the user decide how to treat such an error. There are valid cases such as a temporarily sleeping device that should not cause autosuspend to leave the system running. Error behavior is configured in set_up_checks instead of the typical __init__ pattern to prevent all check classes from needing a modification. Fixes: #814
1 parent c06ca54 commit 9edf05e

5 files changed

Lines changed: 154 additions & 2 deletions

File tree

doc/source/configuration_file.inc

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,18 @@ For each check, these generic options can be specified:
111111
Needs to be ``true`` for a check to actually execute.
112112
``false`` is assumed if not specified.
113113

114+
.. option:: error_behavior
115+
116+
Determines how a temporary error raised by this check is handled.
117+
118+
``active``
119+
Treat the error as detected activity, preventing the system from suspending until the check succeeds again.
120+
121+
``ignore``
122+
Log the error and treat the check as if no activity was detected.
123+
124+
Default: ``active``.
125+
114126
Furthermore, each check might have custom options.
115127

116128
Wake up check configuration

src/autosuspend/__init__.py

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,14 @@
1919
from dbus.mainloop.glib import DBusGMainLoop
2020
from gi.repository import GLib
2121

22-
from .checks import Activity, CheckType, ConfigurationError, TemporaryCheckError, Wakeup
22+
from .checks import (
23+
Activity,
24+
CheckType,
25+
ConfigurationError,
26+
ErrorBehavior,
27+
TemporaryCheckError,
28+
Wakeup,
29+
)
2330
from .config import GENERAL_PARAMETERS, ConfigSchema
2431
from .util import logger_by_class_instance
2532
from .util.systemd import LogindDBusException, has_inhibit_lock
@@ -121,6 +128,11 @@ def _safe_execute_activity(check: Activity, logger: logging.Logger) -> str | Non
121128
try:
122129
return check.check()
123130
except TemporaryCheckError:
131+
if check.error_behavior is ErrorBehavior.IGNORE:
132+
logger.warning(
133+
"Check %s failed. Ignoring as configured...", check, exc_info=True
134+
)
135+
return None
124136
logger.warning("Check %s failed. Ignoring...", check, exc_info=True)
125137
return f"Check {check.name} failed temporarily"
126138

@@ -451,6 +463,17 @@ def _determine_check_class_name(name: str, section: configparser.SectionProxy) -
451463
return section.get("class", name)
452464

453465

466+
def _determine_error_behavior(section: configparser.SectionProxy) -> ErrorBehavior:
467+
raw = section.get("error_behavior", fallback=ErrorBehavior.ACTIVE.value)
468+
try:
469+
return ErrorBehavior(raw)
470+
except ValueError as error:
471+
raise ConfigurationError(
472+
f"Invalid value for option 'error_behavior': {raw!r}. "
473+
f"Allowed values: {[e.value for e in ErrorBehavior]}"
474+
) from error
475+
476+
454477
def _set_up_single_check(
455478
section: configparser.SectionProxy,
456479
prefix: str,
@@ -486,6 +509,8 @@ def _set_up_single_check(
486509
raise ConfigurationError(
487510
"Check %s is not a correct %s instance", check, target_class.__name__
488511
)
512+
if isinstance(check, Activity):
513+
check.error_behavior = _determine_error_behavior(section)
489514
_logger.debug("Created check instance %s with options %s", check, check.options())
490515

491516
return check

src/autosuspend/checks/__init__.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import configparser
55
from collections.abc import Mapping
66
from datetime import datetime
7+
from enum import Enum
78
from typing import Any, Self, TypeVar
89

910
from autosuspend.config import ParameterSchemaAware
@@ -29,6 +30,13 @@ class SevereCheckError(RuntimeError):
2930
"""
3031

3132

33+
class ErrorBehavior(Enum):
34+
"""Configures how a :class:`TemporaryCheckError` is handled for a check."""
35+
36+
ACTIVE = "active"
37+
IGNORE = "ignore"
38+
39+
3240
CheckType = TypeVar("CheckType", bound="Check")
3341

3442

@@ -85,6 +93,10 @@ class Activity(Check):
8593
Subclasses must call this class' __init__ method.
8694
"""
8795

96+
def __init__(self, name: str | None = None) -> None:
97+
super().__init__(name)
98+
self.error_behavior = ErrorBehavior.ACTIVE
99+
88100
@abc.abstractmethod
89101
def check(self) -> str | None:
90102
"""Determine if system activity exists that prevents suspending.

tests/test_autosuspend.py

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,80 @@ class = Mpd
114114

115115
mock_class.create.assert_called_once_with("Foo", parser["check.Foo"])
116116

117+
def test_error_behavior_defaults_to_active(self, mocker: MockerFixture) -> None:
118+
mock_class = mocker.patch("autosuspend.checks.activity.Mpd")
119+
check = mocker.MagicMock(spec=autosuspend.checks.Activity)
120+
mock_class.create.return_value = check
121+
122+
parser = configparser.ConfigParser()
123+
parser.read_string("""
124+
[check.Foo]
125+
class = Mpd
126+
enabled = True
127+
""")
128+
129+
autosuspend.set_up_checks(
130+
parser, "check", "activity", autosuspend.Activity # type: ignore
131+
)
132+
133+
assert check.error_behavior == autosuspend.ErrorBehavior.ACTIVE
134+
135+
def test_error_behavior_can_be_set_to_ignore(self, mocker: MockerFixture) -> None:
136+
mock_class = mocker.patch("autosuspend.checks.activity.Mpd")
137+
check = mocker.MagicMock(spec=autosuspend.checks.Activity)
138+
mock_class.create.return_value = check
139+
140+
parser = configparser.ConfigParser()
141+
parser.read_string("""
142+
[check.Foo]
143+
class = Mpd
144+
enabled = True
145+
error_behavior = ignore
146+
""")
147+
148+
autosuspend.set_up_checks(
149+
parser, "check", "activity", autosuspend.Activity # type: ignore
150+
)
151+
152+
assert check.error_behavior == autosuspend.ErrorBehavior.IGNORE
153+
154+
def test_error_behavior_rejects_invalid_value(self, mocker: MockerFixture) -> None:
155+
mock_class = mocker.patch("autosuspend.checks.activity.Mpd")
156+
mock_class.create.return_value = mocker.MagicMock(
157+
spec=autosuspend.checks.Activity
158+
)
159+
160+
parser = configparser.ConfigParser()
161+
parser.read_string("""
162+
[check.Foo]
163+
class = Mpd
164+
enabled = True
165+
error_behavior = bogus
166+
""")
167+
168+
with pytest.raises(autosuspend.ConfigurationError):
169+
autosuspend.set_up_checks(
170+
parser, "check", "activity", autosuspend.Activity # type: ignore
171+
)
172+
173+
def test_error_behavior_not_set_for_wakeups(self, mocker: MockerFixture) -> None:
174+
mock_class = mocker.patch("autosuspend.checks.wakeup.File")
175+
check = mocker.MagicMock(spec=autosuspend.checks.Wakeup)
176+
mock_class.create.return_value = check
177+
178+
parser = configparser.ConfigParser()
179+
parser.read_string("""
180+
[wakeup.Foo]
181+
class = File
182+
enabled = True
183+
""")
184+
185+
autosuspend.set_up_checks(
186+
parser, "wakeup", "wakeup", autosuspend.Wakeup # type: ignore
187+
)
188+
189+
assert not hasattr(check, "error_behavior")
190+
117191
def test_external_class(self, mocker: MockerFixture) -> None:
118192
mock_class = mocker.patch("os.path.TestCheck", create=True)
119193
mock_class.create.return_value = mocker.MagicMock(
@@ -290,6 +364,7 @@ def test_all_called(self, mocker: MockerFixture) -> None:
290364
def test_treat_temporary_errors_as_activity(self, mocker: MockerFixture) -> None:
291365
matching_check = mocker.MagicMock(spec=autosuspend.Activity)
292366
matching_check.name = "foo"
367+
matching_check.error_behavior = autosuspend.ErrorBehavior.ACTIVE
293368
matching_check.check.side_effect = autosuspend.TemporaryCheckError()
294369

295370
assert (
@@ -298,6 +373,20 @@ def test_treat_temporary_errors_as_activity(self, mocker: MockerFixture) -> None
298373
)
299374
matching_check.check.assert_called_once_with()
300375

376+
def test_ignore_temporary_errors_when_configured(
377+
self, mocker: MockerFixture
378+
) -> None:
379+
matching_check = mocker.MagicMock(spec=autosuspend.Activity)
380+
matching_check.name = "foo"
381+
matching_check.error_behavior = autosuspend.ErrorBehavior.IGNORE
382+
matching_check.check.side_effect = autosuspend.TemporaryCheckError()
383+
384+
assert (
385+
autosuspend.execute_checks([matching_check], False, mocker.MagicMock())
386+
is False
387+
)
388+
matching_check.check.assert_called_once_with()
389+
301390

302391
class TestExecuteWakeups:
303392
def test_no_wakeups(self, mocker: MockerFixture) -> None:

tests/test_checks.py

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import configparser
22

3-
from autosuspend.checks import Check
3+
from autosuspend.checks import Activity, Check, ErrorBehavior
44

55

66
class DummyCheck(Check):
@@ -12,6 +12,15 @@ def check(self) -> str | None:
1212
pass
1313

1414

15+
class DummyActivity(Activity):
16+
@classmethod
17+
def create(cls, name: str, config: configparser.SectionProxy) -> "DummyActivity":
18+
raise NotImplementedError()
19+
20+
def check(self) -> str | None:
21+
pass
22+
23+
1524
class TestCheck:
1625
class TestName:
1726
def test_returns_the_provided_name(self) -> None:
@@ -23,3 +32,8 @@ def test_has_a_sensible_default(self) -> None:
2332

2433
def test_has_a_string_representation(self) -> None:
2534
assert isinstance(str(DummyCheck("test")), str)
35+
36+
37+
class TestActivity:
38+
def test_error_behavior_defaults_to_active(self) -> None:
39+
assert DummyActivity("test").error_behavior == ErrorBehavior.ACTIVE

0 commit comments

Comments
 (0)