smartcar / python-sdk

Smartcar Python SDK
MIT License
44 stars 13 forks source link

Wrap simplejson.errors.JSONDecodeError inside SmartcarException #79

Closed dbartenstein closed 3 years ago

dbartenstein commented 4 years ago

On September 10th, 2020 between 7 p.m. and 8 p.m. CEST we faced issues when using Smartcar’s API to communicate with Tesla vehicles. After five minutes a simplejson.errors.JSONDecodeError exception was thrown.

Wouldn’t it be nicer to have that exception wrapped in a SmartcarException with additional information about what caused the error?

  File "/opt/.virtualenvs/django22/lib/python3.7/site-packages/smartcar/vehicle.py", line 311, in batch
    response = self.api.batch(requests)
  File "/opt/.virtualenvs/django22/lib/python3.7/site-packages/smartcar/api.py", line 79, in batch
    return requester.call('POST', url, json=json, headers=headers)
  File "/opt/.virtualenvs/django22/lib/python3.7/site-packages/smartcar/requester.py", line 31, in call
    body = response.json()
  File "/opt/.virtualenvs/django22/lib/python3.7/site-packages/requests/models.py", line 898, in json
    return complexjson.loads(self.text, **kwargs)
  File "/opt/.virtualenvs/django22/lib/python3.7/site-packages/simplejson/__init__.py", line 525, in loads
    return _default_decoder.decode(s)
  File "/opt/.virtualenvs/django22/lib/python3.7/site-packages/simplejson/decoder.py", line 370, in decode
    obj, end = self.raw_decode(s)
  File "/opt/.virtualenvs/django22/lib/python3.7/site-packages/simplejson/decoder.py", line 400, in raw_decode
    return self.scan_once(s, idx=_w(s, idx).end())
simplejson.errors.JSONDecodeError: Expecting value: line 1 column 1 (char 0)
dbartenstein commented 4 years ago

Addition

Very likely the request did not return a JSON body after five minutes, thus body = response.json() failed and the simplejson.errors.JSONDecodeError was raised. https://github.com/smartcar/python-sdk/blob/master/smartcar/requester.py#L31

Background

As a developer I want to call actions via Smartcar’s Python SDK and not having to catch any other exceptions than those derived from SmartcarException.

sankethkatta commented 3 years ago

Hello @dbartenstein, thanks for raising this issue. During that brief window you mentioned, Smartcar's API had an issue where a small subset of errors were not returning the our standard JSON error format. The issue has since been fixed and we do not expect this situation to arise again.

That being said, we like your suggestion to have all SDK errors bubble up to the developer as a SmartcarException. We will put up a PR to implement this and we'll leave this issue open until it is merged.

sankethkatta commented 3 years ago

Fixed by #84