koalalorenzo / python-digitalocean

🐍🐳 Python module to manage Digital Ocean droplets
GNU Lesser General Public License v3.0
1.26k stars 301 forks source link

Error (400/500) responses with no body raise JSONReadError #355

Closed camfairchild closed 2 years ago

camfairchild commented 2 years ago

When the API returns a 400/500 error with no body, this raises a JSONReadError

│ an/Manager.py:66 in get_all_droplets                                         │
│                                                                              │
│    63 │   │   if tag_name:                                                   │
│    64 │   │   │   params["tag_name"] = tag_name                              │
│    65 │   │                                                                  │
│ ❱  66 │   │   data = self.get_data("droplets/", params=params)               │
│    67 │   │                                                                  │
│    68 │   │   droplets = list()                                              │
│    69 │   │   for jsoned in data['droplets']:                                │
│                                                                              │
│ ╭───────────────────────── locals ──────────────────────────╮                │
│ │   params = {'tag_name': ['example_tag'], 'per_page': 200} │                │
│ │     self = <Manager>                                      │                │
│ │ tag_name = ['example_tag']                                │                │
│ ╰───────────────────────────────────────────────────────────╯                │
│                                                                              │
│ /home/user/anaconda3/envs/cudareg/lib/python3.10/site-packages/digitaloce │
│ an/baseapi.py:227 in get_data                                                │
│                                                                              │
│   224 │   │   │   data = req.json()                                          │
│   225 │   │                                                                  │
│   226 │   │   except ValueError as e:                                        │
│ ❱ 227 │   │   │   raise JSONReadError(                                       │
│   228 │   │   │   │   'Read failed from DigitalOcean: %s' % str(e)           │
│   229 │   │   │   )                                                          │
│   230                                                                        │
│                                                                              │
│ ╭──────────────────────── locals ─────────────────────────╮                  │
│ │ params = {'tag_name': ['example_tag'], 'per_page': 200} │                  │
│ │    req = <Response [504]>                               │                  │
│ │   self = <Manager>                                      │                  │
│ │   type = 'GET'                                          │                  │
│ │    url = 'droplets/'                                    │                  │
│ ╰─────────────────────────────────────────────────────────╯                  │
╰──────────────────────────────────────────────────────────────────────────────╯
JSONReadError: Read failed from DigitalOcean: Expecting value: line 1 column 1 
(char 0)

Per the DigitalOcean API v2 docs

400 and 500 level error responses will include a JSON object in their body, including the following attributes:

https://docs.digitalocean.com/reference/api/api-reference/#section/Introduction/HTTP-Statuses So I don't think it deserves a custom exception. Instead, I think it should raise a requests.HTTPError as used in https://requests.readthedocs.io/en/stable/api/#requests.Response.raise_for_status