Skip to content

fix(ble): wrap proxy transport timeouts#57

Merged
Bre77 merged 3 commits into
mainfrom
fm/tfa-ble-captain-present-phase
Jul 10, 2026
Merged

fix(ble): wrap proxy transport timeouts#57
Bre77 merged 3 commits into
mainfrom
fm/tfa-ble-captain-present-phase

Conversation

@Bre77

@Bre77 Bre77 commented Jul 9, 2026

Copy link
Copy Markdown
Member

Intent

Fix a confirmed BLE exception-typing bug found live during a captain-present BLE verification session. VehicleBluetooth._send (GATT write) and connect() (establish_connection + start_notify) wrapped only bleak.exc.BleakError in BluetoothTransportError, but bleak-esphome's @api_error_as_bleak_error decorator converts an aioesphomeapi TimeoutAPIError into a builtin TimeoutError (not a BleakError). So a GATT-write, connect, or start_notify timeout over an ESPHome proxy escaped the wrap as a bare TimeoutError - which is NOT a TeslaFleetError, defeating the library's documented 'catch TeslaFleetError and you're covered' contract (the same contract PR #55 established). Reproduced live twice over an ESPHome proxy (adjust_volume and the wake path both raised 'TimeoutError: Timeout waiting for BluetoothGATTWriteResponse ... after 30.0s'). Deliberate decision: broaden BOTH catch sites from 'except BleakError' to 'except (BleakError, TimeoutError)' so every transport-layer timeout surfaces as BluetoothTransportError with the original exception chained as cause; BluetoothTransportError (not BluetoothTimeout) is the right target because it is what the same call sites already map BleakError to, and a write/connect timeout is a transport failure distinct from the response-wait BluetoothTimeout. Added 3 regression tests in tests/test_ble_send_transport.py (write, establish_connection, start_notify timeout paths) - each fails before the fix, leaking a bare TimeoutError, and passes after. Updated the AGENTS.md Error Handling note to state both exception types are caught and why. Scope deliberately minimal: only the two catch sites plus tests and the doc note; no behavior change beyond exception typing. A BLE test harness under scripts/ble-harness/ is intentionally git-excluded and not part of this change.

What Changed

  • Wrap BLE proxy TimeoutError failures from GATT writes, connection setup, and notification setup as BluetoothTransportError, preserving the original exception as the cause so callers can continue catching TeslaFleetError.
  • Reset the BLE client after connection setup failures so retries do not reuse a connected client without notifications registered.
  • Add regression coverage for write, connect, and start_notify timeout paths, and update BLE transport docs to describe the widened transport error handling.

Risk Assessment

✅ Low: The changed code is narrowly scoped to wrapping BLE transport TimeoutError paths and cleaning up partial connect state; I do not see a remaining merge-blocking or follow-up-worthy defect in the diff.

Testing

Inspected the BLE transport diff, ran the focused BLE transport regression file and the full test suite, then produced caller-level evidence showing write/connect/start-notify builtin TimeoutErrors are caught as TeslaFleetError via BluetoothTransportError with the original cause preserved; all checks passed and transient pytest caches were removed.

Evidence: BLE transport timeout caller-contract transcript

BLE transport timeout caller contract BluetoothTransportError is TeslaFleetError: True GATT write TimeoutError: caught=BluetoothTransportError; cause=TimeoutError; preserved=True establish_connection TimeoutError: caught=BluetoothTransportError; cause=TimeoutError; preserved=True start_notify TimeoutError: caught=BluetoothTransportError; cause=TimeoutError; preserved=True

BLE transport timeout caller contract
BluetoothTransportError is TeslaFleetError: True
GATT write TimeoutError: caught=BluetoothTransportError; cause=TimeoutError; preserved=True
establish_connection TimeoutError: caught=BluetoothTransportError; cause=TimeoutError; preserved=True
start_notify TimeoutError: caught=BluetoothTransportError; cause=TimeoutError; preserved=True

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

🔧 **Review** - 1 issue found → auto-fixed ✅
  • ⚠️ tesla_fleet_api/tesla/vehicle/bluetooth.py:272 - When start_notify() raises a newly wrapped TimeoutError, self.client has already been assigned to the connected client. A caller that catches BluetoothTransportError and retries via connect_if_needed() can then skip connect() because self.client.is_connected is still true, leaving notifications unregistered; subsequent _send() calls can write successfully but never receive queued replies and degrade into BluetoothTimeout. Clear or disconnect self.client on any connect() setup failure before raising BluetoothTransportError, and cover the start_notify failure case in the regression test.

🔧 Fix: Reset BLE client after setup failure
✅ Re-checked - no issues remain.

✅ **Test** - passed

✅ No issues found.

  • Inspected changed files with git diff --stat 2300b74fbd1668496ae5653cdf18865b73fd37e8..b10e7ad677bd85226105a708815517247240f4d7, git diff --name-only 2300b74fbd1668496ae5653cdf18865b73fd37e8..b10e7ad677bd85226105a708815517247240f4d7, and targeted diffs for bluetooth.py / test_ble_send_transport.py.
  • uv run pytest tests/test_ble_send_transport.py
  • uv run python - <<'PY' > /tmp/no-mistakes-evidence/01KX4MB0MKKVHZ872480NS3HPH/ble_transport_timeout_contract.txt ... PY
  • uv run pytest tests
  • Cleaned pytest transient artifacts with find . -type d \( -name &#39;__pycache__&#39; -o -name &#39;.pytest_cache&#39; \) -prune -exec rm -rf {} + and confirmed git status --short was clean.
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

firstmate crewmate added 3 commits July 10, 2026 09:43
VehicleBluetooth._send and connect() wrapped only bleak.exc.BleakError from
their bleak-esphome client calls in BluetoothTransportError. But bleak-esphome's
@api_error_as_bleak_error decorator converts an aioesphomeapi TimeoutAPIError
into a builtin TimeoutError (not a BleakError), so a GATT-write, connect, or
start_notify timeout escaped the wrap as a bare TimeoutError - not a
TeslaFleetError, defeating the documented `except TeslaFleetError` contract.

Reproduced live over an ESPHome proxy (adjust_volume and wake paths both raised
`TimeoutError: Timeout waiting for BluetoothGATTWriteResponse ... after 30.0s`).
Broaden both catch sites to (BleakError, TimeoutError) so every transport-layer
timeout surfaces as BluetoothTransportError with the original exception chained.

Add regression tests covering the write, establish_connection, and start_notify
timeout paths (each fails before this change, leaking a bare TimeoutError).
@Bre77
Bre77 merged commit 759d6fa into main Jul 10, 2026
5 checks passed
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.

1 participant