fix: don't swallow _ExitCli during shutdown#5519
fix: don't swallow _ExitCli during shutdown#5519lawrence3699 wants to merge 1 commit intolivekit:mainfrom
Conversation
|
|
There was a problem hiding this comment.
Pull request overview
This PR fixes a shutdown control-flow issue where _ExitCli (raised by CLI signal handlers) could be accidentally caught by broad except Exception blocks, preventing the normal drain/close shutdown path from running.
Changes:
- Change
_ExitClito inherit fromBaseExceptionso it bypasses genericexcept Exceptionhandlers. - Add a regression test ensuring
SupervisedProc._memory_monitor_task()does not log/swallow_ExitCli.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
livekit-agents/livekit/agents/cli/cli.py |
Makes _ExitCli a BaseException to ensure shutdown signals aren’t intercepted by broad exception handling. |
tests/test_drain_timeout.py |
Adds a regression test covering the _memory_monitor_task() path to ensure _ExitCli propagates and isn’t logged/swallowed. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
could you sign the CLA before we can merge? |
Closes #4664
What was happening
_ExitCliis raised from the CLI signal handlers to stop the worker. Because it inherited fromException, broadexcept Exceptionblocks could catch it before the shutdown path reacheddrain()/aclose().SupervisedProc._memory_monitor_task()is one concrete path: ifSIGTERMlands whilepsutil.Process(...)is running, the task logs and swallows_ExitCliinstead of letting the worker drain.What changed
_ExitCliinherit fromBaseException_memory_monitor_task()Why this is correct
_ExitCliis internal control flow used only for process shutdown, and the CLI already catches it explicitly at the top level. Making it bypass genericexcept Exceptionhandlers lets shutdown propagate without changing the existing explicit_ExitClihandling.Validation
uv run pytest tests/test_drain_timeout.py -quv run ruff check livekit-agents/livekit/agents/cli/cli.py tests/test_drain_timeout.py