mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
fix(proxy): one bad extension no longer aborts proxy startup (#2215)
## 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 <mxjerrett@gmail.com>
This commit is contained in:
parent
79d8056fd7
commit
cb6c828457
2 changed files with 87 additions and 12 deletions
|
|
@ -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)
|
||||
|
|
|
|||
46
tests/test_proxy_extensions.py
Normal file
46
tests/test_proxy_extensions.py
Normal file
|
|
@ -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
|
||||
Loading…
Add table
Add a link
Reference in a new issue