parameters.py - Expanding Error Code Catching#767
Conversation
One public API I use returns errorCode: <error>. This should now catch a wider variety of errors.
auvipy
left a comment
There was a problem hiding this comment.
would be great if you also update the unit tests
thedrow
left a comment
There was a problem hiding this comment.
Could you please provide an example where this is useful?
|
If I have time this week I'll investigate your testing code. From Autodesk API an example error is: {'developerMessage': 'The required parameter(s) client_id not present in the request', 'errorCode': 'AUTH-008', 'more info': 'https://forge.autodesk.com/en/docs/oauth/v2/developers_guide/error_handling/'}So in parameters.py: if 'error' in params:
raise_from_error(params.get('error'), params)'error' is not in |
|
Or rather I had a few moments now. Note I'm editing on GitHub and haven't cloned locally yet. So this might take a commit or two, |
|
Hi, I think the PR is too flexible as it does not enforce the list of field names. While the lowercase check might be a good idea (however a CaseInsensitiveDict would be easier to read?), the string search |
JonathanHuot
left a comment
There was a problem hiding this comment.
We need to enforce the list of field names.
Change lowercase into a CaseInsensitiveDict.
The string search "error" in <string> cannot work (acceptError matches).
|
So if we had a predefined list of error codes? This way it follows RFC but can have added cases for API's that don't follow "standards" self.reportable_errors = ["error", "Error"]
def get_errors(self)
return self.reportable_errors
def add_error_code(self, err)
self.reportable_errors.append(err)
.
.
.
for key in params.keys():
if key in self.reportable_errors:
raise_from_error(params.get(key), params)
.
.
. |
|
Yes, that could work. However, I think the default's list should be what is in the RFC: only includes |
One public API I use returns errorCode: . This should now catch a wider variety of errors.