Skip to content

Catch DisconnectedError in create_task wrapper instead of per-coroutine#806

Open
gemenerik wants to merge 1 commit intoAris/cflib2/DisconnectedErrorfrom
rik/cflib2/DisconnectedError
Open

Catch DisconnectedError in create_task wrapper instead of per-coroutine#806
gemenerik wants to merge 1 commit intoAris/cflib2/DisconnectedErrorfrom
rik/cflib2/DisconnectedError

Conversation

@gemenerik
Copy link
Copy Markdown
Member

The existing _task_done_callback was meant to catch DisconnectedError for all tasks created via create_task, but PySide6's QtAsyncio logs unhandled task exceptions before done callbacks run. So the callback did catch the error (preventing os._exit), but the traceback was already printed. By wrapping the coroutine inside create_task, the exception is caught before it ever leaves the coroutine, so PySide6 never sees it. This also removes the now-redundant per-coroutine except DisconnectedError: pass blocks and the dead except DisconnectedError from _task_done_callback.

PySide6's QtAsyncio prints tracebacks for unhandled task exceptions
before done callbacks run, so the existing _task_done_callback catch
was too late. Wrap coroutines in create_task to catch DisconnectedError
before it reaches PySide6, and remove the now-redundant per-coroutine
except blocks.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR changes how DisconnectedError is handled for background asyncio tasks to avoid PySide6/QtAsyncio printing “unhandled task exception” tracebacks before task done-callbacks run, by catching DisconnectedError inside the create_task() wrapper rather than inside each streaming coroutine.

Changes:

  • Wrap all create_task() coroutines with _wrap_disconnected() to swallow DisconnectedError before it escapes the coroutine.
  • Remove now-redundant per-coroutine except DisconnectedError: pass blocks from streaming loops.
  • Remove the dead DisconnectedError handling branch from _task_done_callback().

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
src/cfclient/gui.py Adds coroutine wrapper to swallow DisconnectedError before QtAsyncio can log it; simplifies done-callback handling.
src/cfclient/ui/tabs/FlightTab.py Removes redundant DisconnectedError swallowing from motor-streaming loop.
src/cfclient/ui/pose_logger.py Removes redundant DisconnectedError swallowing from pose-streaming loop.
src/cfclient/ui/main.py Removes redundant DisconnectedError swallowing from battery-streaming loop.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

logger.debug("Task interrupted by disconnect")


def create_task(coro: Coroutine[object, object, object]) -> asyncio.Task[object]:
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

create_task() now schedules _wrap_disconnected(coro), which always completes with None (and swallows DisconnectedError). The function signature still advertises -> asyncio.Task[object], which is no longer accurate and can break type-checking / mislead callers. Consider updating the signature to Task[None], or making _wrap_disconnected generic and returning the original coroutine’s result (optionally T | None if you want to swallow disconnects).

Copilot uses AI. Check for mistakes.
try:
await coro
except DisconnectedError:
logger.debug("Task interrupted by disconnect")
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The disconnect-swallowing debug log in _wrap_disconnected() loses the task/coroutine identity (previously it logged the task). Including task.get_name() / repr(coro) / asyncio.current_task() (or passing a label into create_task) would make disconnect-related termination easier to trace when multiple background tasks are running.

Suggested change
logger.debug("Task interrupted by disconnect")
task = asyncio.current_task()
if task is not None:
logger.debug(
"Task %s (%r) interrupted by disconnect",
task.get_name(),
coro,
)
else:
logger.debug("Task interrupted by disconnect (%r)", coro)

Copilot uses AI. Check for mistakes.
Comment on lines 86 to 90
"""Schedule a coroutine as a task with automatic exception logging.

Use this instead of asyncio.ensure_future() to ensure exceptions
are logged immediately rather than silently swallowed.
"""
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

create_task() docstring says it ensures exceptions are logged rather than swallowed; after this change DisconnectedError is intentionally swallowed before the task completes. Consider updating the docstring to explicitly document that DisconnectedError is treated as a normal shutdown case and won’t be logged/propagated.

Copilot uses AI. Check for mistakes.
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