Skip to content

Better failure handling for failed SSO #91

@ohshazbot

Description

@ohshazbot

Hi, I experienced an error in an upstream utility using this library.

withings-gc-bridge  | Traceback (most recent call last):
withings-gc-bridge  |   File "/usr/local/lib/python3.10/site-packages/requests/models.py", line 974, in json
withings-gc-bridge  |     return complexjson.loads(self.text, **kwargs)
withings-gc-bridge  |   File "/usr/local/lib/python3.10/json/__init__.py", line 346, in loads
withings-gc-bridge  |     return _default_decoder.decode(s)
withings-gc-bridge  |   File "/usr/local/lib/python3.10/json/decoder.py", line 337, in decode
withings-gc-bridge  |     obj, end = self.raw_decode(s, idx=_w(s, 0).end())
withings-gc-bridge  |   File "/usr/local/lib/python3.10/json/decoder.py", line 355, in raw_decode
withings-gc-bridge  |     raise JSONDecodeError("Expecting value", s, err.value) from None
withings-gc-bridge  | json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)
withings-gc-bridge  | 
withings-gc-bridge  | During handling of the above exception, another exception occurred:
withings-gc-bridge  | 
withings-gc-bridge  | Traceback (most recent call last):
withings-gc-bridge  |   File "/app/main.py", line 283, in <module>
withings-gc-bridge  |     bridge.sync()
withings-gc-bridge  |   File "/app/main.py", line 81, in sync
withings-gc-bridge  |     garmin = self.init_garmin()
withings-gc-bridge  |   File "/app/main.py", line 105, in init_garmin
withings-gc-bridge  |     garmin.login(self.tokenstore)
withings-gc-bridge  |   File "/usr/local/lib/python3.10/site-packages/garminconnect/__init__.py", line 239, in login
withings-gc-bridge  |     self.display_name = self.garth.profile["displayName"]
withings-gc-bridge  |   File "/usr/local/lib/python3.10/site-packages/garth/http.py", line 107, in profile
withings-gc-bridge  |     return self.user_profile
withings-gc-bridge  |   File "/usr/local/lib/python3.10/site-packages/garth/http.py", line 97, in user_profile
withings-gc-bridge  |     self._user_profile = self.connectapi(
withings-gc-bridge  |   File "/usr/local/lib/python3.10/site-packages/garth/http.py", line 177, in connectapi
withings-gc-bridge  |     resp = self.request(method, "connectapi", path, api=True, **kwargs)
withings-gc-bridge  |   File "/usr/local/lib/python3.10/site-packages/garth/http.py", line 133, in request
withings-gc-bridge  |     self.refresh_oauth2()
withings-gc-bridge  |   File "/usr/local/lib/python3.10/site-packages/garth/http.py", line 172, in refresh_oauth2
withings-gc-bridge  |     self.oauth2_token = sso.exchange(self.oauth1_token, self)
withings-gc-bridge  |   File "/usr/local/lib/python3.10/site-packages/garth/sso.py", line 198, in exchange
withings-gc-bridge  |     ).json()
withings-gc-bridge  |   File "/usr/local/lib/python3.10/site-packages/requests/models.py", line 978, in json
withings-gc-bridge  |     raise RequestsJSONDecodeError(e.msg, e.doc, e.pos)
withings-gc-bridge  | requests.exceptions.JSONDecodeError: Expecting value: line 1 column 1 (char 0)

While it would be nice for upstream libraries (withings-gc-bridge & garminconnect) to have handling, I feel like there's some room for improvement in this library. Look at the sso code, it looks like you're directly pulling the json() result of the post without checking for errors: https://github.com/matin/garth/blob/main/garth/sso.py#L193-L198

I would like to think adding in a raise_for_status() would at least prevent the library from eating the underlying error occurring, improving ability to debug and/or do exception handling:

    result = sess.post(
        url,
        headers=headers,
        data=data,
        timeout=client.timeout,
    )
    result.raise_for_status()
    token = result.json()

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions