From b5669eb6641a7197fbe501ef91c156b8fa64553a Mon Sep 17 00:00:00 2001 From: Nathan Bryant <60950167+nbry@users.noreply.github.com> Date: Wed, 8 May 2024 09:58:40 -0700 Subject: [PATCH] fix: Retry-After in batch path response (#136) --- smartcar/exception.py | 7 +++-- smartcar/vehicle.py | 55 ++++++++++++++++++++----------------- tests/e2e/test_exception.py | 1 + tests/unit/test_vehicle.py | 48 ++++++++++++++++++++++++++++++++ 4 files changed, 83 insertions(+), 28 deletions(-) diff --git a/smartcar/exception.py b/smartcar/exception.py index 6afad842..d7dfe535 100644 --- a/smartcar/exception.py +++ b/smartcar/exception.py @@ -39,9 +39,11 @@ def __init__(self, **kwargs): super().__init__(self.message) -def exception_factory(status_code: int, headers: dict, body: str): +def exception_factory( + status_code: int, headers: dict, body: str, check_content_type=True +): # v1.0 Exception: Content type other than application/json - if "application/json" not in headers["Content-Type"]: + if check_content_type and "application/json" not in headers["Content-Type"]: return SmartcarException(status_code=status_code, message=body) # Parse body into JSON. Throw SDK error if this fails. @@ -103,7 +105,6 @@ def exception_factory(status_code: int, headers: dict, body: str): suggested_user_message=response.get("suggestedUserMessage"), ) - # Weird... else: return SmartcarException( status_code=status_code, diff --git a/smartcar/vehicle.py b/smartcar/vehicle.py index 392b5ec0..3e9cfa7a 100644 --- a/smartcar/vehicle.py +++ b/smartcar/vehicle.py @@ -1,6 +1,6 @@ from collections import namedtuple import json -from typing import List +from typing import Callable, List import smartcar.config as config import smartcar.helpers as helpers import smartcar.smartcar @@ -397,6 +397,30 @@ def send_destination(self, latitude, longitude) -> types.Action: ) return types.select_named_tuple("send_destination", response) + @staticmethod + def _batch_path_response( + path: str, path_response: dict, top_response: dict + ) -> Callable[[], namedtuple]: + if path_response.get("code") == 200: + # attach top-level sc-request-id to res_dict + path_response["headers"]["sc-request-id"] = top_response.headers.get( + "sc-request-id" + ) + # use lambda default args to avoid issues with closures + return lambda p=path, r=path_response: types.select_named_tuple(p, r) + + # if individual response is erroneous, attach a lambda that returns a SmartcarException + def _attribute_raise_exception(smartcar_exception): + raise smartcar_exception + + path_status_code = path_response.get("code") + path_headers = path_response.get("headers", {}) + path_body = json.dumps(path_response.get("body")) + sc_exception = sce.exception_factory( + path_status_code, path_headers, path_body, False + ) + return lambda e=sc_exception: _attribute_raise_exception(e) + def batch(self, paths: List[str]) -> namedtuple: """ POST Vehicle.batch @@ -432,32 +456,13 @@ def batch(self, paths: List[str]) -> namedtuple: # success of the request. batch_dict = dict() path_responses = response.json()["responses"] - for res_dict in path_responses: + for path_response in path_responses: path, attribute = helpers.format_path_and_attribute_for_batch( - res_dict["path"] + path_response["path"] + ) + batch_dict[attribute] = Vehicle._batch_path_response( + path, path_response, response ) - - if res_dict.get("code") == 200: - # attach top-level sc-request-id to res_dict - res_dict["headers"]["sc-request-id"] = response.headers.get( - "sc-request-id" - ) - # use lambda default args to avoid issues with closures - batch_dict[attribute] = ( - lambda p=path, r=res_dict: types.select_named_tuple(p, r) - ) - else: - # if individual response is erroneous, attach a lambda that returns a SmartcarException - def _attribute_raise_exception(smartcar_exception): - raise smartcar_exception - - code = res_dict.get("code") - headers = response.headers - body = json.dumps(res_dict.get("body")) - sc_exception = sce.exception_factory(code, headers, body) - batch_dict[attribute] = ( - lambda e=sc_exception: _attribute_raise_exception(e) - ) # STEP 3 - Attach Meta to batch_dict batch_dict["meta"] = types.build_meta(response.headers) diff --git a/tests/e2e/test_exception.py b/tests/e2e/test_exception.py index 9de5c180..1e0f2ae4 100644 --- a/tests/e2e/test_exception.py +++ b/tests/e2e/test_exception.py @@ -1,3 +1,4 @@ +import json import smartcar import smartcar.smartcar from smartcar.smartcar import get_user diff --git a/tests/unit/test_vehicle.py b/tests/unit/test_vehicle.py index e3cc9a99..4839a530 100644 --- a/tests/unit/test_vehicle.py +++ b/tests/unit/test_vehicle.py @@ -1,4 +1,5 @@ from smartcar import Vehicle +from smartcar.exception import SmartcarException def test_vehicle_constructor(): @@ -57,3 +58,50 @@ def test_vehicle_constructor_options(): test_url = vehicle._format_url(path) expected_url = f"https://api.smartcar.com/v{version}/vehicles/{vid}/{path}" assert test_url == expected_url + + +def test_batch_path_response(): + path = "battery_capacity" + path_response = { + "code": 429, + "path": "/battery/capacity", + "body": { + "statusCode": 429, + "type": "RATE_LIMIT", + "code": "VEHICLE", + "description": "You have reached the throttling rate limit for this vehicle. Please see the retry-after header for when to retry the request.", + "docURL": "https://smartcar.com/docs/errors/api-errors/rate-limit-errors#vehicle", + "resolution": {"type": "RETRY_LATER"}, + "suggestedUserMessage": "Your vehicle is temporarily unable to connect to Optiwatt. Please be patient while we’re working to resolve this issue.", + "requestId": "test-request-id", + }, + "headers": {"Retry-After": 999}, + } + + top_response = { + "responses": [path_response], + "headers": {"Content-Type": "application/json"}, + } + resulting_lambda = Vehicle._batch_path_response(path, path_response, top_response) + + try: + resulting_lambda() + except Exception as e: + path_exception = e + + assert isinstance(path_exception, SmartcarException) + assert path_exception.status_code == 429 + assert path_exception.request_id == "test-request-id" + assert path_exception.type == "RATE_LIMIT" + assert ( + path_exception.description + == "You have reached the throttling rate limit for this vehicle. Please see the retry-after header for when to retry the request." + ) + assert path_exception.code == "VEHICLE" + assert path_exception.resolution == {"type": "RETRY_LATER", "url": None} + assert path_exception.detail == None + assert ( + path_exception.suggested_user_message + == "Your vehicle is temporarily unable to connect to Optiwatt. Please be patient while we’re working to resolve this issue." + ) + assert path_exception.retry_after == 999