Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Page decorator not awaiting page functions #1870

Closed
ProbablyBrian opened this issue Oct 24, 2023 · 8 comments · Fixed by #1877
Closed

Page decorator not awaiting page functions #1870

ProbablyBrian opened this issue Oct 24, 2023 · 8 comments · Fixed by #1877
Labels
bug Something isn't working help wanted Extra attention is needed
Milestone

Comments

@ProbablyBrian
Copy link

Description

Just upgraded to 1.4.0 today and am running into the following problems. Code example to reproduce:

from fastapi import FastAPI
from nicegui import ui

app = FastAPI()


@ui.page("/")
async def homepage():
    ui.label("Homepage!")


ui.run_with(app)

starting this with

$ uvicorn test:app

When navigating to the page, I receive a 500 Internal Server Error. The traceback is extensive but there are a few warnings emitted about coroutines not being awaited:

[...snip FastAPI stack...]
  File "/home/brian/portal/venv_new/lib/python3.11/site-packages/fastapi/routing.py", line 191, in run_endpoint_function
    return await dependant.call(**values)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/brian/portal/venv_new/lib/python3.11/site-packages/nicegui/page.py", line 102, in decorated
    task = background_tasks.create(wait_for_result())
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/brian/portal/venv_new/lib/python3.11/site-packages/nicegui/background_tasks.py", line 21, in create
    assert core.loop is not None
           ^^^^^^^^^^^^^^^^^^^^^
AssertionError

/usr/lib/python3.11/threading.py:247: RuntimeWarning: coroutine 'page.__call__.<locals>.decorated.<locals>.wait_for_result' was never awaited
  self._release_save = lock._release_save
RuntimeWarning: Enable tracemalloc to get the object allocation traceback
/usr/lib/python3.11/threading.py:247: RuntimeWarning: coroutine 'homepage' was never awaited
  self._release_save = lock._release_save
RuntimeWarning: Enable tracemalloc to get the object allocation traceback

I downgraded back to 1.3.18 and ran the exact code and this works fine in the previous version.

Versions:

  • Python 3.11.5
  • Nicegui 1.4.0
  • Starlette 0.27.0
  • FastAPI 0.104.0
  • Uvicorn 0.22.0

Other notes:

  • The behavior is the same with nicegui.APIRouter's page decorator (the way I initially discovered this issue).
  • using ui.run() with no FastAPI Mount works just fine.

Is this something I'm doing incorrectly with the new 1.4.0 version?

@falkoschindler
Copy link
Contributor

Thanks for reporting this issue, @ProbablyBrian!

@rodja I think we broke something when switching to FastAPI's lifespan API: _startup in nicegui.py is only called by our _lifespan, which in turn is only used for creating our core.app. But it doesn't seem to be called when it is attached to an existing app via app.mount(mount_path, core.app) in ui.run_with. This leaves core.loop set to None.

@falkoschindler falkoschindler added the bug Something isn't working label Oct 24, 2023
@falkoschindler falkoschindler added this to the 1.4.1 milestone Oct 24, 2023
@falkoschindler
Copy link
Contributor

The FastAPI documentation clearly says:

🚨 Have in mind that these lifespan events (startup and shutdown) will only be executed for the main application, not for Sub Applications - Mounts.

@falkoschindler
Copy link
Contributor

This seems to work:

# ui_run_with.py
...
app.mount(mount_path, core.app)
app.on_event('startup')(_startup)
app.on_event('shutdown')(_shutdown)

Or as a workaround for the user code:

from fastapi import FastAPI
from nicegui import ui
from nicegui.nicegui import _shutdown, _startup

app = FastAPI()

@ui.page("/")
async def homepage():
    ui.label("Homepage!")

ui.run_with(app)
app.on_event('startup')(_startup)
app.on_event('shutdown')(_shutdown)

@rodja
Copy link
Member

rodja commented Oct 25, 2023

This issue is originating in Starlette (see encode/starlette#649). I think

from nicegui.nicegui import _shutdown, _startup
...
ui.run_with(app)
app.on_event('startup')(_startup)
app.on_event('shutdown')(_shutdown)

will only work if you do not use the lifespan-API in your main application. If you use these, I guess you would need to run

from nicegui.nicegui import _shutdown, _startup
...
ui.run_with(app)
app.lifespan.add_event_handler("startup", _startup)
app.lifespan.add_event_handler("shutdown", _shutdown)

Quite ugly in my opinion. Maybe we can add this magic to ui.run_with, checking if the app has a lifespan object?

@falkoschindler
Copy link
Contributor

@rodja Oh, I see. You mean something like

def lifespan(app):
    print('lifespan start')
    yield
    print('lifespan end')

app = FastAPI(lifespan=lifespan)

This doesn't work indeed.
Unfortunately there is no lifespan attribute in app. But maybe we find another indicator.

@falkoschindler falkoschindler added the help wanted Extra attention is needed label Oct 25, 2023
@falkoschindler
Copy link
Contributor

I'm pretty lost. I don't find a way to register any startup or shutdown handler if lifespan is set.

Let's ask StackOverflow: https://stackoverflow.com/questions/77362216/

@falkoschindler
Copy link
Contributor

falkoschindler commented Oct 26, 2023

I think I found a solution.

ui_run_with.py:

app.mount(mount_path, core.app)
main_app_lifespan = app.router.lifespan_context

@asynccontextmanager
async def lifespan_wrapper(app):
    _startup()
    async with main_app_lifespan(app) as maybe_state:
        yield maybe_state
    _shutdown()

app.router.lifespan_context = lifespan_wrapper

It seems hacky, but works with and without lifespan on the main app.

@falkoschindler
Copy link
Contributor

I updated PR #1877 accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants