Skip to content

ENG-7314: Only check for valid app name once per config instance#5710

Merged
adhami3310 merged 2 commits intomainfrom
masenf/protect-app-module-lookup
Aug 14, 2025
Merged

ENG-7314: Only check for valid app name once per config instance#5710
adhami3310 merged 2 commits intomainfrom
masenf/protect-app-module-lookup

Conversation

@masenf
Copy link
Collaborator

@masenf masenf commented Aug 13, 2025

After the backend comes up, we check the app name once and validate that it's good. There is no need to check it again in the same process.

Fixes the spurious app.app module not found errors that occur when deleting and replacing the main app module while the backend is running, but before triggering a reload.

Repro code

import asyncio
from pathlib import Path

import reflex as rx


class State(rx.State):
    """The app state."""

    shuffling: bool = False
    interval: int = 0
    loaded: int = 0
    tasked: int = 0

    @rx.event
    def set_interval(self, interval: int):
        self.interval = interval

    @rx.event
    def on_load(self):
        self.loaded += 1

    @rx.event(background=True)
    async def bg_task(self):
        async with self:
            self.tasked += 1
        await asyncio.sleep(0.1)
        if self.tasked > 0:
            yield State.bg_task

    @rx.event(background=True)
    async def raise_over_400(self):
        while True:
            async with self:
                if self.tasked > 400:
                    self.tasked = 0
                    raise Exception("Task limit exceeded")
            await asyncio.sleep(0.1)

    @rx.event
    def backend_exception(self):
        raise RuntimeError("This is a backend exception")

    @rx.event
    def frontend_exception(self):
        return rx.call_script("foobarbaz")

    @rx.event(background=True)
    async def shuffle_app_module(self):
        """Rename the main app module for a couple seconds.

        This simulates replacing the code before manually triggering a hot reload.
        """
        async with self:
            if self.shuffling:
                return rx.toast("Shuffling in progress...")
            self.shuffling = True
        try:
            current_path = Path(__file__)
            new_path = current_path.with_name("renamed.py")
            current_path.rename(new_path)
            await asyncio.sleep(2)
            new_path.rename(current_path)
        finally:
            async with self:
                self.shuffling = False


def index() -> rx.Component:
    return rx.container(
        rx.vstack(
            rx.button(
                "Shuffle App Module",
                on_click=State.shuffle_app_module,
                disabled=State.shuffling,
            ),
            rx.hstack(
                rx.text(State.loaded),
                rx.button(
                    "Toggle periodic on_load",
                    on_click=State.set_interval(rx.cond(State.interval > 0, 0, 300)),
                ),
            ),
            rx.hstack(
                rx.text(State.tasked),
                rx.cond(
                    State.tasked > 0,
                    rx.button("Stop Background Task", on_click=State.set_tasked(0)),
                    rx.button("Start Background Task", on_click=State.bg_task),
                ),
            ),
            rx.hstack(
                rx.button("Backend Exception", on_click=State.backend_exception),
                rx.button("Frontend Exception", on_click=State.frontend_exception),
            ),
            rx.moment(
                interval=State.interval,
                on_change=rx.state.OnLoadInternalState.on_load_internal,
            ),
        ),
    )


app = rx.App()
app.add_page(index, on_load=State.on_load)
REFLEX_HOT_RELOAD_EXCLUDE_PATHS=repro_module_not_found reflex run

Click on "Shuffle App Module" to simulate removing the app module and putting it back 2 seconds later. While the app module is shuffled, clicking on Backend or Frontend exception should NOT raise the module not found error. For the other ones, start the background task or the periodic onload, then click "Shuffle App Module", this should NOT raise the module not found error.

Without this fix, the above 3 scenarios cause an exception on the backend. (on_load path was fixed previously)

After the backend comes up, we check the app name once and validate that it's
good. There is no need to check it again in the same process.

Fixes the spurious `app.app module not found` errors that occur when deleting
and replacing the main app module while the backend is running, but before
triggering a reload.
@linear
Copy link

linear bot commented Aug 13, 2025

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greptile Summary

This PR introduces a caching mechanism to optimize app name validation in Reflex applications. The change adds a _app_name_is_valid boolean field to the Config dataclass that tracks whether the app name has already been validated for the current process instance.

The implementation works by:

  1. Adding the _app_name_is_valid field to the Config class (defaulting to False)
  2. Modifying the get_app function in prerequisites.py to check this flag before calling _check_app_name
  3. Setting the flag to True after successful validation in _check_app_name

This optimization addresses a specific developer experience issue where spurious "app.app module not found" errors would appear when deleting and replacing the main app module while the backend is running. The underlying problem was that _check_app_name performs filesystem operations to validate module existence, which could temporarily fail during file replacement operations in development workflows.

By caching the validation result on the Config instance (which persists for the backend process lifetime), the system eliminates redundant disk checks after initial successful validation. This maintains the same validation behavior on first access while improving performance and removing transient error messages during common development operations.

Confidence score: 4/5

  • This PR is safe to merge with minimal risk as it only optimizes an existing validation flow without changing core logic
  • Score reflects the targeted nature of the fix and clear performance benefit, though the caching approach assumes Config instance stability
  • Pay close attention to the interaction between the two modified files to ensure the flag is properly managed

2 files reviewed, 1 comment

Edit Code Review Bot Settings | Greptile

@codspeed-hq
Copy link

codspeed-hq bot commented Aug 13, 2025

CodSpeed Performance Report

Merging #5710 will not alter performance

Comparing masenf/protect-app-module-lookup (be878c6) with main (38e1efc)

Summary

✅ 8 untouched benchmarks

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
@adhami3310 adhami3310 merged commit 948b993 into main Aug 14, 2025
41 checks passed
@adhami3310 adhami3310 deleted the masenf/protect-app-module-lookup branch August 14, 2025 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants