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

Quiet KeyboardInterrupt #2383

Closed
wants to merge 4 commits into from
Closed

Conversation

maxfischer2781
Copy link
Contributor

Summary

This PR quiets the KeyboardInterrupt when quitting a programatic run of uvicorn.

  • The default traceback is much shorter now. It shows that it originates in uvicorn and notes that it quits uvicorn to avoid confusing it with a regular error.

    New traceback
    INFO:     Started server process [33977]
    INFO:     Waiting for application startup.
    INFO:     Application startup complete.
    INFO:     Uvicorn running on http://127.0.0.1:8000 (Press CTRL+C to quit)
    ^CINFO:     Shutting down
    INFO:     Waiting for application shutdown.
    INFO:     Application shutdown complete.
    INFO:     Finished server process [33977]
    Traceback (most recent call last):
      File "/Users/maxfischer/code_projects/uvicorn/run_uvicorn.py", line 3, in <module>
        uvicorn.run("test:dummy_app")
      File "/Users/maxfischer/code_projects/uvicorn/uvicorn/main.py", line 584, in run
        raise KeyboardInterrupt("quit uvicorn") from None
    KeyboardInterrupt: quit uvicorn
    
    Old traceback
    INFO:     Started server process [33710]
    INFO:     Waiting for application startup.
    INFO:     Application startup complete.
    INFO:     Uvicorn running on http://127.0.0.1:8000 (Press CTRL+C to quit)
    ^CINFO:     Shutting down
    INFO:     Waiting for application shutdown.
    INFO:     Application shutdown complete.
    INFO:     Finished server process [33710]
    Traceback (most recent call last):
      File "/opt/homebrew/Cellar/python@3.12/3.12.2_1/Frameworks/Python.framework/Versions/3.12/lib/python3.12/asyncio/runners.py", line 118, in run
        return self._loop.run_until_complete(task)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "uvloop/loop.pyx", line 1517, in uvloop.loop.Loop.run_until_complete
    asyncio.exceptions.CancelledError
    
    During handling of the above exception, another exception occurred:
    
    Traceback (most recent call last):
      File "/Users/maxfischer/code_projects/uvicorn/run_uvicorn.py", line 3, in <module>
        uvicorn.run("test:dummy_app")
      File "/Users/maxfischer/code_projects/uvicorn/uvicorn/main.py", line 577, in run
        server.run()
      File "/Users/maxfischer/code_projects/uvicorn/uvicorn/server.py", line 65, in run
        return asyncio.run(self.serve(sockets=sockets))
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "/opt/homebrew/Cellar/python@3.12/3.12.2_1/Frameworks/Python.framework/Versions/3.12/lib/python3.12/asyncio/runners.py", line 194, in run
        return runner.run(main)
               ^^^^^^^^^^^^^^^^
      File "/opt/homebrew/Cellar/python@3.12/3.12.2_1/Frameworks/Python.framework/Versions/3.12/lib/python3.12/asyncio/runners.py", line 123, in run
        raise KeyboardInterrupt()
    KeyboardInterrupt
    
  • uvicorn.run now has a flag suppress_ki that completely silences the KeyboardInterrupt. This allows programmatic usage to almost restore the pre-Cooperative signal handling #1600 behaviour - the noisy exception is gone (as before), but triggering CTRL+C again still allows to shut down the program (as is usual for Python).

Open design decisions

Should suppress_ki default to True? It's no problem for the CLI to set it explicitly to False and async code (which motivated #1600) does not go through this function at all.

Is suppress_ki a suitable name? ki is commonly used as an abbreviation of KeyboardInterrupt (see e.g. trio) but usually only needed in very technical situations.

Checklist

  • I understand that this PR may be closed in case there was no previous discussion. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.

@Kludex
Copy link
Member

Kludex commented Jul 5, 2024

Why not always suppressing on uvicorn.run?

@maxfischer2781
Copy link
Contributor Author

There should at least be some way to not suppress KI in CLI and programmatic use, since that is what well-behaved programs should do. I don't have any strong opinion on whether that should be the default or not, though.

@Kludex
Copy link
Member

Kludex commented Jul 5, 2024

There should at least be some way to not suppress KI in CLI and programmatic use, since that is what well-behaved programs should do. I don't have any strong opinion on whether that should be the default or not, though.

I don't think it should on uvicorn.run, on Server.serve() makes sense.

I want to maintain the previous behavior.

@maxfischer2781
Copy link
Contributor Author

Okay, see #2384.

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