bear / python-twitter

A Python wrapper around the Twitter API.
Apache License 2.0
3.41k stars 955 forks source link

The use of TwitterError #639

Closed webtweakers closed 4 years ago

webtweakers commented 4 years ago

Throughout the python-twitter library, TwitterError has been used in a few different ways: for value errors, for type errors, for http errors and for Twitter's API errors. And in all cases TwitterError will be instantiated in different ways. TwitterError contains either a string, list or dict. Some examples:

String: raise TwitterError("The twitter.Api instance must be authenticated.")

List: raise TwitterError(data['errors'])

Dict: raise TwitterError({'message': "Image data could not be processed"})

Additionally the Twitter API regularly returns an HTTP status code, like 420, which is not properly caught by python-twitter. It will simply do: raise TwitterError({'Unknown error': '{0}'.format(json_data)})

A similar issue has been raised before: https://github.com/bear/python-twitter/issues/569

Twitter is keen on returning errors when trying to use their API, so catching and being able to handle those errors properly is really crucial. The suggested method in issue 569 is insufficient.

I would propose to let ValueErrors and TypeErrors just be - don't cast them to TwitterError.

From the Python documentation:

Passing arguments of the wrong type (e.g. passing a list when an int is expected) should result in a TypeError, but passing arguments with the wrong value (e.g. a number outside expected boundaries) should result in a ValueError.

So a case like this:

try:
    total_count = int(total_count)
except ValueError:
    raise TwitterError({'message': "total_count must be an integer"})

Would become this:

try:
    total_count = int(total_count)
except ValueError:
    raise TypeError("total_count must be an integer")

Another example - this:

if len(uids) > 100:
    raise TwitterError("No more than 100 users may be requested per request.")

Would become this:

if len(uids) > 100:
    raise ValueError("No more than 100 users may be requested per request.")

What is left are the actual API errors, returned by Twitter's API. Here it would be REALLY helpful if a thing like TwitterAPIError existed, containing a single error with a message and code attribute.

Additionally it would be extremely helpful if the HTTP errors were not swept under the carpet as an "Unknown error", but bubbled up so they could be properly handled as well. Maybe introduce an additional TwitterHTTPError for those cases.

I'm realising these changes would make python-twitter not backward compatible, but that's something I personally can live with. I will probably create a branch and do this myself. Would a Pull-Request be appreciated, when/if I get to that point?

webtweakers commented 4 years ago

Actually... Nevermind. :)