diff --git a/CHANGELOG.md b/CHANGELOG.md index 9befc927..9955a8fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +# 3.4.1 + * Retry transient 401 responses in `authed_get` by including `BadCredentialsException` in backoff retries. [#234](https://github.com/singer-io/tap-github/pull/234) + * Rationale: GitHub has documented intermittent API-side auth/routing incidents where valid requests can return temporary 401 and succeed on retry. + # 3.4.0 * Exclude 403-forbidden streams from discovery [#229](https://github.com/singer-io/tap-github/pull/229) * Bump dependencies for compliance diff --git a/setup.py b/setup.py index 7c5f0a51..9fa62e4b 100644 --- a/setup.py +++ b/setup.py @@ -3,7 +3,7 @@ from setuptools import setup, find_packages setup(name='tap-github', - version='3.4.0', + version='3.4.1', description='Singer.io tap for extracting data from the GitHub API', author='Stitch', url='http://singer.io', diff --git a/tap_github/client.py b/tap_github/client.py index 3198bbe6..07424ba4 100644 --- a/tap_github/client.py +++ b/tap_github/client.py @@ -196,7 +196,11 @@ def set_auth_in_session(self): # pylint: disable=dangerous-default-value # During 'Timeout' error there is also possibility of 'ConnectionError', # hence added backoff for 'ConnectionError' too. - @backoff.on_exception(backoff.expo, (requests.Timeout, requests.ConnectionError, Server5xxError, TooManyRequests), max_tries=5, factor=2) + # GitHub has documented intermittent transient 401 responses where retrying + # eventually succeeds during API-side routing incidents. Retrying + # BadCredentialsException helps discovery/sync survive these short-lived + # faults while still failing after max_tries for persistent invalid tokens. + @backoff.on_exception(backoff.expo, (requests.Timeout, requests.ConnectionError, Server5xxError, TooManyRequests, BadCredentialsException), max_tries=5, factor=2) def authed_get(self, source, url, headers={}, stream="", should_skip_404 = True): """ Call rest API and return the response in case of status code 200. diff --git a/tests/unittests/test_exception_handling.py b/tests/unittests/test_exception_handling.py index 51e1152d..714c4c58 100644 --- a/tests/unittests/test_exception_handling.py +++ b/tests/unittests/test_exception_handling.py @@ -63,7 +63,7 @@ def test_json_decoder_error(self, mocked_parse_args, mocked_request, mock_verify @parameterized.expand([ [400, "The request is missing or has a bad parameter.", BadRequestException, '', {}, 1], - [401, "Invalid authorization credentials.", BadCredentialsException, '', {}, 1], + [401, "Invalid authorization credentials.", BadCredentialsException, '', {}, 5], [403, "User doesn't have permission to access the resource.", AuthException, '', {}, 1], [500, "An error has occurred at Github's end.", InternalServerError, '', {}, 5], [301, "The resource you are looking for is moved to another URL.", tap_github.client.MovedPermanentlyError, '', {}, 1], @@ -76,7 +76,8 @@ def test_json_decoder_error(self, mocked_parse_args, mocked_request, mock_verify def test_error_message_and_call_count(self, mocked_parse_args, mocked_request, mock_verify_access, mock_sleep, erro_code, error_msg, error_class, content, json_msg, call_count): """ - Verify that `authed_get` raises an error with the proper message for different error codes. - - Verify that tap retries 5 times for Server5xxError and RateLimitExceeded error. + - Verify that tap retries 5 times for Server5xxError, TooManyRequests, + and transient 401 BadCredentialsException. """ mocked_request.return_value = get_response(erro_code, json = json_msg, raise_error = True, content = content) test_client = GithubClient(self.config)