From 32f69227f131ecdc1e2751d870cc1307ee51812b Mon Sep 17 00:00:00 2001 From: Marcus Tillmanns Date: Tue, 31 Oct 2023 09:58:23 +0100 Subject: [PATCH] IOS: Unify error handling Instead of using some flags in some struct, we convert the error handling to use expected_str to clean up the error handling with the outside. Change-Id: I0f8d10c99715989e0069568ebc1d799d412a0600 Reviewed-by: Eike Ziller --- src/plugins/ios/iossettingspage.cpp | 21 ++- src/plugins/ios/iostoolhandler.cpp | 53 ++++--- src/plugins/ios/simulatorcontrol.cpp | 145 ++++++++++--------- src/plugins/ios/simulatorcontrol.h | 56 +++---- src/plugins/ios/simulatoroperationdialog.cpp | 10 +- src/plugins/ios/simulatoroperationdialog.h | 3 +- 6 files changed, 143 insertions(+), 145 deletions(-) diff --git a/src/plugins/ios/iossettingspage.cpp b/src/plugins/ios/iossettingspage.cpp index 3292fefc079..00fa6fb13c3 100644 --- a/src/plugins/ios/iossettingspage.cpp +++ b/src/plugins/ios/iossettingspage.cpp @@ -74,8 +74,10 @@ static SimulatorInfoList selectedSimulators(const QTreeView *deviceTreeView) return list; } -static void onSimOperation(const SimulatorInfo &simInfo, SimulatorOperationDialog* dlg, - const QString &contextStr, const SimulatorControl::ResponseData &response) +static void onSimOperation(const SimulatorInfo &simInfo, + SimulatorOperationDialog *dlg, + const QString &contextStr, + const SimulatorControl::Response &response) { dlg->addMessage(simInfo, response, contextStr); } @@ -224,14 +226,17 @@ void IosSettingsWidget::onCreate() statusDialog->setAttribute(Qt::WA_DeleteOnClose); statusDialog->addMessage(Tr::tr("Creating simulator device..."), Utils::NormalMessageFormat); const auto onSimulatorCreate = [statusDialog](const QString &name, - const SimulatorControl::ResponseData &response) { - if (response.success) { + const SimulatorControl::Response &response) { + if (response) { statusDialog->addMessage(Tr::tr("Simulator device (%1) created.\nUDID: %2") - .arg(name).arg(response.simUdid), Utils::StdOutFormat); + .arg(name) + .arg(response->simUdid), + Utils::StdOutFormat); } else { - statusDialog->addMessage(Tr::tr("Simulator device (%1) creation failed.\nError: %2"). - arg(name).arg(response.commandOutput), - Utils::StdErrFormat); + statusDialog->addMessage(Tr::tr("Simulator device (%1) creation failed.\nError: %2") + .arg(name) + .arg(response.error()), + Utils::StdErrFormat); } }; diff --git a/src/plugins/ios/iostoolhandler.cpp b/src/plugins/ios/iostoolhandler.cpp index 43cc6fab142..88a15290f20 100644 --- a/src/plugins/ios/iostoolhandler.cpp +++ b/src/plugins/ios/iostoolhandler.cpp @@ -738,16 +738,16 @@ void IosSimulatorToolHandlerPrivate::requestTransferApp(const FilePath &appBundl m_deviceId = deviceIdentifier; isTransferringApp(m_bundlePath, m_deviceId, 0, 100, ""); - auto onSimulatorStart = [this](const SimulatorControl::ResponseData &response) { - if (!isResponseValid(response)) - return; + auto onSimulatorStart = [this](const SimulatorControl::Response &response) { + if (response) { + if (!isResponseValid(*response)) + return; - if (response.success) { installAppOnSimulator(); } else { errorMsg(Tr::tr("Application install on simulator failed. Simulator not running.")); - if (!response.commandOutput.isEmpty()) - errorMsg(response.commandOutput); + if (!response.error().isEmpty()) + errorMsg(response.error()); didTransferApp(m_bundlePath, m_deviceId, IosToolHandler::Failure); emit q->finished(q); } @@ -778,13 +778,15 @@ void IosSimulatorToolHandlerPrivate::requestRunApp(const FilePath &appBundlePath return; } - auto onSimulatorStart = [this, extraArgs](const SimulatorControl::ResponseData &response) { - if (!isResponseValid(response)) - return; - if (response.success) { + auto onSimulatorStart = [this, extraArgs](const SimulatorControl::Response &response) { + if (response) { + if (!isResponseValid(*response)) + return; + launchAppOnSimulator(extraArgs); } else { - errorMsg(Tr::tr("Application launch on simulator failed. Simulator not running.")); + errorMsg(Tr::tr("Application launch on simulator failed. Simulator not running. %1") + .arg(response.error())); didStartApp(m_bundlePath, m_deviceId, Ios::IosToolHandler::Failure); } }; @@ -827,16 +829,14 @@ void IosSimulatorToolHandlerPrivate::stop(int errorCode) void IosSimulatorToolHandlerPrivate::installAppOnSimulator() { - auto onResponseAppInstall = [this](const SimulatorControl::ResponseData &response) { - if (!isResponseValid(response)) - return; - - if (response.success) { + auto onResponseAppInstall = [this](const SimulatorControl::Response &response) { + if (response) { + if (!isResponseValid(*response)) + return; isTransferringApp(m_bundlePath, m_deviceId, 100, 100, ""); didTransferApp(m_bundlePath, m_deviceId, IosToolHandler::Success); } else { - errorMsg(Tr::tr("Application install on simulator failed. %1") - .arg(response.commandOutput)); + errorMsg(Tr::tr("Application install on simulator failed. %1").arg(response.error())); didTransferApp(m_bundlePath, m_deviceId, IosToolHandler::Failure); } emit q->finished(q); @@ -884,22 +884,21 @@ void IosSimulatorToolHandlerPrivate::launchAppOnSimulator(const QStringList &ext stop(0); }; - auto onResponseAppLaunch = [=](const SimulatorControl::ResponseData &response) { - if (!isResponseValid(response)) - return; - if (response.success) { - m_pid = response.pID; - gotInferiorPid(m_bundlePath, m_deviceId, response.pID); + auto onResponseAppLaunch = [=](const SimulatorControl::Response &response) { + if (response) { + if (!isResponseValid(*response)) + return; + m_pid = response->inferiorPid; + gotInferiorPid(m_bundlePath, m_deviceId, response->inferiorPid); didStartApp(m_bundlePath, m_deviceId, Ios::IosToolHandler::Success); // Start monitoring app's life signs. - futureSynchronizer.addFuture(Utils::asyncRun(monitorPid, response.pID)); + futureSynchronizer.addFuture(Utils::asyncRun(monitorPid, response->inferiorPid)); if (captureConsole) futureSynchronizer.addFuture(Utils::asyncRun(&LogTailFiles::exec, &outputLogger, stdoutFile, stderrFile)); } else { m_pid = -1; - errorMsg(Tr::tr("Application launch on simulator failed. %1") - .arg(response.commandOutput)); + errorMsg(Tr::tr("Application launch on simulator failed. %1").arg(response.error())); didStartApp(m_bundlePath, m_deviceId, Ios::IosToolHandler::Failure); stop(-1); emit q->finished(q); diff --git a/src/plugins/ios/simulatorcontrol.cpp b/src/plugins/ios/simulatorcontrol.cpp index 8b9b445fedb..8f83461941f 100644 --- a/src/plugins/ios/simulatorcontrol.cpp +++ b/src/plugins/ios/simulatorcontrol.cpp @@ -188,30 +188,27 @@ static SimulatorInfo deviceInfo(const QString &simUdid); static QString bundleIdentifier(const Utils::FilePath &bundlePath); static QString bundleExecutable(const Utils::FilePath &bundlePath); -static void startSimulator(QPromise &promise, - const QString &simUdid); -static void installApp(QPromise &promise, +static void startSimulator(QPromise &promise, const QString &simUdid); +static void installApp(QPromise &promise, const QString &simUdid, const Utils::FilePath &bundlePath); -static void launchApp(QPromise &promise, +static void launchApp(QPromise &promise, const QString &simUdid, const QString &bundleIdentifier, bool waitForDebugger, const QStringList &extraArgs, const QString &stdoutPath, const QString &stderrPath); -static void deleteSimulator(QPromise &promise, - const QString &simUdid); -static void resetSimulator(QPromise &promise, - const QString &simUdid); -static void renameSimulator(QPromise &promise, +static void deleteSimulator(QPromise &promise, const QString &simUdid); +static void resetSimulator(QPromise &promise, const QString &simUdid); +static void renameSimulator(QPromise &promise, const QString &simUdid, const QString &newName); -static void createSimulator(QPromise &promise, +static void createSimulator(QPromise &promise, const QString &name, const DeviceTypeInfo &deviceType, const RuntimeInfo &runtime); -static void takeSceenshot(QPromise &promise, +static void takeSceenshot(QPromise &promise, const QString &simUdid, const QString &filePath); @@ -309,23 +306,23 @@ QString SimulatorControl::bundleExecutable(const Utils::FilePath &bundlePath) return Internal::bundleExecutable(bundlePath); } -QFuture SimulatorControl::startSimulator(const QString &simUdid) +QFuture SimulatorControl::startSimulator(const QString &simUdid) { return Utils::asyncRun(Internal::startSimulator, simUdid); } -QFuture SimulatorControl::installApp( - const QString &simUdid, const Utils::FilePath &bundlePath) +QFuture SimulatorControl::installApp(const QString &simUdid, + const Utils::FilePath &bundlePath) { return Utils::asyncRun(Internal::installApp, simUdid, bundlePath); } -QFuture SimulatorControl::launchApp(const QString &simUdid, - const QString &bundleIdentifier, - bool waitForDebugger, - const QStringList &extraArgs, - const QString &stdoutPath, - const QString &stderrPath) +QFuture SimulatorControl::launchApp(const QString &simUdid, + const QString &bundleIdentifier, + bool waitForDebugger, + const QStringList &extraArgs, + const QString &stdoutPath, + const QString &stderrPath) { return Utils::asyncRun(Internal::launchApp, simUdid, @@ -336,32 +333,30 @@ QFuture SimulatorControl::launchApp(const QStrin stderrPath); } -QFuture SimulatorControl::deleteSimulator(const QString &simUdid) +QFuture SimulatorControl::deleteSimulator(const QString &simUdid) { return Utils::asyncRun(Internal::deleteSimulator, simUdid); } -QFuture SimulatorControl::resetSimulator(const QString &simUdid) +QFuture SimulatorControl::resetSimulator(const QString &simUdid) { return Utils::asyncRun(Internal::resetSimulator, simUdid); } -QFuture SimulatorControl::renameSimulator(const QString &simUdid, - const QString &newName) +QFuture SimulatorControl::renameSimulator(const QString &simUdid, + const QString &newName) { return Utils::asyncRun(Internal::renameSimulator, simUdid, newName); } -QFuture -SimulatorControl::createSimulator(const QString &name, - const DeviceTypeInfo &deviceType, - const RuntimeInfo &runtime) +QFuture SimulatorControl::createSimulator( + const QString &name, const DeviceTypeInfo &deviceType, const RuntimeInfo &runtime) { return Utils::asyncRun(Internal::createSimulator, name, deviceType, runtime); } -QFuture SimulatorControl::takeSceenshot(const QString &simUdid, - const QString &filePath) +QFuture SimulatorControl::takeSceenshot(const QString &simUdid, + const QString &filePath) { return Utils::asyncRun(Internal::takeSceenshot, simUdid, filePath); } @@ -419,14 +414,14 @@ QString bundleExecutable(const Utils::FilePath &bundlePath) return executable; } -void startSimulator(QPromise &promise, const QString &simUdid) +void startSimulator(QPromise &promise, const QString &simUdid) { SimulatorControl::ResponseData response(simUdid); SimulatorInfo simInfo = deviceInfo(simUdid); if (!simInfo.available) { promise.addResult( - response.withError(Tr::tr("Simulator device is not available. (%1)").arg(simUdid))); + make_unexpected(Tr::tr("Simulator device is not available. (%1)").arg(simUdid))); return; } // Shutting down state checks are for the case when simulator start is called within a short @@ -436,7 +431,7 @@ void startSimulator(QPromise &promise, const QSt while (simInfo.isShuttingDown() && !simulatorStartDeadline.hasExpired()) { // Wait till the simulator shuts down, if doing so. if (promise.isCanceled()) { - promise.addResult(response.withError(Tr::tr("Simulator start was canceled."))); + promise.addResult(make_unexpected(Tr::tr("Simulator start was canceled."))); return; } @@ -446,14 +441,14 @@ void startSimulator(QPromise &promise, const QSt if (simInfo.isShuttingDown()) { promise.addResult( - response.withError(Tr::tr("Cannot start Simulator device. Previous instance taking " - "too long to shut down. (%1)") - .arg(simInfo.toString()))); + make_unexpected(Tr::tr("Cannot start Simulator device. Previous instance taking " + "too long to shut down. (%1)") + .arg(simInfo.toString()))); return; } if (!simInfo.isShutdown()) { - promise.addResult(response.withError( + promise.addResult(make_unexpected( Tr::tr("Cannot start Simulator device. Simulator not in shutdown state. (%1)") .arg(simInfo.toString()))); return; @@ -462,7 +457,7 @@ void startSimulator(QPromise &promise, const QSt expected_str result = launchSimulator(simUdid, [&promise] { return promise.isCanceled(); }); if (!result) { - promise.addResult(response.withError(result.error())); + promise.addResult(make_unexpected(result.error())); return; } @@ -475,24 +470,29 @@ void startSimulator(QPromise &promise, const QSt do { info = deviceInfo(simUdid); if (promise.isCanceled()) { - promise.addResult(response.withError(Tr::tr("Simulator start was canceled."))); + promise.addResult(make_unexpected(Tr::tr("Simulator start was canceled."))); return; } } while (!info.isBooted() && !simulatorStartDeadline.hasExpired()); - if (info.isBooted()) - response.success = true; + if (!info.isBooted()) { + promise.addResult(make_unexpected( + Tr::tr("Cannot start Simulator device. Simulator not in booted state. (%1)") + .arg(info.toString()))); + return; + } - promise.addResult(response.withSuccess()); + promise.addResult(response); } -void installApp(QPromise &promise, - const QString &simUdid, const Utils::FilePath &bundlePath) +void installApp(QPromise &promise, + const QString &simUdid, + const Utils::FilePath &bundlePath) { SimulatorControl::ResponseData response(simUdid); if (!bundlePath.exists()) { - promise.addResult(response.withError(Tr::tr("Bundle path does not exist."))); + promise.addResult(make_unexpected(Tr::tr("Bundle path does not exist."))); return; } @@ -501,12 +501,12 @@ void installApp(QPromise &promise, &response.commandOutput, [&promise] { return promise.isCanceled(); }); if (!result) - promise.addResult(response.withError(result.error())); + promise.addResult(make_unexpected(result.error())); else - promise.addResult(response.withSuccess()); + promise.addResult(response); } -void launchApp(QPromise &promise, +void launchApp(QPromise &promise, const QString &simUdid, const QString &bundleIdentifier, bool waitForDebugger, @@ -517,7 +517,7 @@ void launchApp(QPromise &promise, SimulatorControl::ResponseData response(simUdid); if (bundleIdentifier.isEmpty()) { - promise.addResult(response.withError(Tr::tr("Invalid (empty) bundle identifier."))); + promise.addResult(make_unexpected(Tr::tr("Invalid (empty) bundle identifier."))); return; } @@ -545,19 +545,24 @@ void launchApp(QPromise &promise, [&promise] { return promise.isCanceled(); }); if (!result) { - promise.addResult(response.withError(result.error())); + promise.addResult(make_unexpected(result.error())); return; } const QString pIdStr = stdOutput.trimmed().split(' ').last().trimmed(); bool validPid = false; - response.pID = pIdStr.toLongLong(&validPid); - response.success = validPid; + response.inferiorPid = pIdStr.toLongLong(&validPid); - promise.addResult(response.withSuccess()); + if (!validPid) { + promise.addResult( + make_unexpected(Tr::tr("Failed to convert inferior pid. (%1)").arg(pIdStr))); + return; + } + + promise.addResult(response); } -void deleteSimulator(QPromise &promise, const QString &simUdid) +void deleteSimulator(QPromise &promise, const QString &simUdid) { SimulatorControl::ResponseData response(simUdid); expected_str result = runSimCtlCommand({"delete", simUdid}, @@ -566,12 +571,12 @@ void deleteSimulator(QPromise &promise, const QS [&promise] { return promise.isCanceled(); }); if (!result) - promise.addResult(response.withError(result.error())); + promise.addResult(make_unexpected(result.error())); else - promise.addResult(response.withSuccess()); + promise.addResult(response); } -void resetSimulator(QPromise &promise, const QString &simUdid) +void resetSimulator(QPromise &promise, const QString &simUdid) { SimulatorControl::ResponseData response(simUdid); expected_str result = runSimCtlCommand({"erase", simUdid}, @@ -580,12 +585,12 @@ void resetSimulator(QPromise &promise, const QSt [&promise] { return promise.isCanceled(); }); if (!result) - promise.addResult(response.withError(result.error())); + promise.addResult(make_unexpected(result.error())); else - promise.addResult(response.withSuccess()); + promise.addResult(response); } -void renameSimulator(QPromise &promise, +void renameSimulator(QPromise &promise, const QString &simUdid, const QString &newName) { @@ -595,12 +600,12 @@ void renameSimulator(QPromise &promise, &response.commandOutput, [&promise] { return promise.isCanceled(); }); if (!result) - promise.addResult(response.withError(result.error())); + promise.addResult(make_unexpected(result.error())); else - promise.addResult(response.withSuccess()); + promise.addResult(response); } -void createSimulator(QPromise &promise, +void createSimulator(QPromise &promise, const QString &name, const DeviceTypeInfo &deviceType, const RuntimeInfo &runtime) @@ -618,15 +623,17 @@ void createSimulator(QPromise &promise, &stdOutput, &response.commandOutput, [&promise] { return promise.isCanceled(); }); - response.simUdid = result ? stdOutput.trimmed() : QString(); + + if (result) + response.simUdid = stdOutput.trimmed(); if (!result) - promise.addResult(response.withError(result.error())); + promise.addResult(make_unexpected(result.error())); else - promise.addResult(response.withSuccess()); + promise.addResult(response); } -void takeSceenshot(QPromise &promise, +void takeSceenshot(QPromise &promise, const QString &simUdid, const QString &filePath) { @@ -637,9 +644,9 @@ void takeSceenshot(QPromise &promise, [&promise] { return promise.isCanceled(); }); if (!result) - promise.addResult(response.withError(result.error())); + promise.addResult(make_unexpected(result.error())); else - promise.addResult(response.withSuccess()); + promise.addResult(response); } QString SimulatorInfo::toString() const diff --git a/src/plugins/ios/simulatorcontrol.h b/src/plugins/ios/simulatorcontrol.h index f0bb0e11d1c..4dc6ff8c6ba 100644 --- a/src/plugins/ios/simulatorcontrol.h +++ b/src/plugins/ios/simulatorcontrol.h @@ -54,30 +54,17 @@ class SimulatorControl { public: struct ResponseData { - ResponseData(const QString & udid): - simUdid(udid) { } + ResponseData(const QString &udid) + : simUdid(udid) + {} QString simUdid; - bool success = false; - qint64 pID = -1; + qint64 inferiorPid{-1}; QString commandOutput; - - ResponseData withError(const QString errorMsg) - { - ResponseData result = *this; - result.commandOutput = errorMsg; - result.success = false; - return result; - } - - ResponseData withSuccess() - { - ResponseData result = std::move(*this); - result.success = true; - return result; - } }; + using Response = Utils::expected_str; + public: static QList availableDeviceTypes(); static QFuture> updateDeviceTypes(QObject *context); @@ -90,22 +77,21 @@ public: static QString bundleExecutable(const Utils::FilePath &bundlePath); public: - static QFuture startSimulator(const QString &simUdid); - static QFuture installApp(const QString &simUdid, - const Utils::FilePath &bundlePath); - static QFuture launchApp(const QString &simUdid, - const QString &bundleIdentifier, - bool waitForDebugger, - const QStringList &extraArgs, - const QString &stdoutPath = QString(), - const QString &stderrPath = QString()); - static QFuture deleteSimulator(const QString &simUdid); - static QFuture resetSimulator(const QString &simUdid); - static QFuture renameSimulator(const QString &simUdid, const QString &newName); - static QFuture createSimulator(const QString &name, - const DeviceTypeInfo &deviceType, - const RuntimeInfo &runtime); - static QFuture takeSceenshot(const QString &simUdid, const QString &filePath); + static QFuture startSimulator(const QString &simUdid); + static QFuture installApp(const QString &simUdid, const Utils::FilePath &bundlePath); + static QFuture launchApp(const QString &simUdid, + const QString &bundleIdentifier, + bool waitForDebugger, + const QStringList &extraArgs, + const QString &stdoutPath = QString(), + const QString &stderrPath = QString()); + static QFuture deleteSimulator(const QString &simUdid); + static QFuture resetSimulator(const QString &simUdid); + static QFuture renameSimulator(const QString &simUdid, const QString &newName); + static QFuture createSimulator(const QString &name, + const DeviceTypeInfo &deviceType, + const RuntimeInfo &runtime); + static QFuture takeSceenshot(const QString &simUdid, const QString &filePath); }; } // Ios::Internal diff --git a/src/plugins/ios/simulatoroperationdialog.cpp b/src/plugins/ios/simulatoroperationdialog.cpp index 180b28d76e4..b260f503101 100644 --- a/src/plugins/ios/simulatoroperationdialog.cpp +++ b/src/plugins/ios/simulatoroperationdialog.cpp @@ -94,15 +94,15 @@ void SimulatorOperationDialog::addMessage(const QString &message, Utils::OutputF } void SimulatorOperationDialog::addMessage(const SimulatorInfo &siminfo, - const SimulatorControl::ResponseData &response, - const QString &context) + const SimulatorControl::Response &response, + const QString &context) { - QTC_CHECK(siminfo.identifier == response.simUdid); - if (response.success) { + if (response) { + QTC_CHECK(siminfo.identifier == response->simUdid); addMessage(Tr::tr("%1, %2\nOperation %3 completed successfully.").arg(siminfo.name) .arg(siminfo.runtimeName).arg(context), Utils::StdOutFormat); } else { - QString erroMsg = response.commandOutput.trimmed(); + QString erroMsg = response.error(); QString message = Tr::tr("%1, %2\nOperation %3 failed.\nUDID: %4\nError: %5").arg(siminfo.name) .arg(siminfo.runtimeName).arg(context).arg(siminfo.identifier) .arg(erroMsg.isEmpty() ? Tr::tr("Unknown") : erroMsg); diff --git a/src/plugins/ios/simulatoroperationdialog.h b/src/plugins/ios/simulatoroperationdialog.h index affa0d7c07b..46cbb3f787e 100644 --- a/src/plugins/ios/simulatoroperationdialog.h +++ b/src/plugins/ios/simulatoroperationdialog.h @@ -30,7 +30,8 @@ public: public: void addFutures(const QList > &futureList); void addMessage(const QString &message, Utils::OutputFormat format); - void addMessage(const SimulatorInfo &siminfo, const SimulatorControl::ResponseData &response, + void addMessage(const SimulatorInfo &siminfo, + const SimulatorControl::Response &response, const QString &context); private: