diff --git a/lib/cli/src/crewai_cli/deploy/archive.py b/lib/cli/src/crewai_cli/deploy/archive.py index 38b733fe4..760058003 100644 --- a/lib/cli/src/crewai_cli/deploy/archive.py +++ b/lib/cli/src/crewai_cli/deploy/archive.py @@ -36,6 +36,15 @@ _EXCLUDED_SUFFIXES = { } +class ArchiveError(ValueError): + """The project ZIP could not be built. + + A ``ValueError`` so existing callers keep working; a distinct type so the + deploy command can tell an archive failure from the git helpers' own + ``ValueError``s when it classifies why a create failed. + """ + + def create_project_zip( project_name: str, *, @@ -46,7 +55,7 @@ def create_project_zip( root = (project_dir or Path.cwd()).resolve() files = _project_files(root, repository) if not files: - raise ValueError("No deployable project files were found.") + raise ArchiveError("No deployable project files were found.") staged_root = _stage_project(root, files) archive_handle = tempfile.NamedTemporaryFile( @@ -62,6 +71,9 @@ def create_project_zip( for relative_path in _walk_files(staged_root): absolute_path = staged_root / relative_path zip_file.write(absolute_path, relative_path.as_posix()) + except (OSError, zipfile.BadZipFile) as exc: + archive_path.unlink(missing_ok=True) + raise ArchiveError(f"Could not build the project ZIP: {exc}") from exc finally: shutil.rmtree(staged_root, ignore_errors=True) diff --git a/lib/cli/src/crewai_cli/deploy/main.py b/lib/cli/src/crewai_cli/deploy/main.py index 6806b50d6..668393fb3 100644 --- a/lib/cli/src/crewai_cli/deploy/main.py +++ b/lib/cli/src/crewai_cli/deploy/main.py @@ -4,7 +4,6 @@ import subprocess from typing import Any from urllib.parse import quote import webbrowser -import zipfile from crewai_core.plus_api import CreateCrewPayload from crewai_core.telemetry import DeployFailureReason, DeploySource @@ -14,7 +13,7 @@ from rich.console import Console from crewai_cli import git from crewai_cli.command import BaseCommand, PlusAPIMixin from crewai_cli.constants import DEFAULT_CREWAI_ENTERPRISE_URL -from crewai_cli.deploy.archive import create_project_zip +from crewai_cli.deploy.archive import ArchiveError, create_project_zip from crewai_cli.deploy.validate import DeployValidator, Severity, render_report from crewai_cli.utils import fetch_and_json_env_file, get_project_name @@ -132,31 +131,40 @@ def _deployment_page_url(base_url: str, json_response: dict[str, Any]) -> str | def _creation_failure_reason(exc: BaseException) -> DeployFailureReason: """Classify an exception raised while requesting a deployment, for telemetry. - Only the archive step raises ``ValueError`` / ``OSError`` inside that request - (the project name is validated at construction), so those read as zip errors. + Archive failures are recognised by type (``ArchiveError``), not by base + class: the git helpers raise plain ``ValueError`` too, and those are not + ZIP problems. """ if isinstance(exc, (KeyboardInterrupt, EOFError)): return "user_declined" if isinstance(exc, httpx.HTTPError): return "network_error" - if isinstance(exc, (ValueError, OSError, zipfile.BadZipFile)): + if isinstance(exc, ArchiveError): return "zip_error" return "unexpected" def _response_failure_reason(response: httpx.Response) -> DeployFailureReason | None: - """Classify a create response that ``_validate_response`` will reject. + """Classify a create response that cannot become a created deployment. - Mirrors its checks in the same order; ``None`` means the response will pass. + Status first, so a gateway's HTML error page counts as the API class it is; + then the body, which must be a JSON object carrying ``uuid`` and ``status`` + for the success path to use. ``None`` means the response is a creation. """ - try: - response.json() - except (json.JSONDecodeError, ValueError): - return "invalid_response" if response.status_code >= 500: return "api_5xx" if not response.is_success: return "api_4xx" + try: + payload = response.json() + except (json.JSONDecodeError, ValueError): + return "invalid_response" + if ( + not isinstance(payload, dict) + or not payload.get("uuid") + or "status" not in payload + ): + return "invalid_response" return None @@ -492,15 +500,22 @@ class DeployCommand(BaseCommand, PlusAPIMixin): self._telemetry.crew_deployment_failed_span( failure_reason, source=source, status_code=response.status_code ) - self._validate_response(response) + # Prints the API's own details and exits for every non-2xx and for + # a body that is not JSON. Only a 2xx that parsed but is not a + # creation payload gets past it. + self._validate_response(response) + console.print( + "Unexpected response from the Enterprise API: no deployment uuid was returned.", + style="bold red", + ) + raise SystemExit(1) + json_response = response.json() - # After _validate_response, not before: it raises SystemExit on a failed - # create, so the span cannot fire for a deployment that was not made. This - # is the first point at which the uuid exists -- the pre-flight span at the - # top of this method counts the attempt and cannot carry it. - created_uuid = json_response.get("uuid") + # Only here, after the response has been classified as a creation: this + # is the first point at which the uuid exists -- the pre-flight span at + # the top of this method counts the attempt and cannot carry it. self._telemetry.crew_deployment_created_span( - uuid=str(created_uuid) if created_uuid else None, source=source + uuid=str(json_response["uuid"]), source=source ) self._display_creation_success(json_response) diff --git a/lib/cli/tests/deploy/test_archive.py b/lib/cli/tests/deploy/test_archive.py index 2dd2336c6..38327624a 100644 --- a/lib/cli/tests/deploy/test_archive.py +++ b/lib/cli/tests/deploy/test_archive.py @@ -4,7 +4,7 @@ import zipfile import pytest -from crewai_cli.deploy.archive import create_project_zip +from crewai_cli.deploy.archive import ArchiveError, create_project_zip def test_create_project_zip_excludes_local_artifacts(tmp_path: Path): @@ -305,3 +305,31 @@ type = "crew" assert "run_crew" not in pyproject assert "json_crew =" not in pyproject assert "[project.scripts]" not in pyproject + + +def test_create_project_zip_with_nothing_to_deploy_raises_archive_error( + tmp_path: Path, +): + """Still a ValueError for existing callers, and a distinct type for the deploy command.""" + with pytest.raises(ArchiveError, match="No deployable project files were found"): + create_project_zip("demo", project_dir=tmp_path) + assert issubclass(ArchiveError, ValueError) + + +def test_create_project_zip_wraps_a_write_failure_and_removes_the_partial_file( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +): + (tmp_path / "pyproject.toml").write_text("[project]\nname = 'demo'\n") + (tmp_path / "uv.lock").write_text("# lock\n") + created: list[Path] = [] + + def failing_zipfile(path, *args, **kwargs): + created.append(Path(path)) + raise OSError("disk full") + + monkeypatch.setattr("crewai_cli.deploy.archive.zipfile.ZipFile", failing_zipfile) + + with pytest.raises(ArchiveError, match="Could not build the project ZIP: disk full"): + create_project_zip("demo", project_dir=tmp_path) + + assert created and not created[0].exists() diff --git a/lib/cli/tests/deploy/test_deploy_main.py b/lib/cli/tests/deploy/test_deploy_main.py index 142fe7d6d..80d052434 100644 --- a/lib/cli/tests/deploy/test_deploy_main.py +++ b/lib/cli/tests/deploy/test_deploy_main.py @@ -9,6 +9,7 @@ import pytest import json import crewai_cli.deploy.main as deploy_main +from crewai_cli.deploy.archive import ArchiveError import httpx from crewai_cli.deploy.validate import Severity, ValidationResult from crewai_cli.utils import parse_toml @@ -942,13 +943,13 @@ class TestDeployCommand(unittest.TestCase): initialized_repository = MagicMock() initialized_repository.origin_url.return_value = None mock_repository.initialize.return_value = initialized_repository - mock_create_project_zip.side_effect = ValueError( + mock_create_project_zip.side_effect = ArchiveError( "No deployable project files were found." ) with patch.object(self.deploy_command, "_telemetry") as telemetry: with patch("sys.stdout", new=StringIO()): - with pytest.raises(ValueError, match="No deployable project files"): + with pytest.raises(ArchiveError, match="No deployable project files"): self.deploy_command.create_crew(skip_validate=True, confirm=True) telemetry.crew_deployment_failed_span.assert_called_once_with( @@ -956,6 +957,73 @@ class TestDeployCommand(unittest.TestCase): ) self.mock_client.create_crew_from_zip.assert_not_called() + @patch("crewai_cli.deploy.main.fetch_and_json_env_file") + @patch("crewai_cli.deploy.main.git.Repository") + @patch("builtins.input") + def test_a_git_helper_error_is_not_a_zip_error( + self, mock_input, mock_repository, mock_fetch_env + ): + """The git helpers raise plain ValueError; only ArchiveError is a ZIP problem.""" + self._git_path(mock_input, mock_repository, mock_fetch_env) + mock_repository.return_value.origin_url.side_effect = ValueError( + "Git remote lookup failed" + ) + + with patch.object(self.deploy_command, "_telemetry") as telemetry: + with patch("sys.stdout", new=StringIO()): + with pytest.raises(ValueError, match="Git remote lookup failed"): + self.deploy_command.create_crew(skip_validate=True) + + telemetry.crew_deployment_failed_span.assert_called_once_with( + "unexpected", source="cli" + ) + + @patch("crewai_cli.deploy.main.fetch_and_json_env_file") + @patch("crewai_cli.deploy.main.git.Repository") + @patch("builtins.input") + def test_a_non_json_error_page_keeps_its_api_class( + self, mock_input, mock_repository, mock_fetch_env + ): + """A gateway's HTML 502 is an api_5xx, not an invalid response.""" + self._git_path(mock_input, mock_repository, mock_fetch_env) + self.mock_client.create_crew.return_value = self._api_response( + 502, ValueError("not json") + ) + + with patch.object(self.deploy_command, "_telemetry") as telemetry: + with patch("sys.stdout", new=StringIO()): + with self.assertRaises(SystemExit): + self.deploy_command.create_crew(skip_validate=True) + + telemetry.crew_deployment_failed_span.assert_called_once_with( + "api_5xx", source="cli", status_code=502 + ) + + @patch("crewai_cli.deploy.main.fetch_and_json_env_file") + @patch("crewai_cli.deploy.main.git.Repository") + @patch("builtins.input") + def test_a_success_body_that_is_not_a_creation_reports_invalid_response( + self, mock_input, mock_repository, mock_fetch_env + ): + """A 2xx without a deployment uuid is not a success and must not crash.""" + self._git_path(mock_input, mock_repository, mock_fetch_env) + for body in ([{"uuid": "in-a-list"}], {"status": "created"}, {}): + with self.subTest(body=body): + self.mock_client.create_crew.return_value = self._api_response( + 200, body + ) + with patch.object(self.deploy_command, "_telemetry") as telemetry: + with patch("sys.stdout", new=StringIO()) as fake_out: + with self.assertRaises(SystemExit) as exit_info: + self.deploy_command.create_crew(skip_validate=True) + + assert exit_info.exception.code == 1 + assert "no deployment uuid was returned" in fake_out.getvalue() + telemetry.crew_deployment_failed_span.assert_called_once_with( + "invalid_response", source="cli", status_code=200 + ) + telemetry.crew_deployment_created_span.assert_not_called() + @patch("crewai_cli.deploy.main.fetch_and_json_env_file") @patch("crewai_cli.deploy.main.git.Repository") @patch("builtins.input")