diff --git a/ntk/__main__.py b/ntk/__main__.py index ec73b1d..5da23e2 100644 --- a/ntk/__main__.py +++ b/ntk/__main__.py @@ -1,8 +1,10 @@ #!/usr/bin/env python import logging +import sys from requests.exceptions import HTTPError +from ntk.exceptions import NTKError from ntk.ntk_parser import Parser logging.basicConfig( @@ -19,6 +21,11 @@ def main(): args.func(args) except AttributeError: print('Use ntk -h or --help to see available commands') + except NTKError as e: + # print new line for support error on process progress bar + print() + logging.error(e) + sys.exit(1) except (TypeError, HTTPError) as e: # print new line for support error on process progress bar print() diff --git a/ntk/command.py b/ntk/command.py index 991f388..8a79690 100644 --- a/ntk/command.py +++ b/ntk/command.py @@ -12,6 +12,7 @@ SASS_EXTENSIONS, ) from ntk.decorator import parser_config +from ntk.exceptions import NTKError from ntk.gateway import Gateway from ntk.utils import get_template_name, progress_bar @@ -49,12 +50,16 @@ def _handle_files_change(self, changes): if not pathfile.endswith(valid_extensions): continue template_name = get_template_name(pathfile) - if event_type in [Change.added, Change.modified]: - logging.info(f'[{self.config.env}] {event_type.name.title()} {template_name}') - self._push_templates([template_name], compile_sass=True) - elif event_type == Change.deleted: - logging.info(f'[{self.config.env}] {event_type.name.title()} {template_name}') - self._delete_templates([template_name]) + try: + if event_type in [Change.added, Change.modified]: + logging.info(f'[{self.config.env}] {event_type.name.title()} {template_name}') + self._push_templates([template_name], compile_sass=True) + elif event_type == Change.deleted: + logging.info(f'[{self.config.env}] {event_type.name.title()} {template_name}') + self._delete_templates([template_name]) + except NTKError as error: + # Keep watching on a transient failure instead of killing the watcher. + logging.error(f'[{self.config.env}] {error}') def _push_templates(self, template_names, compile_sass=False): template_names = self._get_accept_files(template_names) diff --git a/ntk/decorator.py b/ntk/decorator.py index 711960a..a814e38 100644 --- a/ntk/decorator.py +++ b/ntk/decorator.py @@ -1,6 +1,8 @@ import functools import logging +from ntk.exceptions import NTKAuthError, NTKNotFoundError + logging.basicConfig( format='%(asctime)s %(levelname)s %(message)s', level=logging.INFO, @@ -35,6 +37,14 @@ def _decorator(func): @functools.wraps(func) def _wrapper(self, *func_args, **func_kwargs): response = func(self, *func_args, **func_kwargs) + + if response.status_code == 401: + raise NTKAuthError(f'Invalid API key for {self.store}.') + + if response.status_code == 404: + raise NTKNotFoundError( + f'Not found: {response.url} — check the store URL and theme id.') + error_default = f'{func.__name__.capitalize().replace("_", " ")} of {self.store} failed.' error_msg = "" content_type = response.headers.get('content-type', '').lower() diff --git a/ntk/exceptions.py b/ntk/exceptions.py new file mode 100644 index 0000000..ad67e43 --- /dev/null +++ b/ntk/exceptions.py @@ -0,0 +1,14 @@ +class NTKError(Exception): + """Base class for ntk errors surfaced to the CLI entry point.""" + + +class NTKAuthError(NTKError): + """Raised on a 401 response (invalid or missing API key).""" + + +class NTKNotFoundError(NTKError): + """Raised on a 404 response (store, theme, or template not found).""" + + +class NTKConnectionError(NTKError): + """Raised when a request cannot reach the store after retries.""" diff --git a/ntk/gateway.py b/ntk/gateway.py index 2edfdd3..44678ed 100644 --- a/ntk/gateway.py +++ b/ntk/gateway.py @@ -1,7 +1,20 @@ +import time import requests from urllib.parse import urljoin from ntk.decorator import check_error +from ntk.exceptions import NTKConnectionError + +MAX_RETRIES = 3 +RETRY_BACKOFF_SECONDS = 1 + +# Transport-level failures that are worth retrying rather than surfacing as a raw stack trace. +TRANSIENT_EXCEPTIONS = ( + requests.exceptions.ConnectionError, + requests.exceptions.Timeout, + requests.exceptions.ChunkedEncodingError, + requests.exceptions.SSLError, +) class Gateway: @@ -14,10 +27,21 @@ def _request(self, request_type, url, apikey=None, payload={}, files={}): if apikey: headers = {'Authorization': f'Bearer {apikey}'} - response = requests.request(request_type, url, headers=headers, data=payload, files=files) - if response.status_code == 429 and "throttled" in response.content.decode(): - return self._request(request_type, url, apikey, payload, files) - return response + for attempt in range(MAX_RETRIES): + try: + response = requests.request(request_type, url, headers=headers, data=payload, files=files) + except TRANSIENT_EXCEPTIONS: + if attempt == MAX_RETRIES - 1: + raise NTKConnectionError(f'Unable to reach {self.store} after {MAX_RETRIES} attempts.') + time.sleep(RETRY_BACKOFF_SECONDS * (2 ** attempt)) + continue + + throttled = response.status_code == 429 and "throttled" in response.content.decode() + if throttled and attempt < MAX_RETRIES - 1: + time.sleep(RETRY_BACKOFF_SECONDS * (2 ** attempt)) + continue + + return response @check_error(error_format='Missing Themes in {store}') def get_themes(self): diff --git a/tests/test_command.py b/tests/test_command.py index b6e5766..e7cf86a 100644 --- a/tests/test_command.py +++ b/tests/test_command.py @@ -487,6 +487,21 @@ def test_watch_command_with_create_image_file_should_call_gateway_with_correct_a ) self.assertIn(expected_call_added, self.mock_gateway.mock_calls) + @patch("ntk.command.Command._push_templates", autospec=True) + def test_watch_logs_and_continues_when_push_raises_ntk_error(self, mock_push_templates): + """A transient NTKError during watch should be logged, not kill the watcher.""" + from ntk.exceptions import NTKAuthError + mock_push_templates.side_effect = NTKAuthError('Invalid API key for http://development.com.') + self.command.config.parser_config(self.parser) + changes = [ + (Change.modified, './templates/index.html'), + ] + # Should not raise — the error is caught inside _handle_files_change. + with self.assertLogs(level='ERROR') as log: + self.command._handle_files_change(changes) + self.assertTrue( + any('Invalid API key for http://development.com.' in line for line in log.output)) + @patch("ntk.command.Command._get_accept_files", autospec=True) @patch("ntk.command.Command._compile_sass", autospec=True) def test_watch_command_with_sass_directory_should_call_compile_sass( diff --git a/tests/test_gateway.py b/tests/test_gateway.py index 0165d24..73faba6 100644 --- a/tests/test_gateway.py +++ b/tests/test_gateway.py @@ -1,7 +1,10 @@ import unittest from unittest.mock import call, MagicMock, patch -from ntk.gateway import Gateway +import requests + +from ntk.exceptions import NTKAuthError, NTKConnectionError, NTKNotFoundError +from ntk.gateway import Gateway, MAX_RETRIES class TestGateway(unittest.TestCase): @@ -42,8 +45,9 @@ def test_request(self, mock_request): ] assert mock_request.mock_calls == expected_calls + @patch('ntk.gateway.time.sleep', autospec=True) @patch('ntk.gateway.requests.request', autospec=True) - def test_request_with_rate_limit_should_retry(self, mock_request): + def test_request_with_rate_limit_should_retry(self, mock_request, mock_sleep): mock_response_429 = MagicMock() mock_response_429.status_code = 429 mock_response_429.content.decode.return_value = "throttled" @@ -82,6 +86,66 @@ def test_request_with_rate_limit_should_retry(self, mock_request): ] assert mock_request.mock_calls == expected_calls + @patch('ntk.gateway.time.sleep', autospec=True) + @patch('ntk.gateway.requests.request', autospec=True) + def test_request_rate_limit_gives_up_after_max_retries(self, mock_request, mock_sleep): + mock_response_429 = MagicMock() + mock_response_429.status_code = 429 + mock_response_429.content.decode.return_value = "throttled" + + mock_request.return_value = mock_response_429 + + response = self.gateway._request('GET', 'http://simple.com/api/admin/themes/', apikey=self.apikey) + + assert mock_request.call_count == MAX_RETRIES + assert response is mock_response_429 + + @patch('ntk.gateway.time.sleep', autospec=True) + @patch('ntk.gateway.requests.request', autospec=True) + def test_request_retries_transient_error_then_succeeds(self, mock_request, mock_sleep): + mock_response_200 = MagicMock() + mock_response_200.status_code = 200 + + mock_request.side_effect = [requests.exceptions.ConnectionError(), mock_response_200] + + response = self.gateway._request('GET', 'http://simple.com/api/admin/themes/', apikey=self.apikey) + + assert mock_request.call_count == 2 + assert response is mock_response_200 + + @patch('ntk.gateway.time.sleep', autospec=True) + @patch('ntk.gateway.requests.request', autospec=True) + def test_request_raises_after_persistent_transient_error(self, mock_request, mock_sleep): + # A Timeout is one of the broadened transient exceptions that should be retried, not surfaced raw. + mock_request.side_effect = requests.exceptions.Timeout() + + with self.assertRaises(NTKConnectionError): + self.gateway._request('GET', 'http://simple.com/api/admin/themes/', apikey=self.apikey) + + assert mock_request.call_count == MAX_RETRIES + + ##### + # 401 / 404 handling (check_error decorator) + ##### + @patch('ntk.gateway.requests.request', autospec=True) + def test_request_401_raises_auth_error(self, mock_request): + mock_request.return_value.status_code = 401 + + with self.assertRaises(NTKAuthError) as error: + self.gateway.get_themes() + + self.assertEqual(str(error.exception), 'Invalid API key for http://simple.com.') + + @patch('ntk.gateway.requests.request', autospec=True) + def test_request_404_raises_not_found_error_with_url(self, mock_request): + mock_request.return_value.status_code = 404 + mock_request.return_value.url = 'http://simple.com/api/admin/themes/6/templates/' + + with self.assertRaises(NTKNotFoundError) as error: + self.gateway.get_templates(theme_id=6) + + self.assertIn('http://simple.com/api/admin/themes/6/templates/', str(error.exception)) + ##### # get_themes ##### diff --git a/tests/test_main.py b/tests/test_main.py new file mode 100644 index 0000000..19b88e7 --- /dev/null +++ b/tests/test_main.py @@ -0,0 +1,34 @@ +import unittest +from unittest.mock import MagicMock, patch + +from ntk.__main__ import main +from ntk.exceptions import NTKNotFoundError + + +class TestMain(unittest.TestCase): + @patch('ntk.__main__.Parser', autospec=True) + def test_main_exits_1_on_ntk_error(self, mock_parser): + args = MagicMock() + args.func.side_effect = NTKNotFoundError( + 'Not found: http://simple.com/api/admin/themes/6/templates/ — check the store URL and theme id.') + mock_parser.return_value.create_parser.return_value.parse_args.return_value = args + + with self.assertRaises(SystemExit) as exit_context: + with self.assertLogs(level='ERROR') as log: + main() + + self.assertEqual(exit_context.exception.code, 1) + self.assertTrue(any('Not found' in line for line in log.output)) + + @patch('ntk.__main__.Parser', autospec=True) + def test_main_prints_help_hint_on_attribute_error(self, mock_parser): + args = MagicMock() + args.func.side_effect = AttributeError() + mock_parser.return_value.create_parser.return_value.parse_args.return_value = args + + # No command supplied — should print the help hint and not raise. + main() + + +if __name__ == '__main__': + unittest.main()