search

meraki / dashboard-api-python

Official Dashboard API library (SDK) for Python
MIT License
289 stars 151 forks source link

Exceptions from API are not handled #201

Closed andy-shady-org closed 1 year ago

andy-shady-org commented 1 year ago

When raising APIError on line 187 of rest_session.py, its failing to pass any information about the handled request exception. For example, if the error is a requests.exceptions.ProxyError, this is caught on line 181, but any details are lost as 'e' is not passed to APIError, albeit logged, not as an error however, so people with logging off, would never see it.

In addition, if an exception is raised by requests for each of the retry times, the value of response will never be set, so passing this to APIError becomes pointless so anyone catching APIError will not know what failed.

TKIPisalegacycipher commented 1 year ago

Hi @andy-shady-org, I think that makes sense and good catch! Would you like to draft a PR to update the behavior?

andy-shady-org commented 1 year ago

sure, I'll write a patch for this.

andy-shady-org commented 1 year ago

OK, I've submitted a pull request. Basically, Ive added a class to the exceptions to wrap any exception coming from requests. Some of these will be OSError exceptions, like the ProxyError mentioned, which wont have a status code, so for these Ive raised a 503. This could probably be done better, but OSError.errno is not always populated, so 503 is better than nothing.

As you call response.json() in your APIError class, Ive added a method so this doesn't raise an exception, as any given RequestException would potentially not have this call.

TKIPisalegacycipher commented 1 year ago

Thanks again @andy-shady-org, I've merged your PR and I'll close this issue. Stay tuned for the next published release of the library.