Skip to content

fix(python-sdk): forward BLE notification callback errors to the listener - #21318

Open
kodjima33 wants to merge 1 commit into
mainfrom
issues-improver/21264-ble-notification-callback-error
Open

kodjima33 wants to merge 1 commit into
mainfrom
issues-improver/21264-ble-notification-callback-error

Conversation

@kodjima33

@kodjima33 kodjima33 commented Oct 12, 2026 •

Copy link
Copy Markdown
Collaborator

What

sdks/python/omi/ble.py::listen() (and listen_payload()) awaited the caller's
on_packet/on_payload callback result inline inside the notification handler, with no
try/except. bleak.BleakClient.start_notify detects that handler is a coroutine function
and wraps it so every notification dispatches via a detached asyncio.create_task (see
bleak/__init__.py), never awaited by the coroutine the SDK caller is actually awaiting.
So when a packet callback raised, the exception became an unretrieved task exception
logged by asyncio's default handler, and the caller of listen()/listen_payload() just
saw the listener hang instead of seeing the real failure (#21264).

Fix

Catch the exception inside the handler, store the first one, and set the same stopped
event the disconnect callback already uses. The listener's poll loop watches that event
and now re-raises the stored exception from the coroutine the caller is awaiting, instead
of depending on the notification call frame (which nothing awaits in production).

Why this needed a real-dispatch test, not just a handler-level one

The existing test_ble_callbacks.py::test_callback_exceptions_propagate only ever awaited
the captured handler directly from the test body, which is not the path bleak itself
takes in production (it schedules the handler via a detached task) -- so that test gave
false confidence. Added tests/test_ble_notification_dispatch.py, which swaps in a
duck-typed backend via BleakClient(backend=...) so the real BleakClient.start_notify
wrapping/dispatch runs, and updated the handler-level test so it no longer asserts the
call site itself raises (it must not, in production nothing awaits that call).

Testing

  • Verified the bug independently against current main by reading bleak 0.22.3's actual
    start_notify source (confirms the asyncio.create_task dispatch) before writing any
    fix.
  • bash sdks/python/test.sh: 56 passed (was 53 before; +3 new regression tests).
  • Confirmed the new/updated tests fail against the unfixed ble.py with exactly the
    reported symptom (Task exception was never retrieved, TimeoutError on the caller),
    and pass once the fix is applied.
  • No physical BLE hardware involved; the duck-typed backend only replaces device I/O,
    bleak's own dispatch/wrapping code is unmodified and still runs.

Failure-Class: new

@TuEmb @mdmohsin7 — BLE/device-link change per repo convention, requesting your review
before merge.

Auto-generated from issue feedback by the mini issues-improver — tested but NOT merged
(BLE exception: requires BLE-owner review per this job's rules).

fixes #21264

View guided diff Turn on auto-fix

…ener

bleak.BleakClient.start_notify wraps an async notification callback so each
notification dispatches via a detached asyncio.create_task, never awaited by
the coroutine listen()/listen_payload() callers actually await. A packet
callback exception therefore became an unretrieved task exception instead of
reaching the caller, which just saw the listener hang.

Catch the exception inside the handler, store the first one, and signal the
same event the disconnect callback already uses; the poll loop re-raises it
from the coroutine the caller awaits.

Added tests/test_ble_notification_dispatch.py, which swaps in a duck-typed
backend via BleakClient(backend=...) so bleak's real start_notify
wrapping/dispatch runs (the existing test_ble_callbacks.py coverage only ever
awaited the captured handler directly, a path bleak itself never takes).
Updated that existing test to match the new contract: the notification call
site itself must not raise, since nothing awaits it in production.

Refs #21264

This branch has not been deployed

No deployments
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.

Python SDK BLE callback errors never reach the awaiting listener

1 participant