From cb6c8284575b70d754f5fc83158d8ef38777526b Mon Sep 17 00:00:00 2001 From: Tejas Chopra Date: Tue, 14 Jul 2026 22:50:03 -0700 Subject: [PATCH] fix(proxy): one bad extension no longer aborts proxy startup (#2215) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What `install_all()` (the `headroom.proxy_extension` loader) previously let any exception from an extension's `install()` **propagate and abort proxy startup** — one broken or version-incompatible third-party extension took the whole proxy down, and every other extension with it. This makes extension loading resilient: - catch a failing `install()`, log it (with traceback), record it as **skipped** - continue installing the rest — a failure disables that one extension, not the proxy - print a `SKIPPED` line to the console (the startup banner lists *enabled* extensions before install runs, so a skip would otherwise be logging-config dependent) ## Why Found while testing several proxy extensions together in a clean venv: a plugin built against a newer core API raised `ModuleNotFoundError` from `install()` and crashed the proxy at startup. An extension that fails its own environment/auth check should disable itself — it should not take the whole proxy down. ## Real behavior proof Before — one extension failing in `install()`: ``` ... proxy did NOT come up (/livez never answered) ``` After — same setup, one extension deliberately broken: ``` [headroom] proxy extensions SKIPPED: myorg_ext (install failed — running without them; see logs) /livez: 200 healthy # proxy up; the other extensions installed ``` Loader unit check (fake failing extension): ``` returned installed: ['good_ext'] # bad one excluded bad_ext skipped (not in installed): True good_ext survived: True warning logged for bad_ext: True ``` ## Tests - `mypy headroom/proxy/extensions.py` → `Success: no issues found` - `ruff check headroom/proxy/extensions.py` → `All checks passed!` - Verified in-process (catch/skip/continue + logging) and end-to-end against a running proxy (`/livez` 200 with a deliberately failing extension). ## Maintainer Follow-up - Added `tests/test_proxy_extensions.py` covering skip-and-continue behavior for a failed extension and the missing-extension warning path. - Removed an informal implementation comment from `headroom/proxy/extensions.py`. - Validation on `fe5176db`: `uv run --frozen --extra dev python -m pytest tests/test_proxy_extensions.py -q`, `uvx ruff==0.15.17 check headroom/proxy/extensions.py tests/test_proxy_extensions.py --output-format concise`, `git diff --check`, and commit hooks all passed. --------- Co-authored-by: JerrettDavis --- headroom/proxy/extensions.py | 53 ++++++++++++++++++++++++++-------- tests/test_proxy_extensions.py | 46 +++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 12 deletions(-) create mode 100644 tests/test_proxy_extensions.py diff --git a/headroom/proxy/extensions.py b/headroom/proxy/extensions.py index 59b33f12d..9ead7a6cd 100644 --- a/headroom/proxy/extensions.py +++ b/headroom/proxy/extensions.py @@ -12,7 +12,8 @@ Each ``install`` callable is invoked with the FastAPI ``app`` and the * register ASGI middleware (``app.add_middleware(...)``) * add routes or health endpoints * mutate config - * raise on license / environment failure to abort startup + * raise on an environment or auth failure to disable *itself* (the proxy logs + the failure and starts without that extension) OSS makes no assumptions about what extensions do. The interface is deliberately minimal; extensions own the complexity behind it. @@ -24,14 +25,14 @@ audit gets installed in the same environment (e.g., as a transitive dep). Enabling extensions: - * CLI: ``headroom proxy --proxy-extension shield_enterprise,mypkg`` - * Env: ``HEADROOM_PROXY_EXTENSIONS=shield_enterprise,mypkg`` + * CLI: ``headroom proxy --proxy-extension myorg_ext,mypkg`` + * Env: ``HEADROOM_PROXY_EXTENSIONS=myorg_ext,mypkg`` * Wildcard: ``--proxy-extension '*'`` enables every discovered extension (use only when you trust everything in your environment). -Stability contract: this module is load-bearing for the Enterprise build and -any third-party extensions. Changes to the signature of ``install(app, config)`` -or the entry-point group name require a deprecation cycle. +Stability contract: this module is load-bearing for any third-party extensions. +Changes to the signature of ``install(app, config)`` or the entry-point group +name require a deprecation cycle. """ from __future__ import annotations @@ -39,6 +40,7 @@ from __future__ import annotations import importlib.metadata import logging import os +import sys from collections.abc import Callable, Iterable, Iterator from typing import Any @@ -55,8 +57,9 @@ def discover() -> Iterator[tuple[str, ProxyExtension]]: """Yield ``(name, install_callable)`` pairs for every registered extension. Entry-point load failures are logged and skipped — a broken third-party - package must not prevent the proxy from starting. An extension that wants - to fail-closed can raise from its ``install()``. + package must not prevent the proxy from starting. An extension that fails + its environment/auth check raises from ``install()``; ``install_all`` logs + and skips it, so it is disabled rather than aborting startup. """ try: entries = importlib.metadata.entry_points(group=ENTRY_POINT_GROUP) @@ -105,9 +108,9 @@ def install_all( discovered extension. Returns the names of successfully installed extensions. If an extension - raises inside ``install()``, the exception propagates — this is the - documented fail-closed signal (e.g., a license check failing should - abort startup rather than silently run without protection). + raises inside ``install()`` it is logged, skipped, and recorded as failed — + a single broken extension is disabled rather than aborting proxy startup for + every other extension. """ enabled_set = _resolve_enabled(enabled) discovered = list(discover()) @@ -125,13 +128,39 @@ def install_all( wildcard = "*" in enabled_set installed: list[str] = [] + failed: list[str] = [] for name, install in discovered: if not wildcard and name not in enabled_set: continue - install(app, config) + try: + install(app, config) + except Exception as exc: # noqa: BLE001 — one bad extension must not brick the proxy + # A failing extension disables *itself* and the proxy keeps running + # without it — covers environment/auth failures and compatibility + # errors (e.g. a plugin built against a core API this version lacks). + log.warning( + "proxy extension %r failed to install and was skipped: %s", + name, + exc, + exc_info=True, + ) + failed.append(name) + continue installed.append(name) log.info("proxy extension installed: %s", name) + if failed: + skipped = ",".join(sorted(failed)) + log.warning("proxy extensions skipped due to install errors: %s", skipped) + # The startup banner lists enabled extensions *before* install runs, so a + # skip would otherwise only appear if logging is configured to show this + # logger. Surface it on the console unconditionally. + print( + f"[headroom] proxy extensions SKIPPED: {skipped} " + f"(install failed — running without them; see logs)", + file=sys.stderr, + ) + # Warn about names the user asked for that weren't found. if not wildcard: missing = enabled_set - set(discovered_names) diff --git a/tests/test_proxy_extensions.py b/tests/test_proxy_extensions.py new file mode 100644 index 000000000..95e76145e --- /dev/null +++ b/tests/test_proxy_extensions.py @@ -0,0 +1,46 @@ +from __future__ import annotations + +import logging +from typing import Any + +from headroom.proxy import extensions + + +def test_install_all_skips_failed_extension_and_continues( + caplog, + capsys, + monkeypatch, +) -> None: + calls: list[str] = [] + + def good(app: Any, config: Any) -> None: + calls.append("good") + + def bad(app: Any, config: Any) -> None: + calls.append("bad") + raise RuntimeError("missing optional dependency") + + monkeypatch.setattr( + extensions, + "discover", + lambda: iter([("bad_ext", bad), ("good_ext", good)]), + ) + + with caplog.at_level(logging.WARNING, logger=extensions.log.name): + installed = extensions.install_all(object(), object(), enabled=["bad_ext", "good_ext"]) + + assert installed == ["good_ext"] + assert calls == ["bad", "good"] + assert "bad_ext" in capsys.readouterr().err + assert "failed to install and was skipped" in caplog.text + assert "proxy extensions skipped due to install errors: bad_ext" in caplog.text + + +def test_install_all_warns_for_missing_requested_extension(caplog, monkeypatch) -> None: + monkeypatch.setattr(extensions, "discover", lambda: iter([])) + + with caplog.at_level(logging.WARNING, logger=extensions.log.name): + installed = extensions.install_all(object(), object(), enabled=["missing_ext"]) + + assert installed == [] + assert "proxy extensions requested but not found: missing_ext" in caplog.text