diff --git a/Code/Tools/ProjectManager/Source/DownloadWorker.cpp b/Code/Tools/ProjectManager/Source/DownloadWorker.cpp index 560bfe05de..163e2c5869 100644 --- a/Code/Tools/ProjectManager/Source/DownloadWorker.cpp +++ b/Code/Tools/ProjectManager/Source/DownloadWorker.cpp @@ -33,7 +33,7 @@ namespace O3DE::ProjectManager } else { - emit Done(tr("Gem download failed")); + emit Done(gemInfoResult.GetError().c_str()); } } diff --git a/Code/Tools/ProjectManager/Source/GemRepo/GemRepoScreen.cpp b/Code/Tools/ProjectManager/Source/GemRepo/GemRepoScreen.cpp index 91432c2346..057976921e 100644 --- a/Code/Tools/ProjectManager/Source/GemRepo/GemRepoScreen.cpp +++ b/Code/Tools/ProjectManager/Source/GemRepo/GemRepoScreen.cpp @@ -92,8 +92,8 @@ namespace O3DE::ProjectManager return; } - bool addGemRepoResult = PythonBindingsInterface::Get()->AddGemRepo(repoUri); - if (addGemRepoResult) + AZ::Outcome addGemRepoResult = PythonBindingsInterface::Get()->AddGemRepo(repoUri); + if (addGemRepoResult.IsSuccess()) { Reinit(); emit OnRefresh(); @@ -101,8 +101,8 @@ namespace O3DE::ProjectManager else { QString failureMessage = tr("Failed to add gem repo: %1.").arg(repoUri); - QMessageBox::critical(this, tr("Operation failed"), failureMessage); - AZ_Error("Project Manger", false, failureMessage.toUtf8()); + QMessageBox::critical(this, failureMessage, addGemRepoResult.GetError().c_str()); + AZ_Error("Project Manager", false, failureMessage.toUtf8()); } } } diff --git a/Code/Tools/ProjectManager/Source/PythonBindings.cpp b/Code/Tools/ProjectManager/Source/PythonBindings.cpp index d1dedaaec4..c30e9ebd28 100644 --- a/Code/Tools/ProjectManager/Source/PythonBindings.cpp +++ b/Code/Tools/ProjectManager/Source/PythonBindings.cpp @@ -61,6 +61,7 @@ namespace Platform namespace RedirectOutput { using RedirectOutputFunc = AZStd::function; + AZStd::string lastPythonError; struct RedirectOutput { @@ -210,6 +211,16 @@ namespace RedirectOutput }); SetRedirection("stderr", g_redirect_stderr_saved, g_redirect_stderr, []([[maybe_unused]] const char* msg) { + if (lastPythonError.empty()) + { + lastPythonError = msg; + const int lengthOfErrorPrefix = 11; + auto errorPrefix = lastPythonError.find("ERROR:root:"); + if (errorPrefix != AZStd::string::npos) + { + lastPythonError.erase(errorPrefix, lengthOfErrorPrefix); + } + } AZ_TracePrintf("Python", msg); }); @@ -1051,12 +1062,13 @@ namespace O3DE::ProjectManager return result && refreshResult; } - bool PythonBindings::AddGemRepo(const QString& repoUri) + AZ::Outcome PythonBindings::AddGemRepo(const QString& repoUri) { bool registrationResult = false; bool result = ExecuteWithLock( [&] { + RedirectOutput::lastPythonError.clear(); auto pyUri = QString_To_Py_String(repoUri); auto pythonRegistrationResult = m_register.attr("register")( pybind11::none(), pybind11::none(), pybind11::none(), pybind11::none(), pybind11::none(), pybind11::none(), pyUri); @@ -1065,7 +1077,12 @@ namespace O3DE::ProjectManager registrationResult = !pythonRegistrationResult.cast(); }); - return result && registrationResult; + if (!result || !registrationResult) + { + return AZ::Failure(AZStd::move(RedirectOutput::lastPythonError)); + } + + return AZ::Success(); } bool PythonBindings::RemoveGemRepo(const QString& repoUri) @@ -1225,6 +1242,7 @@ namespace O3DE::ProjectManager auto result = ExecuteWithLockErrorHandling( [&] { + RedirectOutput::lastPythonError.clear(); auto downloadResult = m_download.attr("download_gem")( QString_To_Py_String(gemName), // gem name pybind11::none(), // destination path @@ -1248,7 +1266,7 @@ namespace O3DE::ProjectManager } else if (!downloadSucceeded) { - return AZ::Failure("Failed to download gem."); + return AZ::Failure(AZStd::move(RedirectOutput::lastPythonError)); } return AZ::Success(); diff --git a/Code/Tools/ProjectManager/Source/PythonBindings.h b/Code/Tools/ProjectManager/Source/PythonBindings.h index ecc6f65dc3..9f5f06110e 100644 --- a/Code/Tools/ProjectManager/Source/PythonBindings.h +++ b/Code/Tools/ProjectManager/Source/PythonBindings.h @@ -62,7 +62,7 @@ namespace O3DE::ProjectManager // Gem Repos AZ::Outcome RefreshGemRepo(const QString& repoUri) override; bool RefreshAllGemRepos() override; - bool AddGemRepo(const QString& repoUri) override; + AZ::Outcome AddGemRepo(const QString& repoUri) override; bool RemoveGemRepo(const QString& repoUri) override; AZ::Outcome, AZStd::string> GetAllGemRepoInfos() override; AZ::Outcome, AZStd::string> GetAllGemRepoGemsInfos() override; diff --git a/Code/Tools/ProjectManager/Source/PythonBindingsInterface.h b/Code/Tools/ProjectManager/Source/PythonBindingsInterface.h index 65337869fd..26a487f307 100644 --- a/Code/Tools/ProjectManager/Source/PythonBindingsInterface.h +++ b/Code/Tools/ProjectManager/Source/PythonBindingsInterface.h @@ -200,9 +200,9 @@ namespace O3DE::ProjectManager /** * Registers this gem repo with the current engine. * @param repoUri the absolute filesystem path or url to the gem repo. - * @return true on success, false on failure. + * @return an outcome with a string error message on failure. */ - virtual bool AddGemRepo(const QString& repoUri) = 0; + virtual AZ::Outcome AddGemRepo(const QString& repoUri) = 0; /** * Unregisters this gem repo with the current engine. diff --git a/scripts/o3de/o3de/register.py b/scripts/o3de/o3de/register.py index 8a2bb788aa..8420e202dc 100644 --- a/scripts/o3de/o3de/register.py +++ b/scripts/o3de/o3de/register.py @@ -490,7 +490,7 @@ def register_repo(json_data: dict, repo_sha256 = hashlib.sha256(url.encode()) cache_file = manifest.get_o3de_cache_folder() / str(repo_sha256.hexdigest() + '.json') - result = utils.download_file(parsed_uri, cache_file) + result = utils.download_file(parsed_uri, cache_file, True) if result == 0: json_data.setdefault('repos', []).insert(0, repo_uri) diff --git a/scripts/o3de/o3de/repo.py b/scripts/o3de/o3de/repo.py index c8fac38605..3ebb79fb27 100644 --- a/scripts/o3de/o3de/repo.py +++ b/scripts/o3de/o3de/repo.py @@ -24,6 +24,7 @@ def process_add_o3de_repo(file_name: str or pathlib.Path, repo_set: set) -> int: file_name = pathlib.Path(file_name).resolve() if not validation.valid_o3de_repo_json(file_name): + logger.error(f'Repository JSON {file_name} could not be loaded or is missing required values') return 1 cache_folder = manifest.get_o3de_cache_folder() @@ -114,7 +115,7 @@ def get_gem_json_paths_from_cached_repo(repo_uri: str) -> set: file_name = pathlib.Path(cache_filename).resolve() if not file_name.is_file(): - logger.error(f'Could not find cached repo json file for {repo_uri}') + logger.error(f'Could not find cached repository json file for {repo_uri}. Try refreshing the repository.') return gem_set with file_name.open('r') as f: @@ -167,6 +168,7 @@ def refresh_repo(repo_uri: str, download_file_result = utils.download_file(parsed_uri, cache_file, True) if download_file_result != 0: + logger.error(f'Repo json {repo_uri} could not download.') return download_file_result if not validation.valid_o3de_repo_json(cache_file): diff --git a/scripts/o3de/o3de/utils.py b/scripts/o3de/o3de/utils.py index 88f84ae75e..7b7d0e3e27 100644 --- a/scripts/o3de/o3de/utils.py +++ b/scripts/o3de/o3de/utils.py @@ -125,7 +125,8 @@ def download_file(parsed_uri, download_path: pathlib.Path, force_overwrite: bool """ if download_path.is_file(): if not force_overwrite: - logger.warn(f'File already downloaded to {download_path}.') + logger.error(f'File already downloaded to {download_path} and force_overwrite is not set.') + return 1 else: try: os.unlink(download_path) @@ -134,20 +135,25 @@ def download_file(parsed_uri, download_path: pathlib.Path, force_overwrite: bool return 1 if parsed_uri.scheme in ['http', 'https', 'ftp', 'ftps']: - with urllib.request.urlopen(parsed_uri.geturl()) as s: - download_file_size = 0 - try: - download_file_size = s.headers['content-length'] - except KeyError: - pass - def download_progress(downloaded_bytes): - if download_progress_callback: - return download_progress_callback(int(downloaded_bytes), int(download_file_size)) - return False - with download_path.open('wb') as f: - download_cancelled = copyfileobj(s, f, download_progress) - if download_cancelled: - return 1 + try: + with urllib.request.urlopen(parsed_uri.geturl()) as s: + download_file_size = 0 + try: + download_file_size = s.headers['content-length'] + except KeyError: + pass + def download_progress(downloaded_bytes): + if download_progress_callback: + return download_progress_callback(int(downloaded_bytes), int(download_file_size)) + return False + with download_path.open('wb') as f: + download_cancelled = copyfileobj(s, f, download_progress) + if download_cancelled: + logger.warn(f'Download of file to {download_path} cancelled.') + return 1 + except urllib.error.HTTPError as e: + logger.error(f'HTTP Error {e.code} opening {parsed_uri.geturl()}') + return 1 else: origin_file = pathlib.Path(parsed_uri.geturl()).resolve() if not origin_file.is_file(): @@ -167,7 +173,7 @@ def download_zip_file(parsed_uri, download_zip_path: pathlib.Path, force_overwri return download_file_result if not zipfile.is_zipfile(download_zip_path): - logger.error(f"File zip {download_zip_path} is invalid.") + logger.error(f"File zip {download_zip_path} is invalid. Try re-downloading the file.") download_zip_path.unlink() return 1