diff --git a/news/6733.bugfix.md b/news/6733.bugfix.md new file mode 100644 index 00000000000..a3574f94b21 --- /dev/null +++ b/news/6733.bugfix.md @@ -0,0 +1 @@ +Re-constructing the ASGI app on the same app instance no longer re-applies app mutations. Plugin `post_compile` hooks run at most once per plugin per app instance (previously every ASGI app construction re-ran them, e.g. installing a duplicate middleware that gated every event twice), and the assembled ASGI app — including the compiled-frontend mount and `api_transformer` wiring — is built once and cached. diff --git a/packages/reflex-base/src/reflex_base/plugins/base.py b/packages/reflex-base/src/reflex_base/plugins/base.py index 6673309840b..a239059c666 100644 --- a/packages/reflex-base/src/reflex_base/plugins/base.py +++ b/packages/reflex-base/src/reflex_base/plugins/base.py @@ -139,6 +139,11 @@ def pre_compile(self, **context: Unpack[PreCompileContext]) -> None: def post_compile(self, **context: Unpack[PostCompileContext]) -> None: """Called after the compilation of the plugin. + Runs at most once per app instance — even when the ASGI app is + constructed repeatedly, and even when another plugin's failed hook + forces a retry — so hooks may mutate the app (e.g. + ``add_middleware``) without their own idempotency guard. + Args: context: The context for the plugin. """ diff --git a/reflex/app.py b/reflex/app.py index 9364c547436..0b4adaa6f9c 100644 --- a/reflex/app.py +++ b/reflex/app.py @@ -42,6 +42,7 @@ ) from reflex_base.event.context import EventContext from reflex_base.event.processor import BaseStateEventProcessor, EventProcessor +from reflex_base.plugins import Plugin from reflex_base.registry import RegistrationContext from reflex_base.telemetry_context import CompileTrigger, TelemetryContext from reflex_base.utils import console, memo_paths @@ -402,6 +403,17 @@ class App(MiddlewareMixin, LifespanMixin): # that already compiled and skips re-evaluation. _evaluated_pages: set[str] = dataclasses.field(default_factory=set) + # Plugins whose post_compile hook already ran for this app instance. + # The hooks mutate the app (add_middleware, mounting routes), so each + # must run at most once, even when another plugin's failed hook forces + # the ASGI app assembly to be retried. + _post_compiled_plugins: set[Plugin] = dataclasses.field(default_factory=set) + + # The ASGI app assembled by __call__, built at most once per app + # instance: assembly mutates persistent objects (self._api, a Starlette + # api_transformer), so re-constructing the ASGI app must not re-run it. + _cached_asgi_app: ASGIApp | None = None + # The backend API object. _api: Starlette | None = None @@ -708,9 +720,6 @@ def __call__(self) -> ASGIApp: Returns: The backend api. - - Raises: - ValueError: If the app has not been initialized. """ from reflex_base.vars.base import GLOBAL_CACHE @@ -725,14 +734,29 @@ def __call__(self) -> ASGIApp: trigger=get_backend_compile_trigger(), ) - config = get_config() - - for plugin in config.plugins: - plugin.post_compile(app=self) + # Assembly runs at most once per app instance; _compile stays per-call. + if self._cached_asgi_app is None: + self._cached_asgi_app = self._assemble_asgi_app() # We will not be making more vars, so we can clear the global cache to free up memory. GLOBAL_CACHE.clear() + return self._cached_asgi_app + + def _assemble_asgi_app(self) -> ASGIApp: + """Run plugin post_compile hooks and wrap the backend api into an ASGI app. + + Returns: + The assembled ASGI app. + + Raises: + ValueError: If the app has not been initialized. + """ + for plugin in get_config().plugins: + if plugin not in self._post_compiled_plugins: + plugin.post_compile(app=self) + self._post_compiled_plugins.add(plugin) + if not self._api: msg = "The app has not been initialized." raise ValueError(msg) diff --git a/tests/units/test_app.py b/tests/units/test_app.py index 270f040f684..5879e3188af 100644 --- a/tests/units/test_app.py +++ b/tests/units/test_app.py @@ -3064,6 +3064,92 @@ def test_call_app(): assert isinstance(api, Starlette) +def _app_with_plugins(mocker: MockerFixture, *plugins: unittest.mock.Mock) -> App: + """Create an app configured with the given plugin doubles. + + Args: + mocker: The pytest-mock fixture. + plugins: Plugin doubles to install in the app's config. + + Returns: + An app with a mocked-out ``_compile``. + """ + conf = rx.Config(app_name="testing", plugins=list(plugins)) + mocker.patch("reflex_base.config._get_config", return_value=conf) + app = App() + app._compile = unittest.mock.Mock() + return app + + +def test_call_app_runs_plugin_post_compile_once(mocker: MockerFixture): + """Constructing the ASGI app twice runs each plugin's post_compile once. + + ``post_compile`` hooks mutate the app (add middleware, mount routes) and + are not required to be idempotent, so ``App.__call__`` must not re-run + them when the ASGI app is constructed again on the same app instance + (e.g. a test harness rebuilding it). + """ + plugin = unittest.mock.Mock(spec=rx.plugins.Plugin) + app = _app_with_plugins(mocker, plugin) + app() + app() + + plugin.post_compile.assert_called_once() + + +def test_call_app_retries_post_compile_after_failure(mocker: MockerFixture): + """A post_compile failure is not latched as done. + + If a hook raises (e.g. a plugin rejecting a misconfigured app), the next + ASGI construction must run the hook again rather than silently skipping + it. + """ + plugin = unittest.mock.Mock(spec=rx.plugins.Plugin) + plugin.post_compile.side_effect = [ValueError("misconfigured"), None] + app = _app_with_plugins(mocker, plugin) + with pytest.raises(ValueError, match="misconfigured"): + app() + app() + + assert plugin.post_compile.call_count == 2 + + +def test_call_app_partial_post_compile_failure(mocker: MockerFixture): + """A failed hook is retried without re-running already-succeeded hooks. + + With multiple plugins, a failure in a later plugin's post_compile must + not cause the earlier plugins' (non-idempotent) hooks to run again on + the next ASGI construction. + """ + ok_plugin = unittest.mock.Mock(spec=rx.plugins.Plugin) + flaky_plugin = unittest.mock.Mock(spec=rx.plugins.Plugin) + flaky_plugin.post_compile.side_effect = [ValueError("misconfigured"), None] + app = _app_with_plugins(mocker, ok_plugin, flaky_plugin) + with pytest.raises(ValueError, match="misconfigured"): + app() + app() + + ok_plugin.post_compile.assert_called_once() + assert flaky_plugin.post_compile.call_count == 2 + + +def test_call_app_caches_asgi_app(mocker: MockerFixture): + """Repeated ASGI construction returns the same app without re-assembly. + + Assembly mutates persistent objects (``self._api``, ``api_transformer``), + e.g. appending the frontend mount or re-adding CORS middleware, so it must + run at most once per app instance while ``_compile`` still runs per call. + """ + app = _app_with_plugins(mocker) + compile_mock = unittest.mock.Mock() + app._compile = compile_mock + first = app() + second = app() + + assert first is second + assert compile_mock.call_count == 2 + + @pytest.fixture def upload_enabled(monkeypatch): """Fixture that enables Upload and cleans up afterward."""