diff --git a/pyproject.toml b/pyproject.toml index dcd147ac1..db3dce5fa 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -78,7 +78,7 @@ dependencies = [ # From Template "fastapi[all,standard]>=0.123.10", "humanize>=4.14.0,<5", - "nicegui[native]>=3.9.0,<4", # CVE-2026-21871, CVE-2026-21873, CVE-2026-21874 (>=3.5.0); CVE-2026-25516 (>=3.7.0, #418); CVE-2026-27156 (>=3.8.0, #448); CVE-2026-33332 (>=3.9.0, #498). CVE-2026-39844 (>=3.10.0, #531) not yet merged. + "nicegui[native]>=3.11.0,<4", # CVE-2026-21871, CVE-2026-21873, CVE-2026-21874 (>=3.5.0); CVE-2026-25516 (>=3.7.0, #418); CVE-2026-27156 (>=3.8.0, #448); CVE-2026-33332 (>=3.9.0, #498); CVE-2026-39844 (>=3.10.0, #531). 3.11.0 fixes async event handler exception leaks and refines ValueChangeEventArguments generics. "packaging>=26,<27", "platformdirs>=4.5.1,<5", "psutil>=7.1.3,<8", diff --git a/src/aignostics/application/_gui/_page_application_describe.py b/src/aignostics/application/_gui/_page_application_describe.py index 79b47a977..af864c4d1 100644 --- a/src/aignostics/application/_gui/_page_application_describe.py +++ b/src/aignostics/application/_gui/_page_application_describe.py @@ -315,7 +315,7 @@ def _add_application_version_selection_section() -> None: # Show force checkbox for internal users if user_info and user_info.is_internal_user: - def on_force_change(e: ValueChangeEventArguments) -> None: + def on_force_change(e: ValueChangeEventArguments[bool | None]) -> None: if e.value: version_next_button.enable() if unhealthy_tooltip: @@ -324,7 +324,7 @@ def on_force_change(e: ValueChangeEventArguments) -> None: version_next_button.disable() if unhealthy_tooltip: unhealthy_tooltip.set_visibility(True) - submit_form.force = e.value + submit_form.force = bool(e.value) ui.checkbox("Force (skip health check)", on_change=on_force_change).mark("CHECKBOX_FORCE") diff --git a/src/aignostics/application/_gui/_page_application_run_describe.py b/src/aignostics/application/_gui/_page_application_run_describe.py index 740aae23a..684fd4709 100644 --- a/src/aignostics/application/_gui/_page_application_run_describe.py +++ b/src/aignostics/application/_gui/_page_application_run_describe.py @@ -1,5 +1,8 @@ """Run describe page, including download, QuPath and Marimo control.""" +import asyncio +import contextlib +import queue as queue_module import webbrowser from importlib.util import find_spec from multiprocessing import Manager @@ -254,13 +257,21 @@ def _select_data() -> None: # This allows the timer callback to access the queue that's created during start_download progress_state: dict[str, Any] = {"queue": None} - def update_download_progress() -> None: # noqa: C901, PLR0912 + def update_download_progress() -> None: # noqa: C901, PLR0912, PLR0915 """Update the progress indicator with values from the queue.""" progress_queue = progress_state.get("queue") if progress_queue is None: return - while not progress_queue.empty(): - progress = progress_queue.get() + # Drain everything currently in the queue, but never block on `get()`. Using + # `get_nowait()` (instead of the previously-paired `empty()` check + blocking `get()`) + # guards against a tiny IPC race on `multiprocessing.Manager().Queue` where the queue + # could be drained between `empty()` returning False and `get()` running, leaving + # `get()` to block indefinitely and stall the async progress task. + while True: + try: + progress = progress_queue.get_nowait() + except queue_module.Empty: + break if progress.status is DownloadProgressState.DOWNLOADING_INPUT: status_text = ( f"Downloading input slide {progress.item_index + 1} of {progress.item_count}" @@ -319,12 +330,7 @@ def update_download_progress() -> None: # noqa: C901, PLR0912 download_item_progress.set_visibility(False) download_artifact_progress.set_visibility(False) - # Create the timer during dialog construction (in valid slot context), but inactive initially - # This avoids RuntimeError when trying to create timer inside async event handler - progress_timer = ui.timer(0.1, update_download_progress, active=False) - - async def start_download() -> None: - # Read from download_options dict (defined outside @ui.refreshable) to get current values + async def start_download() -> None: # noqa: PLR0915 current_qupath_project = download_options["qupath_project"] current_marimo = download_options["marimo"] current_folder = folder_state["value"] @@ -334,10 +340,42 @@ async def start_download() -> None: ui.notify("Downloading ...", type="info") progress_queue = Manager().Queue() - progress_state["queue"] = progress_queue # Store queue so timer callback can access it - - # Activate the timer now that download is starting - progress_timer.activate() + progress_state["queue"] = progress_queue + + # Drive the progress UI from a plain asyncio task instead of `ui.timer`. + # + # Background: NiceGUI 3.10's PR #5931 made `Timer._handle_delete()` cancel the timer + # whenever its parent container is cleared. The download dialog content lives inside + # `@ui.refreshable`, and `download_run_dialog_open()` calls `dialog_content.refresh()` + # on every open — so a pre-created `ui.timer` captured by this closure is always + # `_is_canceled=True` by the time the user clicks Download, and `Timer.activate()`'s + # `assert not self._is_canceled` raises silently inside NiceGUI's event-handler + # exception path (#531). + # + # Trying to *create* the timer fresh inside this click handler instead doesn't work + # either: `ui.timer` is an Element and needs a live slot context, but the click + # handler's parent_slot is the button's parent slot which has been GC'd in this + # @ui.refreshable + dialog flow ("RuntimeError: The parent element this slot belongs + # to has been deleted."). + # + # An `asyncio.create_task` running our callback doesn't need a slot context, doesn't + # depend on the refreshable lifecycle, and the callback closes over UI element refs + # (`download_item_status`, etc.) that were just created in the latest refresh and are + # valid for the duration of this download. + stop_progress = asyncio.Event() + + async def _drive_progress() -> None: + while not stop_progress.is_set(): + try: + update_download_progress() + except Exception: + logger.exception("update_download_progress raised; continuing") + try: + await asyncio.wait_for(stop_progress.wait(), timeout=0.1) + except TimeoutError: + continue + + progress_task = asyncio.create_task(_drive_progress(), name=f"download-progress-{run.run_id}") try: download_button.disable() download_button.props(add="loading") @@ -353,10 +391,9 @@ async def start_download() -> None: message = "Download returned without results folder." raise ValueError(message) # noqa: TRY301 if current_qupath_project: - if results_folder: - ui.notify("Download and QuPath project creation completed.", type="positive") - download_item_status.set_text("Opening QuPath ...") - await open_qupath(project=results_folder / "qupath", button=download_button) + ui.notify("Download and QuPath project creation completed.", type="positive") + download_item_status.set_text("Opening QuPath ...") + await open_qupath(project=results_folder / "qupath", button=download_button) elif current_marimo: ui.notify("Download and Notebook preparation completed.", type="positive") download_item_status.set_text("Opening Notebook ...") @@ -364,19 +401,30 @@ async def start_download() -> None: else: ui.notify("Download completed.", type="positive") show_in_file_manager(str(results_folder)) - except ValueError as e: + except Exception as e: + # Catching the broad `Exception` here is intentional: any unhandled exception in + # this async event handler would otherwise be silently swallowed by NiceGUI's + # outer task-exception handler, leaving the dialog stuck in the loading state + # with no completion or failure notification. We always want the user to see + # *something* and the progress task + button state cleaned up via the `finally`. + logger.exception("Download failed for run '{}'", run.run_id) ui.notify(f"Download failed: {e}", type="negative", multi_line=True) - progress_timer.deactivate() + finally: + # Force the progress task to terminate even if it's stuck in a sync call (e.g. + # a Manager().Queue() IPC hiccup): set the stop event, hard-cancel the task, + # and wait a bounded time. This guarantees `start_download` cleanup completes + # in finite time no matter what the progress driver was doing. + stop_progress.set() + progress_task.cancel() + with contextlib.suppress(asyncio.CancelledError, Exception, TimeoutError): + await asyncio.wait_for(progress_task, timeout=2.0) progress_state["queue"] = None - return - progress_timer.deactivate() - progress_state["queue"] = None - download_button.props(remove="loading") - download_button.enable() - download_item_status.set_visibility(False) - download_item_progress.set_visibility(False) - download_artifact_status.set_visibility(False) - download_artifact_progress.set_visibility(False) + download_button.props(remove="loading") + download_button.enable() + download_item_status.set_visibility(False) + download_item_progress.set_visibility(False) + download_artifact_status.set_visibility(False) + download_artifact_progress.set_visibility(False) ui.separator() with ui.row(align_items="end").classes("w-full justify-end"): diff --git a/src/aignostics/dataset/_gui.py b/src/aignostics/dataset/_gui.py index cc9065deb..1c2733e05 100644 --- a/src/aignostics/dataset/_gui.py +++ b/src/aignostics/dataset/_gui.py @@ -88,7 +88,7 @@ async def page_idc() -> None: # noqa: C901, PLR0915, RUF029 """ ) - def _on_source_input_change(e: ValueChangeEventArguments) -> None: + def _on_source_input_change(e: ValueChangeEventArguments[str | None]) -> None: """On change event.""" if download_form.download_button is None: return @@ -249,7 +249,7 @@ async def _download(source: str) -> None: with ui.button("Download", icon="cloud_download").mark("BUTTON_DOWNLOAD") as download_button: ui.tooltip("Download the selected dataset") download_form.download_button = download_button - download_form.download_button.on("click", lambda _: _download(source_input.value)) + download_form.download_button.on("click", lambda _: _download(source_input.value or "")) download_form.download_button.disable() def update_progress() -> None: diff --git a/src/aignostics/system/_gui.py b/src/aignostics/system/_gui.py index 31a33c341..54aad49ca 100644 --- a/src/aignostics/system/_gui.py +++ b/src/aignostics/system/_gui.py @@ -63,7 +63,9 @@ async def page_system() -> None: # noqa: PLR0915 # Mask secrets switch with reload functionality with ui.row().classes("w-full items-center gap-2 mb-4"): mask_secrets_switch = ui.switch( - text="Mask secrets", value=True, on_change=lambda e: load_info(mask_secrets=e.value) + text="Mask secrets", + value=True, + on_change=lambda e: load_info(mask_secrets=bool(e.value)), ) spinner = ui.spinner(size="lg").classes( diff --git a/tests/aignostics/notebook/service_test.py b/tests/aignostics/notebook/service_test.py index a353a3441..25ea7b032 100644 --- a/tests/aignostics/notebook/service_test.py +++ b/tests/aignostics/notebook/service_test.py @@ -6,8 +6,6 @@ from unittest.mock import MagicMock, patch import pytest -from fastapi.testclient import TestClient -from nicegui import app from nicegui.testing import User from aignostics.notebook._service import MARIMO_SERVER_STARTUP_TIMEOUT, Service, _get_runner, _Runner @@ -99,7 +97,7 @@ def test_notebook_start_and_stop(caplog: pytest.LogCaptureFixture) -> None: @pytest.mark.flaky(retries=1, delay=5, only_on=[AssertionError]) @pytest.mark.sequential @pytest.mark.timeout(timeout=60 * 2) -def test_serve_notebook(user: User, caplog: pytest.LogCaptureFixture) -> None: +async def test_serve_notebook(user: User, caplog: pytest.LogCaptureFixture) -> None: """Test notebook serving. Args: @@ -112,10 +110,8 @@ def test_serve_notebook(user: User, caplog: pytest.LogCaptureFixture) -> None: # Set up logging to capture DEBUG level and above caplog.set_level(logging.DEBUG) - client = TestClient(app) - try: - response = client.get("/notebook/4711?results_folder=/tmp", timeout=60) + response = await user.http_client.get("/notebook/4711?results_folder=/tmp", timeout=60) assert response.status_code == 200 content = response.content.decode("utf-8") assert "iframe" in content diff --git a/uv.lock b/uv.lock index dabe7ca7f..0bf626562 100644 --- a/uv.lock +++ b/uv.lock @@ -201,7 +201,7 @@ requires-dist = [ { name = "marshmallow", specifier = ">=3.26.2" }, { name = "matplotlib", marker = "extra == 'marimo'", specifier = ">=3.10.7,<4" }, { name = "nbconvert", marker = "extra == 'jupyter'", specifier = ">=7.17.1" }, - { name = "nicegui", extras = ["native"], specifier = ">=3.9.0,<4" }, + { name = "nicegui", extras = ["native"], specifier = ">=3.11.0,<4" }, { name = "openslide-bin", specifier = ">=4.0.0.10,<5" }, { name = "openslide-python", specifier = ">=1.4.3,<2" }, { name = "packaging", specifier = ">=26,<27" }, @@ -4319,7 +4319,7 @@ wheels = [ [[package]] name = "nicegui" -version = "3.9.0" +version = "3.11.0" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "aiofiles" }, @@ -4332,22 +4332,25 @@ dependencies = [ { name = "ifaddr" }, { name = "itsdangerous" }, { name = "jinja2" }, + { name = "lxml" }, { name = "lxml-html-clean" }, { name = "markdown2" }, { name = "orjson", marker = "platform_machine != 'i386' and platform_machine != 'i686' and platform_python_implementation != 'PyPy'" }, { name = "pydantic-core" }, { name = "pygments" }, + { name = "python-dotenv" }, { name = "python-engineio" }, { name = "python-multipart" }, { name = "python-socketio", extra = ["asyncio-client"] }, { name = "starlette" }, + { name = "tinycss2" }, { name = "typing-extensions" }, { name = "uvicorn", extra = ["standard"] }, { name = "watchfiles" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/d3/38/ed046018db555c34ebc17738284d2f85bf9a544734cd44a87311128619a5/nicegui-3.9.0.tar.gz", hash = "sha256:7ae9046b321d029c438f7cd54a697838ed1962cecb92c622912283c66c8bf8f6", size = 19031869, upload-time = "2026-03-19T09:51:52.247Z" } +sdist = { url = "https://files.pythonhosted.org/packages/29/51/32defa2f693a75efb7378cc54bc53747e3bb0fbf302e2ba91b134bd19901/nicegui-3.11.0.tar.gz", hash = "sha256:8be951c54cd425956f9e0bdfd808ecf107a12c07af6e53390e9586c05eab2513", size = 19231197, upload-time = "2026-04-24T13:12:18.626Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/81/11/f7f911f284ceb1b038c26d6f4833bc86d6583d5280156274fdb79be7dcfe/nicegui-3.9.0-py3-none-any.whl", hash = "sha256:4adfdb87a55e30b7fef05ab782efc030534ae6ad9afa330db856dfbb258e23c9", size = 19613351, upload-time = "2026-03-19T09:51:48.769Z" }, + { url = "https://files.pythonhosted.org/packages/92/7a/5ba90576eb49cb5dc7d442d6eb7789afb4cfdffed4a8d5bde087b037207a/nicegui-3.11.0-py3-none-any.whl", hash = "sha256:3607d054fd3b0ebc3b83bb3648f08e071375b06d2ea01423a0b9b9e04f887b2b", size = 19801597, upload-time = "2026-04-24T13:12:15.018Z" }, ] [package.optional-dependencies]