From 3626f06bcb65b4157e9900da012fcd1d5c3c5d04 Mon Sep 17 00:00:00 2001 From: fran-tirapu Date: Wed, 6 Nov 2024 14:34:52 +0100 Subject: [PATCH 1/3] altool retry by return codes -5 and -11 --- CHANGELOG.md | 7 +++++++ pyproject.toml | 2 +- src/codemagic/__version__.py | 2 +- src/codemagic/models/altool/altool.py | 14 +++++++++----- tests/models/altool/test_altool_retrying.py | 19 +++++++++++++++++++ 5 files changed, 37 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d91a9544..078487aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,10 @@ +Version 0.54.2 +------------- + +**Bugfixes** +- Introduce a new retrying condition for `altool` commands as part of `app-store-connect` action when unknown return codes occurs. + + Version 0.54.1 ------------- diff --git a/pyproject.toml b/pyproject.toml index b5163937..94abc9b8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "codemagic-cli-tools" -version = "0.54.1" +version = "0.54.2" description = "CLI tools used in Codemagic builds" readme = "README.md" authors = [ diff --git a/src/codemagic/__version__.py b/src/codemagic/__version__.py index 631b097c..0deb7a52 100644 --- a/src/codemagic/__version__.py +++ b/src/codemagic/__version__.py @@ -1,5 +1,5 @@ __title__ = "codemagic-cli-tools" __description__ = "CLI tools used in Codemagic builds" -__version__ = "0.54.1.dev" +__version__ = "0.54.2.dev" __url__ = "https://github.com/codemagic-ci-cd/cli-tools" __licence__ = "GNU General Public License v3.0" diff --git a/src/codemagic/models/altool/altool.py b/src/codemagic/models/altool/altool.py index b6b43352..43deccc2 100644 --- a/src/codemagic/models/altool/altool.py +++ b/src/codemagic/models/altool/altool.py @@ -39,9 +39,10 @@ class AltoolCommandError(Exception): - def __init__(self, error_message: str, process_output: str): + def __init__(self, error_message: str, process_output: str, return_code: int = 1): super().__init__(error_message) self.process_output = process_output + self.return_code = return_code class Altool(RunningCliAppMixin, StringConverterMixin): @@ -196,7 +197,7 @@ def _run_retrying_command( return self._run_command(command, f'Failed to {action_name} archive at "{artifact_path}"', cli_app) except AltoolCommandError as error: has_retries = retries > 0 - should_retry = self._should_retry_command(error.process_output) + should_retry = self._should_retry_command(error) if has_retries and should_retry: if attempt == 1: print_fn(f"Failed to {action_name} archive, but this might be a temporary issue, retrying...") @@ -258,6 +259,7 @@ def _run_command( raise AltoolCommandError( error_message, self._hide_environment_variable_values(cpe.stdout), + cpe.returncode, ) self._log_process_output(stdout, cli_app) @@ -286,14 +288,16 @@ def _hide_environment_variable_values(cls, altool_output: Optional[AnyStr]) -> s return output - @classmethod - def _should_retry_command(cls, process_output: str): + def _should_retry_command(self, command_error: AltoolCommandError) -> bool: + if command_error.return_code in [-5, -11]: + self.logger.info(f"Unknown altool exit code {command_error.return_code}, retrying...") + return True patterns = ( re.compile("Unable to authenticate.*-19209"), re.compile("server returned an invalid response.*try your request again"), re.compile("The request timed out."), ) - return any(pattern.search(process_output) for pattern in patterns) + return any(pattern.search(command_error.process_output) for pattern in patterns) @classmethod def _get_action_result(cls, action_stdout: AnyStr) -> Optional[AltoolResult]: diff --git a/tests/models/altool/test_altool_retrying.py b/tests/models/altool/test_altool_retrying.py index 7c5a606b..013051a5 100644 --- a/tests/models/altool/test_altool_retrying.py +++ b/tests/models/altool/test_altool_retrying.py @@ -140,6 +140,25 @@ def test_retrying_command_success(mock_altool, mock_auth_error_stdout, mock_succ assert mock_kill_xcodes.call_count == 2 +@mock.patch.object(PlatformType, "from_path", lambda _artifact_path: PlatformType.IOS) +def test_retrying_command_by_return_code_and_success(caplog, mock_altool, mock_success_result): + raise_errors = mock.Mock( + side_effect=( + AltoolCommandError("my error", "process output", -11), + AltoolCommandError("my error", "process output", -5), + AltoolCommandError("my error", "process output", -5), + mock_success_result, + ), + ) + + with mock.patch.object(mock_altool, "_run_command", side_effect=raise_errors): + result = mock_altool.upload_app(pathlib.Path("app.ipa"), retries=4, retry_wait_seconds=0) + + assert result is mock_success_result + assert caplog.text.count("Unknown altool exit code -11, retrying...") == 1 + assert caplog.text.count("Unknown altool exit code -5, retrying...") == 2 + + @mock.patch.object(PlatformType, "from_path", lambda _artifact_path: PlatformType.IOS) def test_retrying_command_immediate_success(mock_altool, mock_success_stdout, mock_success_result): with mock.patch.object(mock_altool, "_run_command", side_effect=[mock_success_result]): From 2d71b49c07cf75e94ec84e2fa3673faf396f1c48 Mon Sep 17 00:00:00 2001 From: fran-tirapu Date: Wed, 6 Nov 2024 14:44:54 +0100 Subject: [PATCH 2/3] update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 078487aa..f6e32c45 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ Version 0.54.2 ------------- **Bugfixes** -- Introduce a new retrying condition for `altool` commands as part of `app-store-connect` action when unknown return codes occurs. +- Introduce a new retrying condition for `altool` commands as part of `app-store-connect` action when unknown return codes occurs. [PR #435](https://github.com/codemagic-ci-cd/cli-tools/pull/435) Version 0.54.1 From 0e2de608eb70503ac5abc92f653b60ed28f23fdd Mon Sep 17 00:00:00 2001 From: fran-tirapu Date: Wed, 6 Nov 2024 15:37:15 +0100 Subject: [PATCH 3/3] refactor to Unexpected --- CHANGELOG.md | 2 +- src/codemagic/models/altool/altool.py | 2 +- tests/models/altool/test_altool_retrying.py | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f6e32c45..40d0e539 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ Version 0.54.2 ------------- **Bugfixes** -- Introduce a new retrying condition for `altool` commands as part of `app-store-connect` action when unknown return codes occurs. [PR #435](https://github.com/codemagic-ci-cd/cli-tools/pull/435) +- Introduce a new retrying condition for `altool` commands as part of `app-store-connect` action when unexpected return codes occurs. [PR #435](https://github.com/codemagic-ci-cd/cli-tools/pull/435) Version 0.54.1 diff --git a/src/codemagic/models/altool/altool.py b/src/codemagic/models/altool/altool.py index 43deccc2..2f9b2b27 100644 --- a/src/codemagic/models/altool/altool.py +++ b/src/codemagic/models/altool/altool.py @@ -290,7 +290,7 @@ def _hide_environment_variable_values(cls, altool_output: Optional[AnyStr]) -> s def _should_retry_command(self, command_error: AltoolCommandError) -> bool: if command_error.return_code in [-5, -11]: - self.logger.info(f"Unknown altool exit code {command_error.return_code}, retrying...") + self.logger.info(f"Unexpected altool exit code {command_error.return_code}, retrying...") return True patterns = ( re.compile("Unable to authenticate.*-19209"), diff --git a/tests/models/altool/test_altool_retrying.py b/tests/models/altool/test_altool_retrying.py index 013051a5..57d45ff6 100644 --- a/tests/models/altool/test_altool_retrying.py +++ b/tests/models/altool/test_altool_retrying.py @@ -155,8 +155,8 @@ def test_retrying_command_by_return_code_and_success(caplog, mock_altool, mock_s result = mock_altool.upload_app(pathlib.Path("app.ipa"), retries=4, retry_wait_seconds=0) assert result is mock_success_result - assert caplog.text.count("Unknown altool exit code -11, retrying...") == 1 - assert caplog.text.count("Unknown altool exit code -5, retrying...") == 2 + assert caplog.text.count("Unexpected altool exit code -11, retrying...") == 1 + assert caplog.text.count("Unexpected altool exit code -5, retrying...") == 2 @mock.patch.object(PlatformType, "from_path", lambda _artifact_path: PlatformType.IOS)