diff --git a/servicewright/adapters/fastapi/_imports.py b/servicewright/adapters/fastapi/_imports.py index 8e03272..15ea1d4 100644 --- a/servicewright/adapters/fastapi/_imports.py +++ b/servicewright/adapters/fastapi/_imports.py @@ -3,7 +3,12 @@ Importing this module fails with a friendly message when the ``fastapi`` extra is not installed, so ``import servicewright`` never pays the cost (or the failure) of the FastAPI dependencies. Every HTTP submodule imports its -third-party symbols from here. +third-party symbols from here -- ``starlette``, ``pydantic`` and +``deadline_budget`` included, none of which is a name the extras table mentions. +A submodule that reaches for one of them directly gets there first and reports +it by its own name, which is what issue #51 was. + +``TYPE_CHECKING``-only imports are exempt: they never run. """ from __future__ import annotations @@ -12,20 +17,33 @@ try: import uvicorn - from fastapi import FastAPI, Request, status + from deadline_budget import DeadlineExceededError + from fastapi import Depends, FastAPI, Header, Request, status + from fastapi.exceptions import RequestValidationError from fastapi.middleware.cors import CORSMiddleware from fastapi.middleware.gzip import GZipMiddleware + from pydantic import BaseModel, ConfigDict, Field + from starlette.datastructures import Headers, MutableHeaders from starlette.exceptions import HTTPException as StarletteHTTPException from starlette.responses import JSONResponse, Response except ImportError as exc: # pragma: no cover - exercised only without the extra raise ImportError(_INSTALL_HINT) from exc __all__ = [ + "BaseModel", "CORSMiddleware", + "ConfigDict", + "DeadlineExceededError", + "Depends", "FastAPI", + "Field", "GZipMiddleware", + "Header", + "Headers", "JSONResponse", + "MutableHeaders", "Request", + "RequestValidationError", "Response", "StarletteHTTPException", "status", diff --git a/servicewright/adapters/fastapi/exceptions.py b/servicewright/adapters/fastapi/exceptions.py index 776c31d..bdc9ae4 100644 --- a/servicewright/adapters/fastapi/exceptions.py +++ b/servicewright/adapters/fastapi/exceptions.py @@ -18,9 +18,6 @@ import logging from typing import TYPE_CHECKING, Any -from deadline_budget import DeadlineExceededError as LibraryDeadlineExceededError -from fastapi.exceptions import RequestValidationError - from ...core.errors import ( INTERNAL_ERROR_CODE, ErrorInfo, @@ -29,7 +26,13 @@ ServiceError, mask_private_error, ) -from ._imports import JSONResponse, StarletteHTTPException, status +from ._imports import DeadlineExceededError as LibraryDeadlineExceededError +from ._imports import ( + JSONResponse, + RequestValidationError, + StarletteHTTPException, + status, +) if TYPE_CHECKING: from collections.abc import Awaitable, Callable diff --git a/servicewright/adapters/fastapi/headers.py b/servicewright/adapters/fastapi/headers.py index 912234e..b5a062f 100644 --- a/servicewright/adapters/fastapi/headers.py +++ b/servicewright/adapters/fastapi/headers.py @@ -9,7 +9,7 @@ from typing import Annotated from uuid import UUID -from fastapi import Header +from ._imports import Header IDEMPOTENCY_KEY_PATTERN = r"^[A-Za-z0-9_\-]+$" IDEMPOTENCY_KEY_MAX_LEN = 128 diff --git a/servicewright/adapters/fastapi/middlewares/context.py b/servicewright/adapters/fastapi/middlewares/context.py index ba38feb..ce3127c 100644 --- a/servicewright/adapters/fastapi/middlewares/context.py +++ b/servicewright/adapters/fastapi/middlewares/context.py @@ -6,8 +6,6 @@ import uuid from typing import TYPE_CHECKING, Any -from starlette.datastructures import Headers - from ....core.context import ( bind_context_values, current_context, @@ -16,6 +14,7 @@ is_safe_context_id, set_context_value, ) +from .._imports import Headers if TYPE_CHECKING: from collections.abc import Callable @@ -92,7 +91,7 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: # 2. Extract from cookies (if any). if self.cookie_extractors: - from starlette.requests import Request + from .._imports import Request request = Request(scope) for cookie, ctx_key in self.cookie_extractors.items(): diff --git a/servicewright/adapters/fastapi/middlewares/logging.py b/servicewright/adapters/fastapi/middlewares/logging.py index 9458101..e931e15 100644 --- a/servicewright/adapters/fastapi/middlewares/logging.py +++ b/servicewright/adapters/fastapi/middlewares/logging.py @@ -12,7 +12,7 @@ import time from typing import TYPE_CHECKING -from starlette.datastructures import Headers +from .._imports import Headers if TYPE_CHECKING: from collections.abc import Sequence diff --git a/servicewright/adapters/fastapi/middlewares/processing_time.py b/servicewright/adapters/fastapi/middlewares/processing_time.py index ab722a3..689f8fa 100644 --- a/servicewright/adapters/fastapi/middlewares/processing_time.py +++ b/servicewright/adapters/fastapi/middlewares/processing_time.py @@ -5,7 +5,7 @@ import time from typing import TYPE_CHECKING -from starlette.datastructures import MutableHeaders +from .._imports import MutableHeaders if TYPE_CHECKING: from starlette.types import ASGIApp, Message, Receive, Scope, Send diff --git a/servicewright/adapters/fastapi/schemas.py b/servicewright/adapters/fastapi/schemas.py index cbe69fa..4803d65 100644 --- a/servicewright/adapters/fastapi/schemas.py +++ b/servicewright/adapters/fastapi/schemas.py @@ -4,7 +4,7 @@ from typing import Any, Literal -from pydantic import BaseModel, ConfigDict, Field +from ._imports import BaseModel, ConfigDict, Field class LivenessResponse(BaseModel): diff --git a/servicewright/adapters/fastapi/unit_scope.py b/servicewright/adapters/fastapi/unit_scope.py index 3e4882f..f98f14b 100644 --- a/servicewright/adapters/fastapi/unit_scope.py +++ b/servicewright/adapters/fastapi/unit_scope.py @@ -20,7 +20,7 @@ import contextvars from typing import TYPE_CHECKING, Annotated -from fastapi import Depends, Request +from ._imports import Depends, Request if TYPE_CHECKING: from starlette.types import ASGIApp, Receive, Scope, Send diff --git a/tests/unit/test_adapter_extras.py b/tests/unit/test_adapter_extras.py new file mode 100644 index 0000000..68f53ef --- /dev/null +++ b/tests/unit/test_adapter_extras.py @@ -0,0 +1,100 @@ +"""Extra-gated subpackages name their extra when the extra is not installed. + +Regression cover for issue #51: ``servicewright.adapters.fastapi`` raised +``ModuleNotFoundError: No module named 'starlette'`` in a bare install, because +several modules in the subpackage imported their third-party symbols directly +instead of through ``_imports.py``. Whichever unguarded import ran first was the +message the user got, and ``starlette`` is a name that appears in no extras +table. The contract (``docs/agents.md``, the adapters section and the errors +table) is that importing one of these without its extra raises an ``ImportError`` +naming what to install. + +The dev environment installs every extra, so absence is simulated in a fresh +interpreter: a ``sys.meta_path`` finder refuses everything the extra puts on the +path, which is what the import machinery does when the extra is genuinely +missing. Nothing is patched in this process, so no test can leak a half-imported +adapter into the next one. +""" + +from __future__ import annotations + +import subprocess +import sys + +import pytest + +pytestmark = pytest.mark.unit + +# The subpackage, the extra its message must name, and the top-level packages +# the extra puts on the path -- blocking all of them is the bare install. +_GATED_SUBPACKAGES: list[tuple[str, str, tuple[str, ...]]] = [ + ( + "fastapi", + "fastapi", + ("fastapi", "starlette", "uvicorn", "pydantic", "deadline_budget", "prometheus_fastapi_instrumentator"), + ), + ("litestar", "litestar", ("litestar", "uvicorn")), + ("grpc", "grpc", ("grpc", "grpc_health", "grpc_server_kit")), + ("apscheduler4", "apscheduler4", ("apscheduler",)), + ("apscheduler3", "apscheduler3", ("apscheduler",)), + ("dishka", "dishka", ("dishka",)), + ("settings", "settings", ("pydantic", "pydantic_settings")), +] + +# Imports ``sys.argv[1]`` with the comma-separated top-level packages in +# ``sys.argv[2]`` made unimportable, and prints the exception type and message. +_PROBE = """ +import sys + + +class Blocker: + def __init__(self, blocked): + self._blocked = blocked + + def find_spec(self, fullname, path=None, target=None): + if fullname.partition(".")[0] in self._blocked: + raise ModuleNotFoundError("No module named " + repr(fullname), name=fullname) + return None + + +module, blocked = sys.argv[1], set(sys.argv[2].split(",")) +sys.meta_path.insert(0, Blocker(blocked)) +for name in [n for n in sys.modules if n.partition(".")[0] in blocked]: + del sys.modules[name] + +try: + __import__(module) +except ImportError as exc: + print(type(exc).__name__ + ": " + str(exc)) +else: + print("imported, with the extra blocked") +""" + + +def _import_without(module: str, blocked: tuple[str, ...]) -> str: + """Return what importing ``module`` raises in an interpreter without ``blocked``.""" + result = subprocess.run( # noqa: S603 - fixed argv + [sys.executable, "-c", _PROBE, module, ",".join(blocked)], + capture_output=True, + text=True, + check=False, + ) + assert result.returncode == 0, result.stdout + result.stderr + return result.stdout.strip() + + +@pytest.mark.parametrize( + ("subpackage", "extra", "blocked"), + _GATED_SUBPACKAGES, + ids=[subpackage for subpackage, _, _ in _GATED_SUBPACKAGES], +) +def test__gated_subpackage__imported_without_its_extra__raises_import_error_naming_the_extra( + subpackage: str, extra: str, blocked: tuple[str, ...] +) -> None: + # Act + raised = _import_without(f"servicewright.adapters.{subpackage}", blocked) + + # Assert: an ImportError of its own, not the bare ModuleNotFoundError the + # machinery raises for a third-party package the user never asked for. + assert raised.startswith("ImportError: "), raised + assert f"servicewright[{extra}]" in raised, raised