Merge pull request #2088 from zauberzeug/late_returns

Warn about late returns in @ui.page builder, which were silently dropped before

nicegui/page.py

@@ -12,6 +12,7 @@ from . import background_tasks, binding, core, helpers
 from .client import Client
 from .favicon import create_favicon_route
 from .language import Language
+from .logging import log
 
 if TYPE_CHECKING:
     from .api_router import APIRouter
@@ -87,6 +88,10 @@ class page:
         core.app.remove_route(self.path)  # NOTE make sure only the latest route definition is used
         parameters_of_decorated_func = list(inspect.signature(func).parameters.keys())
 
+        def check_for_late_return_value(task: asyncio.Task) -> None:
+            if task.result() is not None:
+                log.error(f'ignoring {task.result()}; it was returned after the HTML had been delivered to the client')
+
         async def decorated(*dec_args, **dec_kwargs) -> Response:
             request = dec_kwargs['request']
             # NOTE cleaning up the keyword args so the signature is consistent with "func" again
@@ -105,7 +110,11 @@ class page:
                     if time.time() > deadline:
                         raise TimeoutError(f'Response not ready after {self.response_timeout} seconds')
                     await asyncio.sleep(0.1)
-                result = task.result() if task.done() else None
+                if task.done():
+                    result = task.result()
+                else:
+                    result = None
+                    task.add_done_callback(check_for_late_return_value)
             if isinstance(result, Response):  # NOTE if setup returns a response, we don't need to render the page
                 return result
             binding._refresh_step()  # pylint: disable=protected-access

tests/screen.py

@@ -1,8 +1,9 @@
 import os
+import re
 import threading
 import time
 from contextlib import contextmanager
-from typing import List, Optional
+from typing import List, Optional, Union
 
 import pytest
 from selenium import webdriver
@@ -194,14 +195,18 @@ class Screen:
         print(f'Storing screenshot to {filename}')
         self.selenium.get_screenshot_as_file(filename)
 
-    def assert_py_logger(self, level: str, message: str) -> None:
-        """Assert that the Python logger has received a message with the given level and text."""
+    def assert_py_logger(self, level: str, message: Union[str, re.Pattern]) -> None:
+        """Assert that the Python logger has received a message with the given level and text or regex pattern."""
         try:
             assert self.caplog.records, 'Expected a log message'
             record = self.caplog.records[0]
             print(record.levelname, record.message)
             assert record.levelname.strip() == level, f'Expected "{level}" but got "{record.levelname}"'
-            assert record.message.strip() == message, f'Expected "{message}" but got "{record.message}"'
+
+            if isinstance(message, re.Pattern):
+                assert message.search(record.message), f'Expected regex "{message}" but got "{record.message}"'
+            else:
+                assert record.message.strip() == message, f'Expected "{message}" but got "{record.message}"'
         finally:
             self.caplog.records.clear()
 

tests/test_page.py

@@ -1,4 +1,5 @@
 import asyncio
+import re
 from uuid import uuid4
 
 from fastapi.responses import PlainTextResponse
@@ -286,6 +287,18 @@ def test_returning_custom_response_async(screen: Screen):
     screen.should_not_contain('normal NiceGUI page')
 
 
+def test_warning_about_to_late_responses(screen: Screen):
+    @ui.page('/')
+    async def page(client: Client):
+        await client.connected()
+        ui.label('NiceGUI page')
+        return PlainTextResponse('custom response')
+
+    screen.open('/')
+    screen.should_contain('NiceGUI page')
+    screen.assert_py_logger('ERROR', re.compile('it was returned after the HTML had been delivered to the client'))
+
+
 def test_reconnecting_without_page_reload(screen: Screen):
     @ui.page('/', reconnect_timeout=3.0)
     def page():