diff --git a/README.md b/README.md index ee25608..c854bce 100644 --- a/README.md +++ b/README.md @@ -23,9 +23,33 @@ transcribed from the portal and verified endpoint by endpoint against the live A + **[docs/RETURN_CODES.md](docs/RETURN_CODES.md)** — the complete return code table (both pages of the portal's paginated list), grouped by cause, plus codes the portal does not publish. -Four things the official documentation gets wrong are corrected in +Things the official documentation gets wrong are corrected in [docs/API.md](docs/API.md#corrections-to-the-official-documentation). +## Error handling + +By default, if the API answers with a non-`200` `code` the method logs it and returns `None`. +Transport failures — connection resets, timeouts, non-2xx HTTP — raise instead. + +The catch is that this makes a successful write look identical to a rejected one, since the write +endpoints answer with `data: null` either way. If you need to tell them apart, build the client +with `raise_on_error=True`: + +```python +from alphaess.alphaess import alphaess, AlphaESSApiError + +client = alphaess(appID, appSecret, raise_on_error=True) + +try: + await client.setTimeChargeBySn(sysSn, 0, charge_list, discharge_list) +except AlphaESSApiError as err: + print(err.code, err.expMsg) # e.g. 6001 "time list is null" +``` + +Success then just means nothing was raised — return values don't change. It's off by default, so +upgrading won't alter how your existing code behaves. Full reference in +[docs/RETURN_CODES.md](docs/RETURN_CODES.md#opting-in-to-exceptions--raise_on_error-0021). + # Methods There are public methods in this module that duplicate the AlphaESS OpenAPI and provide wrappers for diff --git a/alphaess/alphaess.py b/alphaess/alphaess.py index ec07bff..6ee9917 100644 --- a/alphaess/alphaess.py +++ b/alphaess/alphaess.py @@ -37,9 +37,39 @@ # RETURN_CODES stays a faithful copy of the documented table. UNDOCUMENTED_RETURN_CODES = { 6017: "No operation permissions", + 10001: "Parameter error (malformed request body, e.g. a required list omitted)", } +class AlphaESSApiError(Exception): + """The API answered, and said no. + + Only raised if you built the client with ``raise_on_error=True``. Otherwise + an API-level failure still returns ``None``, same as it always has. + + Not the same thing as the aiohttp transport errors: we reached the service + and it rejected the request, so sending it again unchanged won't help. + """ + + def __init__(self, code, msg=None, expMsg=None, path=None, description=None): + self.code = code + self.msg = msg + self.expMsg = expMsg + self.path = path + self.description = description + + detail = f"{code}" + if description: + detail += f" ({description})" + if msg: + detail += f": {msg}" + if expMsg: + detail += f" - {expMsg}" + if path: + detail += f" when calling {path}" + super().__init__(detail) + + class alphaess: """Class for Alpha ESS.""" @@ -50,9 +80,17 @@ def __init__( session: aiohttp.ClientSession | None = None, timeout: int = 30, ipaddress=None, - verify_ssl=True + verify_ssl=True, + raise_on_error: bool = False ) -> None: - """Initialize.""" + """Initialize. + + raise_on_error goes last in the signature so existing positional callers + keep working. Leave it False and API-level failures return None like they + always have. Turn it on and they raise AlphaESSApiError instead, which is + the only way to tell a successful write from a rejected one — the write + endpoints answer with ``data: null`` whichever way it went. + """ self.appID = appID self.appSecret = appSecret self.accesstoken = None @@ -64,6 +102,7 @@ def __init__( self.timeout = timeout self.ipaddress = ipaddress self.verify_ssl = verify_ssl + self.raise_on_error = raise_on_error async def close(self) -> None: """Close the AlphaESS API client.""" @@ -104,6 +143,28 @@ def __return_code_description(json_response) -> str: description = RETURN_CODES.get(code) or UNDOCUMENTED_RETURN_CODES.get(code) return f" ({description})" if description else "" + def __handle_failure(self, json_response, path) -> None: + """Log an API-level failure, and raise it if raise_on_error is set. + + Returns normally otherwise, so the caller goes on to return None and + nothing changes for anyone who hasn't opted in. + """ + expMsg = json_response.get("expMsg") + logger.error( + f"Unexpected json_response : {json_response}" + f"{self.__return_code_description(json_response)}" + f"{f' - {expMsg}' if expMsg else ''} when calling {path}") + + if self.raise_on_error: + code = json_response.get("code") + raise AlphaESSApiError( + code=code, + msg=json_response.get("msg") or json_response.get("info"), + expMsg=expMsg, + path=path, + description=RETURN_CODES.get(code) or UNDOCUMENTED_RETURN_CODES.get(code), + ) + async def getESSList(self) -> Optional(list): """According to SN to get system list data""" try: @@ -113,6 +174,8 @@ async def getESSList(self) -> Optional(list): return await self.api_get(resource) + except AlphaESSApiError: + raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") raise @@ -126,6 +189,8 @@ async def getLastPowerData(self, sysSn) -> Optional(list): return await self.api_get(resource) + except AlphaESSApiError: + raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") raise @@ -141,6 +206,8 @@ async def getOneDayPowerBySn(self, sysSn, queryDate=None) -> Optional(list): return await self.api_get(resource) + except AlphaESSApiError: + raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") raise @@ -154,6 +221,8 @@ async def getSumDataForCustomer(self, sysSn) -> Optional(list): return await self.api_get(resource) + except AlphaESSApiError: + raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") raise @@ -169,6 +238,8 @@ async def getOneDateEnergyBySn(self, sysSn, queryDate=None) -> Optional(list): return await self.api_get(resource) + except AlphaESSApiError: + raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") raise @@ -182,6 +253,8 @@ async def getChargeConfigInfo(self, sysSn) -> Optional(list): return await self.api_get(resource) + except AlphaESSApiError: + raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") raise @@ -195,6 +268,8 @@ async def getDisChargeConfigInfo(self, sysSn) -> Optional(list): return await self.api_get(resource) + except AlphaESSApiError: + raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") raise @@ -208,6 +283,8 @@ async def getEvChargerConfigList(self, sysSn) -> Optional(list): return await self.api_get(resource) + except AlphaESSApiError: + raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") raise @@ -226,6 +303,8 @@ async def setEvChargerCurrentsBySn(self, sysSn, currentsetting) -> Optional(list return await self.api_post(resource, settings) + except AlphaESSApiError: + raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") raise @@ -239,6 +318,8 @@ async def getEvChargerCurrentsBySn(self, sysSn) -> Optional(list): return await self.api_get(resource) + except AlphaESSApiError: + raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") raise @@ -252,6 +333,8 @@ async def getEvChargerStatusBySn(self, sysSn, evchargerSn) -> Optional(list): return await self.api_get(resource) + except AlphaESSApiError: + raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") raise @@ -271,6 +354,8 @@ async def remoteControlEvCharger(self, sysSn, evchargerSn, controlMode) -> Optio return await self.api_post(resource, settings) + except AlphaESSApiError: + raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") raise @@ -289,6 +374,8 @@ async def bindSn(self, sysSn, code) -> Optional(dict): return await self.api_post(resource, settings) + except AlphaESSApiError: + raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") raise @@ -302,6 +389,8 @@ async def getVerificationCode(self, sysSn, checkCode) -> Optional(dict): return await self.api_get(resource) + except AlphaESSApiError: + raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") raise @@ -319,6 +408,8 @@ async def unBindSn(self, sysSn) -> Optional(dict): return await self.api_post(resource, settings) + except AlphaESSApiError: + raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") raise @@ -343,6 +434,8 @@ async def updateChargeConfigInfo(self, sysSn, batHighCap, gridCharge, timeChae1, return await self.api_post(resource, settings) + except AlphaESSApiError: + raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") raise @@ -367,6 +460,8 @@ async def updateDisChargeConfigInfo(self, sysSn, batUseCap, ctrDis, timeDise1, t return await self.api_post(resource, settings) + except AlphaESSApiError: + raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") raise @@ -380,6 +475,8 @@ async def getTimeChargeBySn(self, sysSn) -> Optional(dict): return await self.api_get(resource) + except AlphaESSApiError: + raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") raise @@ -415,6 +512,8 @@ async def setTimeChargeBySn(self, sysSn, executeCycleType, chargeTimeList, disch return await self.api_post(resource, settings) + except AlphaESSApiError: + raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") raise @@ -465,17 +564,19 @@ async def api_get(self, path, json=None) -> Optional(list): logger.error(f"Unexpected response received: {response.status} when calling {path}") if not self.__is_success(json_response): - logger.error( - f"Unexpected json_response : {json_response}" - f"{self.__return_code_description(json_response)} when calling {path}") + self.__handle_failure(json_response, path) return None else: if json_response["data"] is not None: return json_response["data"] else: - logger.error(f"Unexpected json_response : {json_response} when calling {path}") + # Succeeded, just had nothing to give us. Not an error. + logger.debug( + f"Successful but empty json_response : {json_response} when calling {path}") return None + except AlphaESSApiError: + raise except Exception as e: logger.error(e) raise @@ -503,15 +604,30 @@ async def api_post(self, path, json) -> Optional(dict): if self.__is_success(json_response): return json_response["data"] - logger.error( - f"Unexpected json_response : {json_response}" - f"{self.__return_code_description(json_response)} when calling {path}") + self.__handle_failure(json_response, path) return None + except AlphaESSApiError: + raise except Exception as e: logger.error(e) raise + async def _best_effort(self, method, *args): + """Call an endpoint for getdata, tolerating a refusal. + + getdata is best-effort by contract: it returns whatever it could + gather. With raise_on_error set that would otherwise stop dead on the + first endpoint this account cannot use, losing the rest of the data + too, so an API-level refusal leaves that one key empty instead. + Transport errors still propagate -- those affect every endpoint. + """ + try: + return await method(*args) + except AlphaESSApiError as err: + logger.debug(f"{getattr(method, '__name__', method)} refused: {err}") + return None + async def getdata(self, get_power=False, get_ev=False, self_delay=0, get_timecharge=False) -> Optional(list): """Get All Data For All serial numbers from Alpha ESS""" try: @@ -530,7 +646,7 @@ async def getdata(self, get_power=False, get_ev=False, self_delay=0, get_timecha } } - units = await self.getESSList() + units = await self._best_effort(self.getESSList) if units is None: logger.warning( "getESSList returned no data (Alpha ESS API busy or unavailable); " @@ -540,32 +656,32 @@ async def getdata(self, get_power=False, get_ev=False, self_delay=0, get_timecha for idx, unit in enumerate(units): if "sysSn" in unit: serial = unit["sysSn"] - unit['SumData'] = await self.getSumDataForCustomer(serial) + unit['SumData'] = await self._best_effort(self.getSumDataForCustomer, serial) await asyncio.sleep(self_delay) - unit['OneDateEnergy'] = await self.getOneDateEnergyBySn(serial, time.strftime("%Y-%m-%d")) + unit['OneDateEnergy'] = await self._best_effort(self.getOneDateEnergyBySn, serial, time.strftime("%Y-%m-%d")) await asyncio.sleep(self_delay) - unit['LastPower'] = await self.getLastPowerData(serial) + unit['LastPower'] = await self._best_effort(self.getLastPowerData, serial) await asyncio.sleep(self_delay) - unit['ChargeConfig'] = await self.getChargeConfigInfo(serial) + unit['ChargeConfig'] = await self._best_effort(self.getChargeConfigInfo, serial) await asyncio.sleep(self_delay) - unit['DisChargeConfig'] = await self.getDisChargeConfigInfo(serial) + unit['DisChargeConfig'] = await self._best_effort(self.getDisChargeConfigInfo, serial) await asyncio.sleep(self_delay) if get_power: await asyncio.sleep(self_delay) - unit['OneDayPower'] = await self.getOneDayPowerBySn(serial, time.strftime("%Y-%m-%d")) + unit['OneDayPower'] = await self._best_effort(self.getOneDayPowerBySn, serial, time.strftime("%Y-%m-%d")) if get_timecharge: await asyncio.sleep(self_delay) - unit['TimeCharge'] = await self.getTimeChargeBySn(serial) + unit['TimeCharge'] = await self._best_effort(self.getTimeChargeBySn, serial) if get_ev: await asyncio.sleep(self_delay) - unit['EVData'] = await self.getEvChargerConfigList(serial) + unit['EVData'] = await self._best_effort(self.getEvChargerConfigList, serial) await asyncio.sleep(self_delay) try: ev_serial = unit['EVData'][0].get('evchargerSn', None) @@ -583,6 +699,8 @@ async def getdata(self, get_power=False, get_ev=False, self_delay=0, get_timecha return alldata + except AlphaESSApiError: + raise except Exception as e: logger.error(e) raise @@ -604,6 +722,8 @@ async def authenticate(self) -> Optional(list): success = True return success + except AlphaESSApiError: + raise except Exception as e: logger.error(e) raise @@ -626,6 +746,8 @@ async def setbatterycharge(self, serial, enabled, cp1start, cp1end, cp2start, cp logger.debug(f"Trying to call {resource} with settings {settings}") return await self.api_post(resource, settings) + except AlphaESSApiError: + raise except Exception as e: logger.error(e) raise @@ -648,6 +770,8 @@ async def setbatterydischarge(self, serial, enabled, dp1start, dp1end, dp2start, logger.debug(f"Trying to call {resource} with settings {settings}") return await self.api_post(resource, settings) + except AlphaESSApiError: + raise except Exception as e: logger.error(e) raise diff --git a/docs/API.md b/docs/API.md index c2340a6..cf66c89 100644 --- a/docs/API.md +++ b/docs/API.md @@ -146,13 +146,29 @@ Interfaces 1–17 are numbered as the portal numbers them (`interface_id` 1–17 charge/discharge pair carries portal ids `110000000000000` and `110000000000001`. **The method is enforced.** Calling an endpoint with the other verb returns a plain HTTP `405` -with no `code` field. +with no `code` field. This is also a reliable way to tell an endpoint that exists from one that +does not: an unknown path returns `404`, a real path called with the wrong verb returns `405`. + +### Two endpoints you can't reach + +There are 21 endpoints in the portal, not 19. The other two are for meter offset, and they only +show up in the commercial & industrial document — the portal splits its endpoints across three +documents (standard, third-party institutions, and C&I) and a normal developer account only gets +the standard one. + +| Portal id | Endpoint | Method | Parameters | +|:--|:--|:--|:--| +| 20 | `getMeterOffsetConfigInfo` | **GET** — the portal says POST, which just `405`s | `sysSn` | +| 21 | `updateMeterOffsetConfigInfo` | POST | `sysSn`, `pmOffset` (0.1 kW units, −500…500 kW, default 0), `pmOffsetEn` (1/0), `pmOffsetS1`/`E1`, `pmOffsetS2`/`E2` | + +They're here so nobody else has to go digging through the portal to work out what the missing two +are. The library doesn't implement them. --- ## Corrections to the official documentation -Four points where the portal (or the bundled Postman collection) is wrong, each confirmed +Points where the portal (or the bundled Postman collection) is wrong, each confirmed against the live API: | Source says | Actual behaviour | How it was confirmed | @@ -160,7 +176,16 @@ against the live API: | `getVerificationCode` takes a JSON body ("request parameter (Json)") | **GET only**, query-string parameters | POST returns HTTP `405 Method Not Allowed` | | `bindSn` is a GET with query parameters *(Postman collection)* | **POST only**, JSON body | GET returns HTTP `405 Method Not Allowed` | | `getOneDayPowerBySn` returns `cobat` and `pChargingPile` | returns **`cbat`** and **`pchargingPile`** | live response inspection | -| 19 return codes exist | at least one more — **`6017 No operation permissions`** | returned by `getTimeChargeBySn` | +| 19 return codes exist | at least two more — **`6017`** and **`10001`** | `6017` from `getTimeChargeBySn`, `10001` from `setTimeChargeBySn` with a list omitted | +| `getMeterOffsetConfigInfo` is a POST | **GET only** | POST returns `405`, GET returns a `code` body | +| *(this document, before 0.0.21)* an empty period list is acceptable — "send `[]`, not `null`" | **wrong** — `[]` is rejected with `6001 "time list is null"` | live `setTimeChargeBySn` call | + +### `expMsg` is worth reading + +Every response carries an `expMsg` field. It's `null` most of the time, but on some parameter +errors it's the only thing that tells you what you actually got wrong — `msg` just says +"Parameter error" without naming the parameter, whereas `expMsg` says `"time list is null"`. +Since 0.0.21 it gets logged, and it's on `AlphaESSApiError.expMsg`. --- @@ -680,7 +705,19 @@ default, since most systems return `6017`. - Maximum **6 groups per day**, maximum **28 groups per week**. - Charge and discharge periods **must not overlap**. -- Both lists are required even if empty — send `[]`, not `null`. +- **Both lists need at least one period in them.** Sending `[]` gets you `6001` with + `expMsg: "time list is null"` — an empty list counts as null. Leaving the key out altogether + gets you `10001` instead. + +That last one is awkward if you only care about charging: there's no obvious way to say "I have +no discharge periods". A `00:00`–`00:00` element does get past validation, but we don't know +whether the device treats that as a zero-length window or as one that wraps midnight and runs all +day, so it isn't safe to use as a filler. + +> **Validation happens before the permission check.** Send a malformed payload to a system that +> isn't entitled to this endpoint and you'll still get `6001` or `10001` — `6017` only appears +> once the payload is valid. So an early `6001` tells you nothing about whether your account has +> access. Captured on a SMILE5-INV that returns `6017` for both the read and the write. **Returns:** `data` is `null`. Success is `code: 200`. diff --git a/docs/RETURN_CODES.md b/docs/RETURN_CODES.md index 123ebfd..9f625fa 100644 --- a/docs/RETURN_CODES.md +++ b/docs/RETURN_CODES.md @@ -144,6 +144,22 @@ Observed in production but absent from the portal's Return Code Description page | Code | Message | Cause | |:--|:--|:--| | `6017` | `No operation permissions` | Your AppID is bound to the SN, but the account tier or the hardware is not entitled to this endpoint. Confirmed on `getTimeChargeBySn` against two bound SMILE5 systems; the same call with an unbound SN returned `6005` instead, proving the binding check passes first and the entitlement check fails second. Handle it as "feature unavailable for this system", not as an error to retry. | +| `10001` | `Parameter Error` | The request body was malformed, as opposed to `6001` which is a valid body with a bad value in it. Seen on `setTimeChargeBySn` when `dischargeTimeList` was left out entirely. Two things to watch: the capitalisation differs from `6001`'s `Parameter error`, and it's outside the `6xxx` range, so anything assuming `6000 <= code <= 6099` will miss it. | + +### Errors come back in a particular order + +Parameters get validated before permissions. If your payload is malformed you'll get `6001` or +`10001` even on a system that has no access to the endpoint at all — `6017` only shows up once +the payload is valid. Here's the same endpoint on the same SMILE5-INV, three times: + +| Request | Response | +|:--|:--| +| `dischargeTimeList: []` | `6001` — `expMsg: "time list is null"` | +| `dischargeTimeList` omitted | `10001` | +| both lists valid | `6017` | + +So getting a `6001` back doesn't mean your account has permission — it just means you never got +far enough to find out. --- @@ -200,6 +216,43 @@ Two consequences worth knowing: success, and the wrappers also return `None` on failure. To confirm a write landed, either watch the log or read the value back with the corresponding `get` endpoint. +### Opting in to exceptions — `raise_on_error` (0.0.21+) + +Both of those go away if you build the client with `raise_on_error=True`. API-level failures then +raise `AlphaESSApiError` with the code attached, and success just means nothing was raised — +which is the only way to confirm a write on the endpoints that return `data: null` either way. + +```python +from alphaess.alphaess import alphaess, AlphaESSApiError + +client = alphaess(appID, appSecret, raise_on_error=True) + +try: + await client.setTimeChargeBySn(sysSn, 0, charge_list, discharge_list) +except AlphaESSApiError as err: + if err.code == 6017: + ... # not entitled to the periodic API — stop trying + else: + ... # err.expMsg often says which parameter was wrong +except aiohttp.ClientError: + ... # transport failure — back off and retry +``` + +| Attribute | Description | +|:--|:--| +| `code` | The `code` field, e.g. `6001` | +| `msg` | The `msg` (or `info`) field. Localised, so don't branch on it | +| `expMsg` | Exception detail, e.g. `"time list is null"` — often the only thing naming the bad parameter | +| `description` | The English description from `RETURN_CODES` / `UNDOCUMENTED_RETURN_CODES`, if we know the code | +| `path` | The URL that was called | + +The flag is off by default, so upgrading won't change anything unless you ask it to. Transport +errors behave the same either way — they always propagate. + +`getdata()` is the one exception, and deliberately so. It stays best-effort even with the flag +on: an endpoint your account can't use leaves that one key `None` instead of costing you the +whole poll. Transport errors still stop it, since those affect every endpoint anyway. + ### Transport-level errors → the exception propagates Connection resets, DNS failures, timeouts and non-2xx HTTP statuses (`raise_for_status` is set, diff --git a/setup.py b/setup.py index 848d4c1..bfe24e6 100644 --- a/setup.py +++ b/setup.py @@ -5,7 +5,7 @@ setup( name="alphaessopenapi", - version="0.0.20", + version="0.0.21", author="Charles Gillanders", author_email="charles@charlesgillanders.com", description="A python library to retrieve energy statistics from your Alpha ESS inverter by polling the Official Alpha ESS Open API.", diff --git a/tests/test_getdata_best_effort.py b/tests/test_getdata_best_effort.py new file mode 100644 index 0000000..69ea0d7 --- /dev/null +++ b/tests/test_getdata_best_effort.py @@ -0,0 +1,74 @@ +"""getdata keeps its best-effort contract when raise_on_error is set. + +getdata is documented as returning whatever it managed to gather. Strict mode +must not turn one refused endpoint into a total loss of the poll. +""" +from unittest.mock import AsyncMock, MagicMock + +import aiohttp +import pytest + +from alphaess.alphaess import AlphaESSApiError, alphaess + + +def _client(): + client = alphaess("appid", "appsecret", session=MagicMock(), raise_on_error=True) + client.getESSList = AsyncMock(return_value=[{"sysSn": "SN"}]) + for name in ("getSumDataForCustomer", "getOneDateEnergyBySn", "getLastPowerData", + "getChargeConfigInfo", "getDisChargeConfigInfo", "getOneDayPowerBySn", + "getTimeChargeBySn", "getEvChargerConfigList"): + setattr(client, name, AsyncMock(return_value={"ok": True})) + return client + + +async def test_one_refused_endpoint_does_not_lose_the_rest(): + client = _client() + client.getChargeConfigInfo = AsyncMock( + side_effect=AlphaESSApiError(code=6017, description="No operation permissions")) + + result = await client.getdata() + + assert len(result) == 1 + assert result[0]["ChargeConfig"] is None + assert result[0]["SumData"] == {"ok": True} + assert result[0]["DisChargeConfig"] == {"ok": True} + + +async def test_refused_timecharge_still_returns_the_unit(): + """getTimeChargeBySn answers 6017 on most systems -- the common case.""" + client = _client() + client.getTimeChargeBySn = AsyncMock(side_effect=AlphaESSApiError(code=6017)) + + result = await client.getdata(get_timecharge=True) + + assert result[0]["TimeCharge"] is None + assert result[0]["LastPower"] == {"ok": True} + + +async def test_refused_esslist_returns_empty_not_an_exception(): + client = _client() + client.getESSList = AsyncMock(side_effect=AlphaESSApiError(code=6042)) + + assert await client.getdata() == [] + + +async def test_transport_errors_still_propagate(): + """A connection failure affects every endpoint -- don't paper over it.""" + client = _client() + client.getChargeConfigInfo = AsyncMock(side_effect=aiohttp.ClientConnectionError("boom")) + + with pytest.raises(aiohttp.ClientConnectionError): + await client.getdata() + + +async def test_default_mode_is_unaffected(): + """Without raise_on_error nothing raises in the first place.""" + client = alphaess("appid", "appsecret", session=MagicMock()) + client.getESSList = AsyncMock(return_value=[{"sysSn": "SN"}]) + for name in ("getSumDataForCustomer", "getOneDateEnergyBySn", "getLastPowerData", + "getChargeConfigInfo", "getDisChargeConfigInfo"): + setattr(client, name, AsyncMock(return_value=None)) + + result = await client.getdata() + + assert result[0]["ChargeConfig"] is None diff --git a/tests/test_raise_on_error.py b/tests/test_raise_on_error.py new file mode 100644 index 0000000..5ea6517 --- /dev/null +++ b/tests/test_raise_on_error.py @@ -0,0 +1,186 @@ +"""Tests for opt-in strict error handling (raise_on_error / AlphaESSApiError). + +Half of this file is deliberately regression testing: the default behaviour of +0.0.21 must be indistinguishable from 0.0.20, so that anyone who upgrades +without reading the changelog sees no difference at all. + +The aiohttp session is fully mocked with unittest.mock -- no network access. +""" +import inspect +import logging +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from alphaess.alphaess import ( + UNDOCUMENTED_RETURN_CODES, + AlphaESSApiError, + alphaess, +) + +from .test_time_charge import _GetContextManager, _make_response + +# A real rejection, captured from the live API: setTimeChargeBySn with an empty +# dischargeTimeList. expMsg carries the only useful detail. +EMPTY_LIST_REJECTION = { + "code": 6001, "msg": "Parameter error", + "expMsg": "time list is null", "data": None, "extra": None, +} +NOT_ENTITLED = { + "code": 6017, "msg": "No operation permissions", + "expMsg": None, "data": None, "extra": None, +} + + +def _client(get_body=None, post_body=None, **kwargs): + session = MagicMock() + if get_body is not None: + session.get = MagicMock(return_value=_GetContextManager(_make_response(get_body))) + if post_body is not None: + session.post = AsyncMock(return_value=_make_response(post_body)) + return alphaess("appid", "appsecret", session=session, **kwargs) + + +# -------------------------------------------------------------------------- +# Backwards compatibility -- the default must not change +# -------------------------------------------------------------------------- + +def test_raise_on_error_defaults_to_false(): + assert alphaess("appid", "appsecret", session=MagicMock()).raise_on_error is False + + +def test_raise_on_error_is_appended_last_in_the_signature(): + """Existing positional callers must keep working unchanged.""" + params = list(inspect.signature(alphaess.__init__).parameters) + assert params == [ + "self", "appID", "appSecret", "session", "timeout", + "ipaddress", "verify_ssl", "raise_on_error", + ] + + +async def test_default_mode_still_returns_none_on_api_error(): + client = _client(get_body=NOT_ENTITLED) + assert await client.getTimeChargeBySn("SN") is None + + +async def test_default_mode_still_returns_none_on_post_rejection(): + client = _client(post_body=EMPTY_LIST_REJECTION) + assert await client.setTimeChargeBySn("SN", 0, [], []) is None + + +async def test_default_mode_never_raises_the_new_exception(): + client = _client(get_body=EMPTY_LIST_REJECTION, post_body=EMPTY_LIST_REJECTION) + assert await client.getChargeConfigInfo("SN") is None + assert await client.updateChargeConfigInfo( + "SN", 100, 0, "00:00", "00:00", "00:00", "00:00") is None + + +async def test_success_return_value_is_unchanged_in_strict_mode(): + """Strict mode only affects failures; successes return exactly as before.""" + body = {"code": 200, "msg": "Success", "data": {"gridCharge": 1}} + assert await _client(get_body=body).getChargeConfigInfo("SN") == {"gridCharge": 1} + assert await _client(get_body=body, raise_on_error=True).getChargeConfigInfo( + "SN") == {"gridCharge": 1} + + +async def test_write_success_returns_none_in_both_modes(): + """The write endpoints answer with data: null on success. That stays true -- + strict mode signals success by NOT raising, not by a new return value.""" + body = {"code": 200, "msg": "Success", "data": None} + assert await _client(post_body=body).setTimeChargeBySn("SN", 0, [], []) is None + assert await _client(post_body=body, raise_on_error=True).setTimeChargeBySn( + "SN", 0, [], []) is None + + +# -------------------------------------------------------------------------- +# Strict mode +# -------------------------------------------------------------------------- + +async def test_strict_mode_raises_on_get_failure(): + client = _client(get_body=NOT_ENTITLED, raise_on_error=True) + with pytest.raises(AlphaESSApiError) as excinfo: + await client.getTimeChargeBySn("SN") + assert excinfo.value.code == 6017 + + +async def test_strict_mode_raises_on_post_failure(): + client = _client(post_body=EMPTY_LIST_REJECTION, raise_on_error=True) + with pytest.raises(AlphaESSApiError): + await client.setTimeChargeBySn("SN", 0, [], []) + + +async def test_exception_carries_the_diagnostic_fields(): + client = _client(post_body=EMPTY_LIST_REJECTION, raise_on_error=True) + with pytest.raises(AlphaESSApiError) as excinfo: + await client.setTimeChargeBySn("SN", 0, [], []) + + err = excinfo.value + assert err.code == 6001 + assert err.msg == "Parameter error" + assert err.expMsg == "time list is null" + assert err.description == "Parameter error" + assert err.path.endswith("/setTimeChargeBySn") + # expMsg is the only thing that says WHICH parameter was wrong. + assert "time list is null" in str(err) + + +async def test_undocumented_code_is_described_on_the_exception(): + client = _client(get_body=NOT_ENTITLED, raise_on_error=True) + with pytest.raises(AlphaESSApiError) as excinfo: + await client.getTimeChargeBySn("SN") + assert excinfo.value.description == UNDOCUMENTED_RETURN_CODES[6017] + + +async def test_10001_is_recognised(): + """Returned when a required list is omitted entirely rather than empty.""" + client = _client(post_body={"code": 10001, "msg": "Parameter Error", "data": None}, + raise_on_error=True) + with pytest.raises(AlphaESSApiError) as excinfo: + await client.setTimeChargeBySn("SN", 0, [], []) + assert excinfo.value.description == UNDOCUMENTED_RETURN_CODES[10001] + + +async def test_strict_mode_leaves_transport_errors_alone(): + """A transport failure must stay distinguishable from an API rejection.""" + import aiohttp + session = MagicMock() + session.get = MagicMock(side_effect=aiohttp.ClientConnectionError("boom")) + client = alphaess("appid", "appsecret", session=session, raise_on_error=True) + + with pytest.raises(aiohttp.ClientConnectionError): + await client.getLastPowerData("SN") + + +async def test_failure_is_logged_once_not_once_per_wrapper(caplog): + """__handle_failure logs; the wrappers must not log the same thing again.""" + client = _client(get_body=NOT_ENTITLED, raise_on_error=True) + + with caplog.at_level(logging.ERROR, logger="alphaess.alphaess"): + with pytest.raises(AlphaESSApiError): + await client.getTimeChargeBySn("SN") + + assert len([r for r in caplog.records if r.levelno >= logging.ERROR]) == 1 + + +# -------------------------------------------------------------------------- +# Logging improvements, which apply in both modes +# -------------------------------------------------------------------------- + +async def test_expmsg_is_included_in_the_error_log(caplog): + client = _client(post_body=EMPTY_LIST_REJECTION) + + with caplog.at_level(logging.ERROR, logger="alphaess.alphaess"): + await client.setTimeChargeBySn("SN", 0, [], []) + + assert "time list is null" in caplog.text + + +async def test_successful_but_empty_response_is_not_logged_as_an_error(caplog): + """getVerificationCode and the write endpoints legitimately return no data.""" + client = _client(get_body={"code": 200, "msg": "Success", "data": None}) + + with caplog.at_level(logging.DEBUG, logger="alphaess.alphaess"): + result = await client.getVerificationCode("SN", "CHECK") + + assert result is None + assert not [r for r in caplog.records if r.levelno >= logging.ERROR]