From af11928e86b6e3d8bab1f74c57d4d2219c9a9f51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erkki=20Sepp=C3=A4l=C3=A4?= Date: Wed, 8 Mar 2023 19:04:31 +0200 Subject: [PATCH 1/7] teslabot: Handle ProtocolError and ConnectionError when sending requests Addresses #16. --- .github/workflows/quality.yml | 3 +++ requirements-types.txt | 1 + requirements.txt | 1 + teslabot/tesla.py | 8 ++++++++ 4 files changed, 13 insertions(+) create mode 100644 requirements-types.txt diff --git a/.github/workflows/quality.yml b/.github/workflows/quality.yml index 7acd1ef..387adfc 100644 --- a/.github/workflows/quality.yml +++ b/.github/workflows/quality.yml @@ -39,6 +39,9 @@ jobs: python -m pip install --upgrade pip python -m pip install mypy wheel --requirement requirements.txt --requirement requirements-slack.txt --requirement requirements-matrix.txt + - name: Install dependencies for mypy checking + run: pip3 install -r requirements-types.txt + - name: Run mypy run: mypy -p teslabot -p tests diff --git a/requirements-types.txt b/requirements-types.txt new file mode 100644 index 0000000..3d3252b --- /dev/null +++ b/requirements-types.txt @@ -0,0 +1 @@ +types-requests==2.28.11.15 diff --git a/requirements.txt b/requirements.txt index f758bf7..d6baccd 100644 --- a/requirements.txt +++ b/requirements.txt @@ -6,3 +6,4 @@ typing-extensions==4.1.1 google-cloud-secret-manager==2.10.0 google-cloud-firestore==2.4.0 setuptools==65.5.1 +urllib3==1.26.14 diff --git a/teslabot/tesla.py b/teslabot/tesla.py index 58b3f00..2d6efc5 100644 --- a/teslabot/tesla.py +++ b/teslabot/tesla.py @@ -13,6 +13,8 @@ import teslapy from urllib.error import HTTPError +from urllib3.exceptions import ProtocolError +from requests.exceptions import ConnectionError from .control import Control, ControlCallback, CommandContext, MessageContext from .commands import Invocation @@ -687,6 +689,12 @@ def call() -> Any: except HTTPError as exn: logger.debug(f"HTTP error: {exn}") error = exn + except ProtocolError as exn: + logger.debug(f"HTTP protocol error: {exn}") + error = exn + except ConnectionError as exn: + logger.debug(f"HTTP connection error: {exn}") + error = exn finally: logger.debug(f"Done sending") await asyncio.sleep(pow(1.15, num_retries) * 2) From c188206bbc7876acb6ae1b4404daf1524eb8af01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erkki=20Sepp=C3=A4l=C3=A4?= Date: Wed, 8 Mar 2023 19:12:16 +0200 Subject: [PATCH 2/7] appscheduler: catch and report leaking exceptions to user Closes #16. --- teslabot/appscheduler.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/teslabot/appscheduler.py b/teslabot/appscheduler.py index 92cca9f..d45c57b 100644 --- a/teslabot/appscheduler.py +++ b/teslabot/appscheduler.py @@ -3,6 +3,7 @@ import time from typing import List, Tuple, Optional, Any, Callable, Awaitable, TypeVar, Generic from dataclasses import dataclass +import traceback from . import scheduler from . import log @@ -300,5 +301,10 @@ async def _activate_timer(self, entry: scheduler.Entry[SchedulerContext]) -> Non await self.control.send_message(context.to_message_context(), f"Timer activated: \"{' '.join(command)}\"") assert self._commands invocation = c.Invocation(name=command[0], args=command[1:]) - await self._commands.invoke(context, invocation) + try: + await self._commands.invoke(context, invocation) + except Exception as exn: + logger.error(f"{context.txn} {exn} {traceback.format_exc()}") + await self.control.send_message(context.to_message_context(), + f"{context.txn} Exception :(") await self._command_ls(context, ()) From de257420b206a113e737c7cbf4ef5c844f7f5c75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erkki=20Sepp=C3=A4l=C3=A4?= Date: Wed, 8 Mar 2023 20:21:19 +0200 Subject: [PATCH 3/7] tesla._retry: generalized retry mechanism --- teslabot/tesla.py | 39 +++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/teslabot/tesla.py b/teslabot/tesla.py index 2d6efc5..5bcf298 100644 --- a/teslabot/tesla.py +++ b/teslabot/tesla.py @@ -663,22 +663,13 @@ def call(vehicle: teslapy.Vehicle) -> Any: return vehicle.command(command, **kwargs) await self._command_on_vehicle(context, vehicle_name, call) - async def _command_on_vehicle(self, - context: CommandContext, - vehicle_name: Optional[str], - fn: Callable[[teslapy.Vehicle], T], - show_success: bool = True) -> Optional[T]: - vehicle = await self._get_vehicle(vehicle_name) - await self._wake(context, vehicle) + async def _retry(self, + fn: Callable[[], Awaitable[T]]) -> Union[T, Exception]: + result_or_error: Optional[Union[T, Exception]] = None num_retries = 0 - error = None - result: Optional[T] = None while num_retries < 5: try: - # https://github.com/python/mypy/issues/9590 - def call() -> Any: - return fn(vehicle) - result = await to_async(call) + result = await fn() error = None break except teslapy.VehicleError as exn: @@ -696,9 +687,29 @@ def call() -> Any: logger.debug(f"HTTP connection error: {exn}") error = exn finally: - logger.debug(f"Done sending") + logger.debug(f"Retry round complete") await asyncio.sleep(pow(1.15, num_retries) * 2) num_retries += 1 + assert result_or_error is not None + return result_or_error + + async def _command_on_vehicle(self, + context: CommandContext, + vehicle_name: Optional[str], + fn: Callable[[teslapy.Vehicle], T], + show_success: bool = True) -> Optional[T]: + vehicle = await self._get_vehicle(vehicle_name) + await self._wake(context, vehicle) + error = None + result: Optional[T] = None + # https://github.com/python/mypy/issues/9590 + def call() -> Any: + return fn(vehicle) + result_or_error = await self._retry(call) + if isinstance(result_or_error, Exception): + error = result_or_error + else: + result = result_or_error if error: await self.control.send_message(context.to_message_context(), f"Error: {error}") return None From 28d2a1400576156728a9d37a1ec623388ec2e964 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erkki=20Sepp=C3=A4l=C3=A4?= Date: Wed, 8 Mar 2023 20:43:25 +0200 Subject: [PATCH 4/7] tesla: retry all tesla interface ops, not just the actual command This includes getting vehicle list. Due to the parser needing access to one we need to cache it, because parser isn't async, while the code to retrieve & check it is. It's probably alright.. --- teslabot/tesla.py | 110 +++++++++++++++++++++++++++++----------------- 1 file changed, 69 insertions(+), 41 deletions(-) diff --git a/teslabot/tesla.py b/teslabot/tesla.py index 5bcf298..c1919be 100644 --- a/teslabot/tesla.py +++ b/teslabot/tesla.py @@ -49,15 +49,16 @@ class VehicleException(AppException): VehicleName = NewType('VehicleName', str) class ValidVehicle(p.Map[str, VehicleName]): - tesla: teslapy.Tesla + app: "App" - def __init__(self, tesla: teslapy.Tesla) -> None: + def __init__(self, app: "App") -> None: super().__init__(map=lambda x: VehicleName(x), parser=p.Delayed[str](self.make_validator)) - self.tesla = tesla + self.app = app def make_validator(self) -> p.Parser[str]: - vehicles = self.tesla.vehicle_list() + # TODO: Cannot do async stuff here, so the cached version must do + vehicles = self.app.cached_vehicle_list display_names = [vehicle["display_name"] for vehicle in vehicles] return p.OneOfStrings(display_names) @@ -143,17 +144,17 @@ async def save(self, state: State) -> None: ClimateArgs = Tuple[Tuple[bool, Optional[VehicleName]], Tuple[()]] def valid_on_off_vehicle(app: "App") -> p.Parser[ClimateArgs]: - return p.Adjacent(p.Adjacent(p.Bool(), p.ValidOrMissing(ValidVehicle(app.tesla))), + return p.Adjacent(p.Adjacent(p.Bool(), p.ValidOrMissing(ValidVehicle(app))), p.Empty()) InfoArgs = Tuple[Optional[VehicleName], Tuple[()]] def valid_info(app: "App") -> p.Parser[InfoArgs]: - return p.Adjacent(p.ValidOrMissing(ValidVehicle(app.tesla)), + return p.Adjacent(p.ValidOrMissing(ValidVehicle(app)), p.Empty()) LockUnlockArgs = Tuple[Optional[VehicleName], Tuple[()]] def valid_lock_unlock(app: "App") -> p.Parser[LockUnlockArgs]: - return p.Adjacent(p.ValidOrMissing(ValidVehicle(app.tesla)), + return p.Adjacent(p.ValidOrMissing(ValidVehicle(app)), p.Empty()) ChargeArgs = Tuple[Tuple[ChargeOp, Optional[VehicleName]], Tuple[()]] @@ -178,7 +179,7 @@ def valid_charge(app: "App") -> p.Parser[ChargeArgs]: p.Map(parser=p.Keyword("schedule", p.HhMm()), map=lambda x: ChargeOpSchedulingEnable(x[0] * 60 + x[1])), ), - p.ValidOrMissing(ValidVehicle(app.tesla))), + p.ValidOrMissing(ValidVehicle(app))), p.Empty() ) @@ -235,14 +236,14 @@ def valid_heater(app: "App") -> p.Parser[HeaterArgs]: map=lambda _: HeaterSteering())), p.OneOfEnumValue(HeaterLevel) ), - p.ValidOrMissing(ValidVehicle(app.tesla)) + p.ValidOrMissing(ValidVehicle(app)) ), p.Empty()) ShareArgs = Tuple[Tuple[str, Optional[VehicleName]], Tuple[()]] def valid_share(app: "App") -> p.Parser[ShareArgs]: return p.Adjacent(p.Adjacent(p.Concat(), - p.ValidOrMissing(ValidVehicle(app.tesla))), + p.ValidOrMissing(ValidVehicle(app))), p.Empty()) def cmd_adjacent(label: str, parser: p.Parser[T]) -> p.Parser[Tuple[str, T]]: @@ -284,6 +285,7 @@ class App(ControlCallback): _scheduler: AppScheduler[None] locations: Locations location_detail: LocationDetail + cached_vehicle_list: List[Any] def __init__(self, control: Control, @@ -293,6 +295,7 @@ def __init__(self, self.state = env.state self.locations = Locations(self.state) self.location_detail = LocationDetail.Full + self.cached_vehicle_list = [] control.callback = self cache_loader: Union[Callable[[], Dict[str, Any]], None] = None cache_dumper: Union[Callable[[Dict[str, Any]], None], None] = None @@ -371,9 +374,10 @@ async def _command_logout(self, context: CommandContext, args: Tuple[()]) -> Non elif not context.admin_room: await self.control.send_message(context.to_message_context(), "Please use the admin room for this command.") else: + # https://github.com/python/mypy/issues/9590 def call() -> None: self.tesla.logout() - await to_async(call) + await self._retry_to_async(call) await self.control.send_message(context.to_message_context(), "Logout successful!") async def command_callback(self, @@ -456,20 +460,27 @@ async def _command_authorized(self, context: CommandContext, authorization_respo # https://github.com/python/mypy/issues/9590 def call() -> None: self.tesla.fetch_token(authorization_response=authorization_response) - await to_async(call) + await self._retry_to_async(call) await self.control.send_message(context.to_message_context(), "Authorization successful") - vehicles = self.tesla.vehicle_list() - await self.control.send_message(context.to_message_context(), str(vehicles[0])) elif not self.tesla.authorized: await self.control.send_message(context.to_message_context(), f"Not authorized. Authorization URL: {self.tesla.authorization_url()} \"Page Not Found\" will be shown at success. Use !authorize https://the/url/you/ended/up/at") + async def _get_vehicle_list(self) -> List[Any]: + def call() -> List[Any]: + self.cached_vehicle_list = self.tesla.vehicle_list() + return self.cached_vehicle_list + result_or_error = await self._retry_to_async(call) + if isinstance(result_or_error, Exception): + raise result_or_error + assert result_or_error is not None + return result_or_error async def _command_vehicles(self, context: CommandContext, valid: Tuple[()]) -> None: - vehicles = self.tesla.vehicle_list() + vehicles = self._get_vehicle_list() await self.control.send_message(context.to_message_context(), f"vehicles: {vehicles}") async def _get_vehicle(self, display_name: Optional[str]) -> teslapy.Vehicle: - vehicles = self.tesla.vehicle_list() + vehicles = await self._get_vehicle_list() if display_name is not None: vehicles = [vehicle for vehicle in vehicles if vehicle["display_name"].lower() == display_name.lower()] if len(vehicles) > 1: @@ -549,6 +560,9 @@ def call(vehicle: teslapy.Vehicle) -> Any: data = await self._command_on_vehicle(context, vehicle_name, call, show_success=False) if not data: return + # refresh cache for parsers etc + await self._get_vehicle_list() + logger.debug(f"data: {data}") dist_hr_unit = data["gui_settings"]["gui_distance_units"] dist_unit = assert_some(re.match(r"^[^/]*", dist_hr_unit), "Expected to find / from dist_hr_unit")[0] @@ -664,12 +678,15 @@ def call(vehicle: teslapy.Vehicle) -> Any: await self._command_on_vehicle(context, vehicle_name, call) async def _retry(self, - fn: Callable[[], Awaitable[T]]) -> Union[T, Exception]: - result_or_error: Optional[Union[T, Exception]] = None + fn: Callable[[], Awaitable[T]]) -> T: num_retries = 0 + result_is_set = False + result: T + error = None while num_retries < 5: try: result = await fn() + result_is_set = True error = None break except teslapy.VehicleError as exn: @@ -690,37 +707,45 @@ async def _retry(self, logger.debug(f"Retry round complete") await asyncio.sleep(pow(1.15, num_retries) * 2) num_retries += 1 - assert result_or_error is not None - return result_or_error + if error is not None: + raise error + assert result_is_set + return result + + async def _retry_to_async(self, fn: Callable[[], T]) -> T: + async def call() -> T: + def call2() -> T: + return fn() + return await to_async(call2) + return await self._retry(call) async def _command_on_vehicle(self, context: CommandContext, vehicle_name: Optional[str], fn: Callable[[teslapy.Vehicle], T], show_success: bool = True) -> Optional[T]: - vehicle = await self._get_vehicle(vehicle_name) - await self._wake(context, vehicle) - error = None result: Optional[T] = None - # https://github.com/python/mypy/issues/9590 - def call() -> Any: - return fn(vehicle) - result_or_error = await self._retry(call) - if isinstance(result_or_error, Exception): - error = result_or_error - else: - result = result_or_error - if error: - await self.control.send_message(context.to_message_context(), f"Error: {error}") + try: + vehicle = await self._get_vehicle(vehicle_name) + await self._retry(lambda: self._wake(context, vehicle)) + # https://github.com/python/mypy/issues/9590 + def call() -> T: + return fn(vehicle) + result = await self._retry_to_async(call) + except Exception as exn: + if isinstance(exn, teslapy.VehicleError): + await self.control.send_message(context.to_message_context(), f"Error: {exn}") + else: + logger.error(f"{context.txn} {exn} {traceback.format_exc()}") + await self.control.send_message(context.to_message_context(), + f"{context.txn} Exception :(") return None - else: - assert result is not None - if show_success: - message = "Success!" - if result != True: # this never happens, though? - message += f" {result}" - await self.control.send_message(context.to_message_context(), message) - return result + if show_success: + message = "Success!" + if result != True: # this never happens, though? + message += f" {result}" + await self.control.send_message(context.to_message_context(), message) + return result async def _command_climate(self, context: CommandContext, args: ClimateArgs) -> None: (mode, vehicle_name), _ = args @@ -746,3 +771,6 @@ async def run(self) -> None: if not self.tesla.authorized: await self.control.send_message(MessageContext(admin_room=True), f"Not authorized. Authorization URL: {self.tesla.authorization_url()} \"Page Not Found\" will be shown at success. Use !authorize https://the/url/you/ended/up/at") + else: + # ensure the vehicle list is cached at least once + await self._get_vehicle_list() From b10c67461465529e5e084861ba4ba6e491c26c05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erkki=20Sepp=C3=A4l=C3=A4?= Date: Wed, 8 Mar 2023 21:26:37 +0200 Subject: [PATCH 5/7] tesla: increased the number of retries 5->10 Bonus: log the number of retries Fixes #10. --- teslabot/tesla.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/teslabot/tesla.py b/teslabot/tesla.py index c1919be..7318a7e 100644 --- a/teslabot/tesla.py +++ b/teslabot/tesla.py @@ -683,7 +683,7 @@ async def _retry(self, result_is_set = False result: T error = None - while num_retries < 5: + while num_retries < 10: try: result = await fn() result_is_set = True @@ -707,6 +707,8 @@ async def _retry(self, logger.debug(f"Retry round complete") await asyncio.sleep(pow(1.15, num_retries) * 2) num_retries += 1 + if num_retries > 0: + logger.debug(f"Number of retries: {num_retries}") if error is not None: raise error assert result_is_set From b7a3854a02bcd32045a737822406260ddd8c84d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erkki=20Sepp=C3=A4l=C3=A4?= Date: Wed, 8 Mar 2023 21:30:31 +0200 Subject: [PATCH 6/7] commands: remove empty fields from parse for user convenience Closes #15. --- teslabot/commands.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/teslabot/commands.py b/teslabot/commands.py index f94faad..31b8814 100644 --- a/teslabot/commands.py +++ b/teslabot/commands.py @@ -45,7 +45,7 @@ class Invocation: @staticmethod def parse(message: str) -> "Invocation": - fields = re.split(r" *", message) + fields = [field for field in re.split(r" *", message) if field != ""] if len(fields): logger.debug(f"Command: {fields}") return Invocation(name=fields[0], From 01145ab926a0bbd099bbf79f39106a1d442f9d5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erkki=20Sepp=C3=A4l=C3=A4?= Date: Wed, 8 Mar 2023 21:36:43 +0200 Subject: [PATCH 7/7] commands: remove content includign and after # to support commments Closes #17. --- README.md | 4 +++- teslabot/commands.py | 8 +++++--- teslabot/control.py | 2 ++ 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 7c5e4dc..5791373 100644 --- a/README.md +++ b/README.md @@ -136,7 +136,7 @@ You can use e.g. `screen`, `tmux` or `systemd` to arrange this process to run on Note that by default you need to prefix commands with ```!```. | command | description | -| --- | --- | +|---------------------------------------------|------------------------------------------------------------------------------------------------------------------------------------------------| | help | Show the list of commands supported. | | authorize | Start the authorization flow. Works only in admin room (though you could only have one and same for control and admin). | | authorize url | Last phase of the authorization flow. | @@ -168,3 +168,5 @@ Note that by default you need to prefix commands with ```!```. | charge schedule disable | Disable charging schedule. | | heater seat n off/low/medium/high | Adjust seat heaters. Works only if AC is on. | | heater steering off/high | Adjust steering wheel heater. Works only if AC is on. | +| command # comment | Run command; ignore # comment | +| # comment | Ignore message | diff --git a/teslabot/commands.py b/teslabot/commands.py index 31b8814..6da0303 100644 --- a/teslabot/commands.py +++ b/teslabot/commands.py @@ -4,6 +4,7 @@ from abc import ABC, abstractmethod from typing import List, Callable, Coroutine, Any, TypeVar, Generic, Optional, Tuple, Mapping, Union, Awaitable from .parser import Parser, ParseResult, ParseOK, ParseFail +from .utils import assert_some from . import log @@ -20,7 +21,7 @@ class CommandsException(Exception): class ParseError(CommandsException): pass -class InvocationParseError(ParseError): +class InvocationEmptyError(ParseError): pass @dataclass @@ -45,13 +46,14 @@ class Invocation: @staticmethod def parse(message: str) -> "Invocation": - fields = [field for field in re.split(r" *", message) if field != ""] + command = assert_some(re.match(r"^[^#]*", message), "This should always match")[0] # extract non-comment part + fields = [field for field in re.split(r" *", command) if field != ""] if len(fields): logger.debug(f"Command: {fields}") return Invocation(name=fields[0], args=fields[1:]) else: - raise InvocationParseError() + raise InvocationEmptyError() class Command(ABC, Generic[Context]): name: str diff --git a/teslabot/control.py b/teslabot/control.py index b4d54dd..a0d5347 100644 --- a/teslabot/control.py +++ b/teslabot/control.py @@ -84,6 +84,8 @@ async def process_message(self, command_context: CommandContext, message: str) - await self.local_commands.invoke(command_context, invocation) else: await self.callback.command_callback(command_context, invocation) + except commands.InvocationEmptyError as exn: + logger.debug("Ignoring empty message (or completely commented)") except commands.CommandParseError as exn: logger.error(f"{command_context.txn}: Failed to parse command: {message}") def format(word: str, highlight: bool) -> str: