fix(server): a raising tuning provider no longer takes down get_merged() for everyone (#899)

server.py

@@ -427,7 +427,12 @@ def get_merged(self, reference_pitch: float = DEFAULT_REFERENCE_PITCH) -> dict:
                     for name, freqs in names.items():
                         result[instrument][name] = [round(f * scale, 4) for f in freqs]
             except Exception:
-                logger.exception("tuning provider %r raised during get_merged()", provider_id)
+                # `log`, not `logger` — there is no `logger` in this module. This handler
+                # exists so ONE bad provider cannot break tunings for everyone; with the
+                # wrong name it raised NameError from inside the except and did precisely
+                # what it was written to prevent. See #899 and
+                # tests/test_tuning_provider_isolation.py.
+                log.exception("tuning provider %r raised during get_merged()", provider_id)
         return result
 
 

tests/test_tuning_provider_isolation.py

@@ -0,0 +1,72 @@
+"""A raising tuning provider must not take down get_merged() for everyone. (#899)
+
+`TuningProviderRegistry.get_merged()` wraps each provider in a try/except precisely so one
+misbehaving plugin cannot break tunings for the rest. The handler called `logger.exception`
+— and there is no `logger` in server.py; the module logger is `log`. So the handler MEANT
+to swallow-and-report instead raised NameError, which propagated out of get_merged().
+
+The net effect was the exact opposite of the handler's purpose: one bad provider took the
+whole merged-tunings call down, and the traceback named the wrong problem.
+
+Nothing exercised the failure path, which is why it survived. This is that path.
+"""
+
+import importlib
+import logging
+import sys
+
+import pytest
+
+
+@pytest.fixture()
+def registry(monkeypatch, tmp_path):
+    monkeypatch.setenv("CONFIG_DIR", str(tmp_path))
+    sys.modules.pop("server", None)
+    mod = importlib.import_module("server")
+    yield mod.TuningProviderRegistry()
+
+
+def test_a_raising_provider_does_not_break_the_others(registry, caplog):
+    """The whole point of the try/except. Before the fix this raised NameError."""
+    def boom():
+        raise RuntimeError("provider exploded")
+
+    def good():
+        return {"guitar": {"My Tuning": [82.41, 110.0, 146.83, 196.0, 246.94, 329.63]}}
+
+    registry.register("bad-plugin", boom)
+    registry.register("good-plugin", good)
+
+    merged = registry.get_merged()          # must NOT raise
+
+    assert "My Tuning" in merged["guitar"], (
+        "the healthy provider's tuning is missing — one raising provider took down the "
+        "merged result for everyone"
+    )
+    # and the default tunings survive
+    assert merged["guitar"], "default tunings were lost"
+
+
+def test_the_failure_is_actually_logged(registry, caplog):
+    """Swallowing is only acceptable if it is reported. A NameError in the handler meant
+    nothing was ever logged — the failure was both fatal AND silent about its real cause."""
+    def boom():
+        raise RuntimeError("provider exploded")
+
+    registry.register("bad-plugin", boom)
+
+    # The feedBack logger sets propagate=False, so pytest's root-logger capture sees
+    # NOTHING from it. Attach caplog's handler directly. (test_plugins.py has a
+    # capture_logger() context manager for this, but it is not importable from here:
+    # pyproject pins pythonpath to [".", "lib"], so `tests` is not a package.)
+    lg = logging.getLogger("feedBack")
+    lg.addHandler(caplog.handler)
+    lg.setLevel(logging.ERROR)
+    try:
+        registry.get_merged()
+    finally:
+        lg.removeHandler(caplog.handler)
+
+    assert any("bad-plugin" in r.getMessage() for r in caplog.records), (
+        "the raising provider was never named in the logs"
+    )