From 6cb81a5caca1456d793bfa6c49225a6eab073130 Mon Sep 17 00:00:00 2001 From: Sean Kim Date: Thu, 9 Jul 2026 15:45:45 -0700 Subject: [PATCH] Validate Enum arguments before registering the collector Enum.__init__ called super().__init__() -- which registers the collector in the CollectorRegistry -- before validating that states is non-empty and that the metric name does not overlap a label name. When either check failed, the ValueError was raised as expected, but a half-built Enum (whose _states was never assigned) had already been registered. That left the registry in a broken state: the name was permanently taken, so recreating the metric raised 'Duplicated timeseries', and any subsequent scrape crashed with AttributeError: 'Enum' object has no attribute '_states' when _child_samples iterated self._states. A realistic trigger is building the states list from configuration that turns out to be empty. Gauge and Histogram already validate before calling super().__init__(); this moves Enum's two guards ahead of registration to match, so a failed constructor leaves the registry untouched. Add test_failed_init_does_not_pollute_registry, which asserts that after two failed Enum constructions the name is still free, the metric can be created, and the registry scrapes cleanly. Signed-off-by: Sean Kim --- prometheus_client/metrics.py | 8 ++++---- tests/test_core.py | 13 +++++++++++++ 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/prometheus_client/metrics.py b/prometheus_client/metrics.py index 4c79c583..e3fe5323 100644 --- a/prometheus_client/metrics.py +++ b/prometheus_client/metrics.py @@ -769,6 +769,10 @@ def __init__(self, _labelvalues: Optional[Sequence[str]] = None, states: Optional[Sequence[str]] = None, ): + if name in labelnames: + raise ValueError(f'Overlapping labels for Enum metric: {name}') + if not states: + raise ValueError(f'No states provided for Enum metric: {name}') super().__init__( name=name, documentation=documentation, @@ -779,10 +783,6 @@ def __init__(self, registry=registry, _labelvalues=_labelvalues, ) - if name in labelnames: - raise ValueError(f'Overlapping labels for Enum metric: {name}') - if not states: - raise ValueError(f'No states provided for Enum metric: {name}') self._kwargs['states'] = self._states = states def _metric_init(self) -> None: diff --git a/tests/test_core.py b/tests/test_core.py index f339b6df..2d17d5c5 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -595,6 +595,19 @@ def test_overlapping_labels(self): with pytest.raises(ValueError): Enum('e', 'help', registry=None, labelnames=['e']) + def test_failed_init_does_not_pollute_registry(self): + registry = CollectorRegistry() + # A validation failure in __init__ must not leave a half-built collector + # registered: otherwise the name stays permanently taken and any later + # scrape of the registry crashes on the missing _states attribute. + with pytest.raises(ValueError): + Enum('task_state', 'help', states=None, registry=registry) + with pytest.raises(ValueError): + Enum('task_state', 'help', states=['a'], labelnames=['task_state'], registry=registry) + # The name is still free, so a correct definition registers and scrapes. + Enum('task_state', 'help', states=['a', 'b'], registry=registry) + self.assertEqual(1, registry.get_sample_value('task_state', {'task_state': 'a'})) + class TestMetricWrapper(unittest.TestCase): def setUp(self):