From f8457a2c791ea4fa8238757053dd7c9d3127ad13 Mon Sep 17 00:00:00 2001 From: Guyzmo Date: Sat, 2 Apr 2016 15:21:47 +0200 Subject: [PATCH 1/2] Improved dispatch error handling Instead of a generic 'service not found' error for any request with HTTP error code between 400 and 500, added checks for 400, 401, 403 and for all the others in that range, give a generic 'Request error' message. Also, changed the second part of the error checking from a simple string into a dict with more details on the error (with message as string, reason as the given json object, and code as the HTTP code returned, to make it easier to act accordingly to the type of error returned by the API. --- bitbucket/bitbucket.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/bitbucket/bitbucket.py b/bitbucket/bitbucket.py index e7d7b2b..424a972 100644 --- a/bitbucket/bitbucket.py +++ b/bitbucket/bitbucket.py @@ -243,12 +243,20 @@ def dispatch(self, method, url, auth=None, params=None, **kwargs): False, 'Unauthorized access, ' 'please check your credentials.') - elif status >= 400 and status < 500: - return (False, 'Service not found.') + elif status == 404: + return (False, dict(message='Service not found', reason=error, code=status)) + elif status == 400: + return (False, dict(message='Bad request sent to server.', reason=error, code=status)) + elif status == 401: + return (False, dict(message='Not enough privileges.', reason=error, code=status)) + elif status == 403: + return (False, dict(message='Not authorized.', reason=error, code=status)) + elif status == 402 or status >= 405: + return (False, dict(message='Request error.', reason=error, code=status)) elif status >= 500 and status < 600: - return (False, 'Server error.') + return (False, dict(message='Server error.', reason=error, code=status)) else: - return (False, error) + return (False, dict(message='Unidentified error.', reason=error, code=status)) def url(self, action, **kwargs): """ Construct and return the URL for a specific API service. """ From e6476fa36d21d0886bc57ef23913fd8fe2f5e17e Mon Sep 17 00:00:00 2001 From: Guyzmo Date: Sat, 2 Apr 2016 16:04:10 +0200 Subject: [PATCH 2/2] Updated tests for new error handling - added KeyError, as when it returns an error the repositories key is missing and the TypeError used does not catch that issue. - added test assertions against other error fields --- bitbucket/bitbucket.py | 2 ++ bitbucket/repository.py | 2 ++ bitbucket/tests/public.py | 8 ++++++-- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/bitbucket/bitbucket.py b/bitbucket/bitbucket.py index 424a972..b5989e4 100644 --- a/bitbucket/bitbucket.py +++ b/bitbucket/bitbucket.py @@ -278,6 +278,8 @@ def get_user(self, username=None): return (response[0], response[1]['user']) except TypeError: pass + except KeyError: + pass return response def get_tags(self, repo_slug=None): diff --git a/bitbucket/repository.py b/bitbucket/repository.py index b900b9b..7e38bd9 100644 --- a/bitbucket/repository.py +++ b/bitbucket/repository.py @@ -58,6 +58,8 @@ def public(self, username=None): return (response[0], response[1]['repositories']) except TypeError: pass + except KeyError: + pass return response def all(self): diff --git a/bitbucket/tests/public.py b/bitbucket/tests/public.py index 867f7cd..07bdeb6 100755 --- a/bitbucket/tests/public.py +++ b/bitbucket/tests/public.py @@ -114,7 +114,9 @@ def test_get_none_user(self): self.bb.username = None success, result = self.bb.get_user() self.assertFalse(success) - self.assertEqual(result, 'Service not found.') + self.assertEqual(result['message'], 'Service not found') + self.assertEqual(result['reason'], 'NOT FOUND') + self.assertEqual(result['code'], 404) def test_get_public_repos(self): """ Test public_repos on specific user.""" @@ -134,7 +136,9 @@ def test_get_none_public_repos(self): self.bb.username = None success, result = self.bb.repository.public() self.assertFalse(success) - self.assertEqual(result, 'Service not found.') + self.assertEqual(result['message'], 'Service not found') + self.assertEqual(result['reason'], 'NOT FOUND') + self.assertEqual(result['code'], 404) if __name__ == "__main__": unittest.main()