Report unusable rule literals as rule and value errors

monit_docker/adapters/rules.py

@@ -21,6 +21,14 @@
 _NUMERIC_CONDITION_PARTS = (('value', 'value_unit'), ('pre_value', 'pre_value_unit'))
 
 
+def _byte_value(literal):
+    """Read a sized literal as bytes; reject a unit the parser cannot resolve."""
+    try:
+        return bitmath.parse_string(literal).bytes
+    except ValueError:
+        raise RuleSyntaxError('unsupported unit in expression value: %r' % literal) from None
+
+
 class RuleParser(object):
     def __init__(self, commands=None, conditions=None, dir_groups=None):
         self.dir_groups = directory_groups(dir_groups)
@@ -44,10 +52,10 @@ def _load_conditions_conf_finalize(self, name, exprs):
             m = m.groupdict()
 
             if m.get('pre_value_unit'):
-                m['pre_value'] = bitmath.parse_string(m['pre_value']).bytes
+                m['pre_value'] = _byte_value(m['pre_value'])
 
             if m.get('value_unit'):
-                m['value'] = bitmath.parse_string(m['value']).bytes
+                m['value'] = _byte_value(m['value'])
 
             r.append({'real': expr,
                       'parsed': m})
@@ -89,10 +97,10 @@ def _parse_exprs(self, expr, datatypes):
             m = m.groupdict()
 
             if m.get('pre_value_unit'):
-                m['pre_value'] = bitmath.parse_string(m['pre_value']).bytes
+                m['pre_value'] = _byte_value(m['pre_value'])
 
             if m.get('value_unit'):
-                m['value'] = bitmath.parse_string(m['value']).bytes
+                m['value'] = _byte_value(m['value'])
 
             if m['cond_alias']:
                 cond_alias = m['cond_alias']

monit_docker/core/rules.py

@@ -69,7 +69,9 @@ def _matches_value(self, condition, actual):
                 expected = type(actual)(condition.value)
                 if pre_value is not None:
                     pre_value = type(actual)(pre_value)
-            except TypeError:
+            except (TypeError, ValueError):
+                # A literal that does not fit the measured value's type is a
+                # value error, not an unexpected exception.
                 raise MonitoringError(110, 'invalid value for expression if: %r' %
                                       condition.source)
         if None not in (condition.pre_operator, pre_value):

tests/test_monit_docker.py

@@ -182,6 +182,18 @@ def test_invalid_later_rule_prevents_any_earlier_action_or_connection(self):
         self.client.containers.list.assert_not_called()
         md.docker.from_env.assert_not_called()
 
+    def test_unsupported_rule_unit_is_a_rule_syntax_error(self):
+        obj = self.client.containers.list.return_value[0]
+        self.assertEqual(self.invoke('monit', '--cmd-if', 'mem_usage > 100 mb ? restart'), 140)
+        obj.restart.assert_not_called()
+        self.client.containers.list.assert_not_called()
+        md.docker.from_env.assert_not_called()
+
+    def test_rule_value_that_does_not_fit_the_resource_is_a_value_error(self):
+        obj = self.client.containers.list.return_value[0]
+        self.assertEqual(self.invoke('monit', '--cmd-if', 'mem_usage > 1.5 ? restart'), 110)
+        obj.restart.assert_not_called()
+
     def test_status_exit_closes_client_and_does_not_sample(self):
         obj = self.client.containers.list.return_value[0]
         obj.status = 'exited'

tests/test_rule_literals.py

@@ -0,0 +1,54 @@
+"""Rule literals that cannot be compared must fail as documented errors.
+
+An unsupported size unit or a literal that does not fit the measured value's
+type used to escape as an unhandled ValueError, so the historical runtime CLI
+reported the reserved "unexpected exception" status instead of a rule or value
+error.
+"""
+import unittest
+
+from monit_docker.adapters.rules import RuleParser
+from monit_docker.core.rules import RuleEvaluator
+from monit_docker.domain.errors import MonitoringError, RuleSyntaxError
+from monit_docker.domain.models import ContainerSnapshot
+
+
+class RuleLiteralTests(unittest.TestCase):
+    def test_unsupported_unit_is_a_rule_syntax_error(self):
+        for expression in ('mem_usage > 100 mb ? restart',
+                           'mem_limit > 100 KB ? restart',
+                           '100 mb < mem_usage < 200 MiB ? restart'):
+            with self.subTest(expression=expression):
+                with self.assertRaises(RuleSyntaxError):
+                    RuleParser().parse(expression)
+
+    def test_unsupported_unit_in_a_configured_condition_is_a_rule_syntax_error(self):
+        with self.assertRaises(RuleSyntaxError):
+            RuleParser(conditions={'heavy': {'expr': ['mem_usage > 100 mb']}})
+
+    def test_value_that_does_not_fit_the_resource_is_a_value_error(self):
+        parser = RuleParser()
+        snapshot = ContainerSnapshot(mem_usage=1048576, mem_limit=8388608,
+                                     cpu_percent=12.5, mem_percent=25.0)
+        for expression in ('mem_usage > 1.5 ? restart',
+                           '1.5 < mem_usage < 2048 ? restart',
+                           'cpu_percent > healthy ? restart'):
+            with self.subTest(expression=expression):
+                with self.assertRaises(MonitoringError) as error:
+                    RuleEvaluator().matches(parser.parse(expression), snapshot)
+                self.assertEqual(error.exception.code, 110)
+
+    def test_supported_units_and_integer_values_still_evaluate(self):
+        parser = RuleParser()
+        snapshot = ContainerSnapshot(mem_usage=209715200, mem_limit=8388608,
+                                     cpu_percent=12.5, mem_percent=25.0)
+        self.assertTrue(RuleEvaluator().matches(
+            parser.parse('mem_usage > 100 MiB ? restart'), snapshot))
+        self.assertTrue(RuleEvaluator().matches(
+            parser.parse('100 MiB < mem_usage < 300 MiB ? restart'), snapshot))
+        self.assertTrue(RuleEvaluator().matches(
+            parser.parse('cpu_percent > 1.5 ? restart'), snapshot))
+
+
+if __name__ == '__main__':
+    unittest.main()