From 01d74bc7145e1e2a35bb082334d278ab99068326 Mon Sep 17 00:00:00 2001 From: Tsvika Shapira Date: Thu, 27 Feb 2025 16:57:53 +0200 Subject: [PATCH 1/8] =?UTF-8?q?=E2=9C=A8=20Add=20TaskError=20exception=20f?= =?UTF-8?q?or=20subprocess=20management?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce the TaskError exception to handle subprocess failures gracefully. Modify `Worker` class in `copier/main.py` to use TaskError when a subprocess encounters a non-zero exit status. Update tests to expect TaskError instead of CalledProcessError. --- copier/errors.py | 35 ++++++++++++++++++++++++++++++++++- copier/main.py | 5 ++++- tests/test_cleanup.py | 7 +++---- 3 files changed, 41 insertions(+), 6 deletions(-) diff --git a/copier/errors.py b/copier/errors.py index 01e6f1ad0..8d6a2b8c7 100644 --- a/copier/errors.py +++ b/copier/errors.py @@ -1,7 +1,10 @@ """Custom exceptions used by Copier.""" +from __future__ import annotations + from pathlib import Path -from typing import TYPE_CHECKING, Sequence +from subprocess import CompletedProcess +from typing import TYPE_CHECKING, Self, Sequence from .tools import printf_exception from .types import PathSeq @@ -120,6 +123,36 @@ class MultipleYieldTagsError(CopierError): """Multiple yield tags are used in one path name, but it is not allowed.""" +class TaskError(CopierError): + """Exception raised when a task fails.""" + + def __init__( + self, + task_command: str | Sequence[str], + returncode: int, + stdout: str | bytes | None, + stderr: str | bytes | None, + ): + self.task_command = task_command + self.returncode = returncode + self.stdout = stdout + self.stderr = stderr + message = f"Task {task_command!r} returned non-zero exit status {returncode}." + super().__init__(message) + + @classmethod + def from_process( + cls, process: CompletedProcess[str] | CompletedProcess[bytes] + ) -> Self: + """Create a TaskError from a CompletedProcess.""" + return cls( + task_command=process.args, + returncode=process.returncode, + stdout=process.stdout, + stderr=process.stderr, + ) + + # Warnings class CopierWarning(Warning): """Base class for all other Copier warnings.""" diff --git a/copier/main.py b/copier/main.py index 84dc32bf9..051c39df8 100644 --- a/copier/main.py +++ b/copier/main.py @@ -41,6 +41,7 @@ from .errors import ( CopierAnswersInterrupt, ExtensionNotFoundError, + TaskError, UnsafeTemplateError, UserMessageError, YieldTagInFileError, @@ -366,7 +367,9 @@ def _execute_tasks(self, tasks: Sequence[Task]) -> None: extra_env = {k.upper(): str(v) for k, v in task.extra_vars.items()} with local.cwd(working_directory), local.env(**extra_env): - subprocess.run(task_cmd, shell=use_shell, check=True, env=local.env) + process = subprocess.run(task_cmd, shell=use_shell, env=local.env) + if process.returncode: + raise TaskError.from_process(process) def _system_render_context(self) -> AnyByStrMutableMapping: """System reserved render context. diff --git a/tests/test_cleanup.py b/tests/test_cleanup.py index b08eeed44..4cc30ac61 100644 --- a/tests/test_cleanup.py +++ b/tests/test_cleanup.py @@ -1,5 +1,4 @@ from pathlib import Path -from subprocess import CalledProcessError import pytest from plumbum import local @@ -10,7 +9,7 @@ def test_cleanup(tmp_path: Path) -> None: """Copier creates dst_path, fails to copy and removes it.""" dst = tmp_path / "new_folder" - with pytest.raises(CalledProcessError): + with pytest.raises(copier.errors.TaskError): copier.run_copy("./tests/demo_cleanup", dst, quiet=True, unsafe=True) assert not dst.exists() @@ -18,7 +17,7 @@ def test_cleanup(tmp_path: Path) -> None: def test_do_not_cleanup(tmp_path: Path) -> None: """Copier creates dst_path, fails to copy and keeps it.""" dst = tmp_path / "new_folder" - with pytest.raises(CalledProcessError): + with pytest.raises(copier.errors.TaskError): copier.run_copy( "./tests/demo_cleanup", dst, quiet=True, unsafe=True, cleanup_on_error=False ) @@ -29,7 +28,7 @@ def test_no_cleanup_when_folder_existed(tmp_path: Path) -> None: """Copier will not delete a folder if it didn't create it.""" preexisting_file = tmp_path / "something" preexisting_file.touch() - with pytest.raises(CalledProcessError): + with pytest.raises(copier.errors.TaskError): copier.run_copy( "./tests/demo_cleanup", tmp_path, From 37e6bf63e233b0914b71d6d4e6020ebab4c8cf26 Mon Sep 17 00:00:00 2001 From: Tsvika Shapira Date: Thu, 27 Feb 2025 19:23:31 +0200 Subject: [PATCH 2/8] replace task_command with command --- copier/errors.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/copier/errors.py b/copier/errors.py index 8d6a2b8c7..7efdacb43 100644 --- a/copier/errors.py +++ b/copier/errors.py @@ -128,16 +128,16 @@ class TaskError(CopierError): def __init__( self, - task_command: str | Sequence[str], + command: str | Sequence[str], returncode: int, stdout: str | bytes | None, stderr: str | bytes | None, ): - self.task_command = task_command + self.command = command self.returncode = returncode self.stdout = stdout self.stderr = stderr - message = f"Task {task_command!r} returned non-zero exit status {returncode}." + message = f"Task {command!r} returned non-zero exit status {returncode}." super().__init__(message) @classmethod @@ -146,7 +146,7 @@ def from_process( ) -> Self: """Create a TaskError from a CompletedProcess.""" return cls( - task_command=process.args, + command=process.args, returncode=process.returncode, stdout=process.stdout, stderr=process.stderr, From 48b5cb2c0d8e47861d8288f2b565fb244e340381 Mon Sep 17 00:00:00 2001 From: Tsvika Shapira Date: Thu, 27 Feb 2025 19:35:33 +0200 Subject: [PATCH 3/8] output the error code to the screen when using CLI --- copier/errors.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/copier/errors.py b/copier/errors.py index 7efdacb43..00d61220e 100644 --- a/copier/errors.py +++ b/copier/errors.py @@ -123,7 +123,7 @@ class MultipleYieldTagsError(CopierError): """Multiple yield tags are used in one path name, but it is not allowed.""" -class TaskError(CopierError): +class TaskError(UserMessageError): """Exception raised when a task fails.""" def __init__( From a79ac5b262b22b621306014304f5750fa5b0e9fe Mon Sep 17 00:00:00 2001 From: Tsvika Shapira Date: Sun, 2 Mar 2025 16:20:21 +0200 Subject: [PATCH 4/8] Refactor UserMessageError to use a specific message attribute Checked for all uses of UserMessageError to verify that it'll work. Also added __str__ method to UserMessageError. --- copier/cli.py | 2 +- copier/errors.py | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/copier/cli.py b/copier/cli.py index 47532c12d..41cc549ae 100644 --- a/copier/cli.py +++ b/copier/cli.py @@ -71,7 +71,7 @@ def _handle_exceptions(method: Callable[[], None]) -> int: except KeyboardInterrupt: raise UserMessageError("Execution stopped by user") except UserMessageError as error: - print(colors.red | "\n".join(error.args), file=sys.stderr) + print(colors.red | error.message, file=sys.stderr) return 1 except UnsafeTemplateError as error: print(colors.red | "\n".join(error.args), file=sys.stderr) diff --git a/copier/errors.py b/copier/errors.py index 00d61220e..aeb75c9a5 100644 --- a/copier/errors.py +++ b/copier/errors.py @@ -22,6 +22,12 @@ class CopierError(Exception): class UserMessageError(CopierError): """Exit the program giving a message to the user.""" + def __init__(self, message: str): + self.message = message + + def __str__(self) -> str: + return self.message + class UnsupportedVersionError(UserMessageError): """Copier version does not support template version.""" From 5285657da2be673baf25caaa21e04614e2d40932 Mon Sep 17 00:00:00 2001 From: Tsvika Shapira Date: Sun, 2 Mar 2025 16:24:55 +0200 Subject: [PATCH 5/8] Improve TaskError compatibility by subclassing CalledProcessError --- copier/errors.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/copier/errors.py b/copier/errors.py index aeb75c9a5..e02c32824 100644 --- a/copier/errors.py +++ b/copier/errors.py @@ -2,6 +2,7 @@ from __future__ import annotations +import subprocess from pathlib import Path from subprocess import CompletedProcess from typing import TYPE_CHECKING, Self, Sequence @@ -129,7 +130,7 @@ class MultipleYieldTagsError(CopierError): """Multiple yield tags are used in one path name, but it is not allowed.""" -class TaskError(UserMessageError): +class TaskError(UserMessageError, subprocess.CalledProcessError): """Exception raised when a task fails.""" def __init__( @@ -139,12 +140,13 @@ def __init__( stdout: str | bytes | None, stderr: str | bytes | None, ): - self.command = command - self.returncode = returncode - self.stdout = stdout - self.stderr = stderr - message = f"Task {command!r} returned non-zero exit status {returncode}." - super().__init__(message) + subprocess.CalledProcessError.__init__( + self, returncode=returncode, cmd=command, output=stdout, stderr=stderr + ) + UserMessageError.__init__( + self, + message=f"Task {command!r} returned non-zero exit status {returncode}.", + ) @classmethod def from_process( From 1fe62fff1f8c5f2251803c72295d44c7747315ff Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Sun, 2 Mar 2025 14:47:01 +0000 Subject: [PATCH 6/8] style: autoformat with pre-commit --- copier/errors.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/copier/errors.py b/copier/errors.py index e02c32824..b166763ff 100644 --- a/copier/errors.py +++ b/copier/errors.py @@ -50,7 +50,7 @@ def __init__(self, conf_path: Path, quiet: bool): class MultipleConfigFilesError(ConfigFileError): """Both copier.yml and copier.yaml found, and that's an error.""" - def __init__(self, conf_paths: "PathSeq"): + def __init__(self, conf_paths: PathSeq): msg = str(conf_paths) printf_exception(self, "MULTIPLE CONFIG FILES", msg=msg) super().__init__(msg) @@ -103,7 +103,7 @@ class CopierAnswersInterrupt(CopierError, KeyboardInterrupt): """ def __init__( - self, answers: "AnswersMap", last_question: "Question", template: "Template" + self, answers: AnswersMap, last_question: Question, template: Template ) -> None: self.answers = answers self.last_question = last_question From f7ee96079cb7e08964d238042eaf5421d7444e04 Mon Sep 17 00:00:00 2001 From: Tsvika Shapira Date: Sun, 2 Mar 2025 17:26:17 +0200 Subject: [PATCH 7/8] Update `Self` import to be version-compatible --- copier/errors.py | 8 +++++++- poetry.lock | 2 +- pyproject.toml | 2 +- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/copier/errors.py b/copier/errors.py index b166763ff..66b064864 100644 --- a/copier/errors.py +++ b/copier/errors.py @@ -3,9 +3,10 @@ from __future__ import annotations import subprocess +import sys from pathlib import Path from subprocess import CompletedProcess -from typing import TYPE_CHECKING, Self, Sequence +from typing import TYPE_CHECKING, Sequence from .tools import printf_exception from .types import PathSeq @@ -14,6 +15,11 @@ from .template import Template from .user_data import AnswersMap, Question +if sys.version_info < (3, 11): + from typing_extensions import Self +else: + from typing import Self + # Errors class CopierError(Exception): diff --git a/poetry.lock b/poetry.lock index be8dc4cb3..78f1cf7b0 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1737,4 +1737,4 @@ type = ["pytest-mypy"] [metadata] lock-version = "2.0" python-versions = ">=3.9" -content-hash = "8778772793661c235a48d4ab2de8839426451f751ca66a79387790f5067c2794" +content-hash = "b871a6f90070af08db2af9a9303eb7dde034eef4363d26c67ca8f08be2be29b6" diff --git a/pyproject.toml b/pyproject.toml index 32c1cc976..b65895fe0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -42,6 +42,7 @@ pyyaml = ">=5.3.1" questionary = ">=1.8.1" eval-type-backport = { version = ">=0.1.3,<0.3.0", python = "<3.10" } platformdirs = ">=4.3.6" +typing-extensions = { version = ">=4.0.0,<5.0.0", python = "<3.11" } [tool.poetry.group.dev] optional = true @@ -59,7 +60,6 @@ types-backports = ">=0.1.3" types-colorama = ">=0.4" types-pygments = ">=2.17" types-pyyaml = ">=6.0.4" -typing-extensions = { version = ">=3.10.0.0,<5.0.0", python = "<3.10" } [tool.poetry.group.docs] optional = true From 70daeb1a44c4e4da622d6e713702dd714d052b2f Mon Sep 17 00:00:00 2001 From: Tsvika Shapira Date: Sun, 2 Mar 2025 18:40:55 +0200 Subject: [PATCH 8/8] switch order of inherited classes --- copier/errors.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/copier/errors.py b/copier/errors.py index 66b064864..68c5b120b 100644 --- a/copier/errors.py +++ b/copier/errors.py @@ -136,7 +136,7 @@ class MultipleYieldTagsError(CopierError): """Multiple yield tags are used in one path name, but it is not allowed.""" -class TaskError(UserMessageError, subprocess.CalledProcessError): +class TaskError(subprocess.CalledProcessError, UserMessageError): """Exception raised when a task fails.""" def __init__(