Repository: bachya/simplisafe-python (language: python)

Issue:
Overhaul request retry mechanism
**Describe what the PR does:**

The previous manner for handling request retries (specifically, in the case of `401` responses) was fairly brittle and one-shot-ish. This PR puts an official request retry mechanism in place.

As a side effect, this should help uses who are currently getting bitten by the flakiness of SimpliSafe's cloud API and how, over the past several months, it can tend to return `409`/`500`/`502` responses during critical activities (like arming the alarm).

**Does this fix a specific issue?**

N/A
  
**Checklist:**

- [x] Confirm that one or more new tests are written for the new functionality.
- [x] Run tests and ensure everything passes (with 100% test coverage).
- [x] Update `README.md` and `docs/` with any new documentation.
- [ ] Add yourself to `AUTHORS.md`.

Clean up retry logging and remove superfluous branch
**Describe what the PR does:**

Following up on #222, this PR:

1. Fixes some incorrect logging during request retry
2. Removes a superfluous branch that couldn't be reached

**Does this fix a specific issue?**

N/A
  
**Checklist:**

- [ ] Confirm that one or more new tests are written for the new functionality.
- [x] Run tests and ensure everything passes (with 100% test coverage).
- [ ] Update `README.md` and `docs/` with any new documentation.
- [ ] Add yourself to `AUTHORS.md`.



Discussion / hints:
# [Codecov](https://codecov.io/gh/bachya/simplisafe-python/pull/222?src=pr&el=h1) Report
> Merging [#222](https://codecov.io/gh/bachya/simplisafe-python/pull/222?src=pr&el=desc) (17e14f5) into [master](https://codecov.io/gh/bachya/simplisafe-python/commit/af5166cdc73baaa5c480a1d075ca5fb2160a6fc4?el=desc) (af5166c) will **not change** coverage.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/bachya/simplisafe-python/pull/222/graphs/tree.svg?width=650&height=150&src=pr&token=HMjB786MgZ)](https://codecov.io/gh/bachya/simplisafe-python/pull/222?src=pr&el=tree)

```diff
@@            Coverage Diff            @@
##            master      #222   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           15        15           
  Lines          857       858    +1     
=========================================
+ Hits           857       858    +1     
```


| [Impacted Files](https://codecov.io/gh/bachya/simplisafe-python/pull/222?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [simplipy/api.py](https://codecov.io/gh/bachya/simplisafe-python/pull/222/diff?src=pr&el=tree#diff-c2ltcGxpcHkvYXBpLnB5) | `100.00% <100.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/bachya/simplisafe-python/pull/222?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/bachya/simplisafe-python/pull/222?src=pr&el=footer). Last update [af5166c...17e14f5](https://codecov.io/gh/bachya/simplisafe-python/pull/222?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

Code Climate has analyzed commit 285bec74 and detected **0 issues** on this pull request.

View more on [Code Climate](https://codeclimate.com/github/bachya/simplisafe-python/pull/223).

# [Codecov](https://codecov.io/gh/bachya/simplisafe-python/pull/223?src=pr&el=h1) Report
> Merging [#223](https://codecov.io/gh/bachya/simplisafe-python/pull/223?src=pr&el=desc) (285bec7) into [dev](https://codecov.io/gh/bachya/simplisafe-python/commit/27be2e9e85c9bf118109ea0972722ce60e0a925f?el=desc) (27be2e9) will **not change** coverage.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/bachya/simplisafe-python/pull/223/graphs/tree.svg?width=650&height=150&src=pr&token=HMjB786MgZ)](https://codecov.io/gh/bachya/simplisafe-python/pull/223?src=pr&el=tree)

```diff
@@            Coverage Diff            @@
##               dev      #223   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           15        15           
  Lines          858       858           
=========================================
  Hits           858       858           
```


| [Impacted Files](https://codecov.io/gh/bachya/simplisafe-python/pull/223?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [simplipy/api.py](https://codecov.io/gh/bachya/simplisafe-python/pull/223/diff?src=pr&el=tree#diff-c2ltcGxpcHkvYXBpLnB5) | `100.00% <100.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/bachya/simplisafe-python/pull/223?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/bachya/simplisafe-python/pull/223?src=pr&el=footer). Last update [27be2e9...285bec7](https://codecov.io/gh/bachya/simplisafe-python/pull/223?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Fix patch:
diff --git a/simplipy/api.py b/simplipy/api.py
index e83214db..f65dc954 100644
--- a/simplipy/api.py
+++ b/simplipy/api.py
@@ -295,6 +295,17 @@ async def request(  # pylint: disable=too-many-branches
 
                     LOGGER.debug("Data received from /%s: %s", endpoint, data)
 
+                    if isinstance(data, str):
+                        # In some cases, the SimpliSafe API will return a quoted string
+                        # in its response body (e.g., "\"node not found\""), which is
+                        # technically valid JSON. Additionally, SimpliSafe sets that
+                        # response's Content-Type header to application/json (#smh).
+                        # Together, these factors will allow a non-true-JSON  payload to
+                        # escape the try/except above. So, if we get here, we use the
+                        # string value (with quotes removed) to raise an error:
+                        message = data.replace('"', "")
+                        data = {"error": message}
+
                     resp.raise_for_status()
                     return data
             except ClientError as err:


Test patch:
diff --git a/tests/test_api.py b/tests/test_api.py
index 5139f4a6..75a44bfe 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -32,7 +32,7 @@ async def test_401_bad_credentials(aresponses):
         "api.simplisafe.com",
         "/v1/api/token",
         "post",
-        aresponses.Response(text="Unauthorized", status=401),
+        aresponses.Response(text='"Unauthorized"', status=401),
     )
 
     async with aiohttp.ClientSession() as session:
@@ -64,13 +64,13 @@ async def test_401_refresh_token_failure(
             "api.simplisafe.com",
             f"/v1/subscriptions/{TEST_SUBSCRIPTION_ID}/settings",
             "get",
-            aresponses.Response(text="Unauthorized", status=401),
+            aresponses.Response(text='"Unauthorized"', status=401),
         )
         v2_server.add(
             "api.simplisafe.com",
             "/v1/api/token",
             "post",
-            aresponses.Response(text="Unauthorized", status=401),
+            aresponses.Response(text='"Unauthorized"', status=401),
         )
 
         async with aiohttp.ClientSession() as session:
@@ -100,7 +100,7 @@ async def test_401_refresh_token_success(
             "api.simplisafe.com",
             f"/v1/users/{TEST_USER_ID}/subscriptions",
             "get",
-            aresponses.Response(text="Unauthorized", status=401),
+            aresponses.Response(text='"Unauthorized"', status=401),
         )
         v2_server.add(
             "api.simplisafe.com",