diff --git a/AGENTS.md b/AGENTS.md index e3de334..d6beff3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -99,7 +99,7 @@ No release-please or version-bump automation. To ship: bump `version` in `pyproj `exceptions.py` maps HTTP status codes and error keys to specific exception classes. `raise_for_status()` parses responses and raises the appropriate exception. Signed command faults have separate hierarchies: `TeslaFleetInformationFault`, `TeslaFleetMessageFault`, `SignedMessageInformationFault`, `WhitelistOperationStatus`. -All exceptions inherit from `TeslaFleetError(BaseException)`, deliberately **not** `Exception` — a bare `except Exception` (e.g. in retry/backoff loops around BLE reads) silently fails to catch `BluetoothTimeout` and every other library error. Catch `TeslaFleetError` (or `BaseException`) explicitly. `VehicleBluetooth` wraps transport-layer failures (`connect`/`connect_if_needed`, the GATT write in `_send`) in `BluetoothTransportError`, a `TeslaFleetError` subclass chaining the original `bleak.exc.BleakError` as its cause — so `except TeslaFleetError` alone now catches BLE transport failures too, not just the response-wait `BluetoothTimeout`. +All exceptions inherit from `TeslaFleetError(BaseException)`, deliberately **not** `Exception` — a bare `except Exception` (e.g. in retry/backoff loops around BLE reads) silently fails to catch `BluetoothTimeout` and every other library error. Catch `TeslaFleetError` (or `BaseException`) explicitly. `VehicleBluetooth` wraps transport-layer failures (`connect`/`connect_if_needed`, notification setup, the GATT write in `_send`) in `BluetoothTransportError`, a `TeslaFleetError` subclass chaining the original transport exception as its cause — so `except TeslaFleetError` alone now catches BLE transport failures too, not just the response-wait `BluetoothTimeout`. These catch sites deliberately catch **both** `bleak.exc.BleakError` and builtin `TimeoutError`: bleak-esphome converts an aioesphomeapi GATT/connect/notify timeout into a bare `TimeoutError` (not a `BleakError`), which would otherwise escape the wrap as a non-`TeslaFleetError`. ### Protobuf diff --git a/README.md b/README.md index 702fd3f..c283e46 100644 --- a/README.md +++ b/README.md @@ -166,10 +166,11 @@ asyncio.run(main()) For more detailed examples, see [Bluetooth for Vehicles](docs/bluetooth_vehicles.md). -BLE connect and GATT write failures from `VehicleBluetooth` raise +BLE connect/notify and GATT write failures from `VehicleBluetooth` raise `BluetoothTransportError`, a `TeslaFleetError` subclass, with the original -`bleak.exc.BleakError` chained as `__cause__`. Catch `TeslaFleetError` to -handle Bluetooth transport failures and `BluetoothTimeout` response-wait +transport exception chained as `__cause__`. Catch `TeslaFleetError` to handle +Bluetooth transport failures (including `bleak.exc.BleakError` and builtin +`TimeoutError` from ESPHome proxies) and `BluetoothTimeout` response-wait timeouts through the same library error hierarchy. ### Routing and Failover diff --git a/docs/bluetooth_vehicles.md b/docs/bluetooth_vehicles.md index 8dfac6d..7e56eb0 100644 --- a/docs/bluetooth_vehicles.md +++ b/docs/bluetooth_vehicles.md @@ -92,9 +92,11 @@ should retry `BluetoothTimeout` with backoff. Keep one BLE connection open across related commands when possible instead of reconnecting for each command. `VehicleBluetooth` raises `BluetoothTransportError`, a `TeslaFleetError` -subclass, when the BLE connection or GATT command write fails before a vehicle -response can be awaited. The original `bleak.exc.BleakError` is available as -the exception's `__cause__`. Catch `TeslaFleetError` to handle both transport +subclass, when the BLE connection, notification setup, or GATT command write +fails before a vehicle response can be awaited. The original transport +exception is available as the exception's `__cause__`; this includes +`bleak.exc.BleakError` and builtin `TimeoutError` from ESPHome proxy connect, +notify, or write timeouts. Catch `TeslaFleetError` to handle both transport failures and response-wait `BluetoothTimeout` failures with one library error hierarchy, or catch `BluetoothTransportError` separately when you need to distinguish a transport failure from a vehicle timeout. diff --git a/tesla_fleet_api/exceptions.py b/tesla_fleet_api/exceptions.py index c627923..2d28f7a 100644 --- a/tesla_fleet_api/exceptions.py +++ b/tesla_fleet_api/exceptions.py @@ -29,7 +29,7 @@ class BluetoothTimeout(TeslaFleetError): class BluetoothTransportError(TeslaFleetError): - """The Bluetooth transport (connect or GATT write) failed before a vehicle response could be awaited.""" + """The Bluetooth transport (connect, notify, or GATT write) failed before a vehicle response could be awaited.""" message = ( "The Bluetooth transport failed before a vehicle response could be awaited." diff --git a/tesla_fleet_api/tesla/vehicle/bluetooth.py b/tesla_fleet_api/tesla/vehicle/bluetooth.py index 3983d46..8c4fb7d 100644 --- a/tesla_fleet_api/tesla/vehicle/bluetooth.py +++ b/tesla_fleet_api/tesla/vehicle/bluetooth.py @@ -179,9 +179,10 @@ class VehicleBluetooth(Commands[BluetoothParentT], Generic[BluetoothParentT]): """Class describing the Tesla Fleet API vehicle endpoints and commands for a specific vehicle with command signing. Callers can catch failures from this class with a single ``TeslaFleetError``: - connect/write transport failures surface as ``BluetoothTransportError`` and - a response-wait timeout as ``BluetoothTimeout``, both ``TeslaFleetError`` - subclasses with the original transport exception chained as their cause. + connect/notify/write transport failures surface as + ``BluetoothTransportError`` and a response-wait timeout as + ``BluetoothTimeout``, both ``TeslaFleetError`` subclasses with the original + transport exception chained as their cause. A ``BluetoothTimeout`` raised by a *mutating* command (RKE/closure actions, HVAC/media/charging commands, ``wake_up``) is inconclusive, not @@ -266,7 +267,17 @@ async def connect(self, max_attempts: int = MAX_CONNECT_ATTEMPTS) -> None: services=[SERVICE_UUID], ) await self.client.start_notify(READ_UUID, self._on_notify) - except BleakError as e: + # bleak-esphome converts an aioesphomeapi transport timeout into a + # builtin TimeoutError, not a BleakError, so catch both to keep every + # connect transport failure within TeslaFleetError. + except (BleakError, TimeoutError) as e: + client = self.client + self.client = None + if client: + try: + await client.disconnect() + except (BleakError, TimeoutError): + pass raise BluetoothTransportError from e async def disconnect(self) -> bool: @@ -344,7 +355,10 @@ async def _send( assert self.client is not None try: await self.client.write_gatt_char(WRITE_UUID, payload, True) - except BleakError as e: + # bleak-esphome converts an aioesphomeapi write timeout into a + # builtin TimeoutError, not a BleakError, so catch both to keep the + # GATT-write transport failure within TeslaFleetError. + except (BleakError, TimeoutError) as e: raise BluetoothTransportError from e # Process the response diff --git a/tests/test_ble_send_transport.py b/tests/test_ble_send_transport.py index da633e1..c51b2c4 100644 --- a/tests/test_ble_send_transport.py +++ b/tests/test_ble_send_transport.py @@ -175,6 +175,24 @@ async def test_mid_write_gatt_failure_raises_bluetooth_transport_error( self.assertIs(ctx.exception.__cause__, underlying) + async def test_write_gatt_timeout_raises_bluetooth_transport_error( + self, + ) -> None: + # bleak-esphome surfaces an aioesphomeapi GATT-write timeout as a + # builtin TimeoutError (not a BleakError); it must still reach callers + # as a TeslaFleetError, not a bare TimeoutError. + vehicle = _make_vehicle() + msg = _outgoing() + underlying = TimeoutError( + "Timeout waiting for BluetoothGATTWriteResponse after 30.0s" + ) + vehicle.client.write_gatt_char = AsyncMock(side_effect=underlying) + + with self.assertRaises(BluetoothTransportError) as ctx: + await vehicle._send(msg, "protobuf_message_as_bytes") + + self.assertIs(ctx.exception.__cause__, underlying) + class ConnectTransportErrorTests(IsolatedAsyncioTestCase): async def test_establish_connection_failure_raises_bluetooth_transport_error( @@ -210,3 +228,48 @@ async def test_connect_if_needed_propagates_transport_error(self) -> None: await vehicle.connect_if_needed() self.assertIs(ctx.exception.__cause__, underlying) + + async def test_establish_connection_timeout_raises_bluetooth_transport_error( + self, + ) -> None: + # bleak-esphome surfaces an aioesphomeapi connect timeout as a builtin + # TimeoutError; the connect path must wrap it in BluetoothTransportError. + parent = MagicMock() + parent.private_key = ec.generate_private_key(ec.SECP256R1()) + vehicle = VehicleBluetooth(parent, VIN) + vehicle.device = MagicMock() + underlying = TimeoutError("connect timed out") + + with patch( + "tesla_fleet_api.tesla.vehicle.bluetooth.establish_connection", + AsyncMock(side_effect=underlying), + ): + with self.assertRaises(BluetoothTransportError) as ctx: + await vehicle.connect() + + self.assertIs(ctx.exception.__cause__, underlying) + + async def test_start_notify_timeout_raises_bluetooth_transport_error( + self, + ) -> None: + # start_notify is likewise decorated by bleak-esphome and can raise a + # builtin TimeoutError after the link is established. + parent = MagicMock() + parent.private_key = ec.generate_private_key(ec.SECP256R1()) + vehicle = VehicleBluetooth(parent, VIN) + vehicle.device = MagicMock() + underlying = TimeoutError("start_notify timed out") + client = MagicMock() + client.start_notify = AsyncMock(side_effect=underlying) + client.disconnect = AsyncMock() + + with patch( + "tesla_fleet_api.tesla.vehicle.bluetooth.establish_connection", + AsyncMock(return_value=client), + ): + with self.assertRaises(BluetoothTransportError) as ctx: + await vehicle.connect() + + self.assertIs(ctx.exception.__cause__, underlying) + client.disconnect.assert_awaited_once() + self.assertIsNone(vehicle.client)