-
-
Notifications
You must be signed in to change notification settings - Fork 743
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
Fix crash on exist with "--uds" if socket not exists #1725
Conversation
Do you have a MRE? |
Yes: With this app ( from starlette.applications import Starlette
app = Starlette()
@app.on_event("startup")
async def startup():
raise RuntimeError And the command INFO: Started server process [28861]
INFO: Waiting for application startup.
ERROR: Traceback (most recent call last):
File "/home/fedora/.cache/pypoetry/virtualenvs/mre-Y0ZI6pHK-py3.10/lib/python3.10/site-packages/starlette/routing.py", line 645, in lifespan
async with self.lifespan_context(app):
File "/home/fedora/.cache/pypoetry/virtualenvs/mre-Y0ZI6pHK-py3.10/lib/python3.10/site-packages/starlette/routing.py", line 540, in __aenter__
await self._router.startup()
File "/home/fedora/.cache/pypoetry/virtualenvs/mre-Y0ZI6pHK-py3.10/lib/python3.10/site-packages/starlette/routing.py", line 622, in startup
await handler()
File "/home/fedora/Documents/./mre.py", line 7, in startup
raise RuntimeError
RuntimeError
ERROR: Application startup failed. Exiting.
Traceback (most recent call last):
File "/home/fedora/.cache/pypoetry/virtualenvs/mre-Y0ZI6pHK-py3.10/bin/uvicorn", line 8, in <module>
sys.exit(main())
File "/home/fedora/.cache/pypoetry/virtualenvs/mre-Y0ZI6pHK-py3.10/lib/python3.10/site-packages/click/core.py", line 1130, in __call__
return self.main(*args, **kwargs)
File "/home/fedora/.cache/pypoetry/virtualenvs/mre-Y0ZI6pHK-py3.10/lib/python3.10/site-packages/click/core.py", line 1055, in main
rv = self.invoke(ctx)
File "/home/fedora/.cache/pypoetry/virtualenvs/mre-Y0ZI6pHK-py3.10/lib/python3.10/site-packages/click/core.py", line 1404, in invoke
return ctx.invoke(self.callback, **ctx.params)
File "/home/fedora/.cache/pypoetry/virtualenvs/mre-Y0ZI6pHK-py3.10/lib/python3.10/site-packages/click/core.py", line 760, in invoke
return __callback(*args, **kwargs)
File "/home/fedora/.cache/pypoetry/virtualenvs/mre-Y0ZI6pHK-py3.10/lib/python3.10/site-packages/uvicorn/main.py", line 404, in main
run(
File "/home/fedora/.cache/pypoetry/virtualenvs/mre-Y0ZI6pHK-py3.10/lib/python3.10/site-packages/uvicorn/main.py", line 571, in run
os.remove(config.uds) # pragma: py-win32
FileNotFoundError: [Errno 2] No such file or directory: '/tmp/uvicorn.sock' |
Thanks. I'll check later today. 🙏 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @JGoutin ! 🎉
I've tweaked a bit one of the tests to make sure we'll not have any regressions here.
@@ -95,7 +95,6 @@ def test_cli_call_multiprocess_run() -> None: | |||
def test_cli_uds(tmp_path: Path) -> None: # pragma: py-win32 | |||
runner = CliRunner() | |||
uds_file = tmp_path / "uvicorn.sock" | |||
uds_file.touch(exist_ok=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change has reduced coverage on this file. Added back in #1741
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which line was reduced?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line 571 in main.py
due to this os.path.exists(config.uds)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's test both scenarios - I'm already on it.
Co-authored-by: Marcelo Trylesinski <marcelotryle@gmail.com>
Co-authored-by: Marcelo Trylesinski <marcelotryle@gmail.com>
Currently uvicorn may crash when exiting if run with the "--uds" option, and if Uvicorn exits before the socket creation (For instance if the application to run crash on statup).
This PR fix the issue by ignoring the exception in that case.