From 41fc0fd0849758917314a5a9024dacb4479828a2 Mon Sep 17 00:00:00 2001 From: "Vladimir Golovnev (Glassez)" Date: Wed, 6 Oct 2021 21:45:37 +0300 Subject: [PATCH 1/2] Don't use output parameters for error handling --- src/base/bittorrent/session.cpp | 9 ++- src/base/bittorrent/torrentinfo.cpp | 44 +++---------- src/base/bittorrent/torrentinfo.h | 5 +- src/base/rss/rss_session.cpp | 96 ++++++++++------------------ src/base/rss/rss_session.h | 15 +++-- src/base/torrentfileswatcher.cpp | 12 ++-- src/gui/addnewtorrentdialog.cpp | 42 ++++++------ src/gui/addnewtorrentdialog.h | 2 +- src/gui/rss/rsswidget.cpp | 18 +++--- src/gui/torrentcreatordialog.cpp | 6 +- src/webui/api/rsscontroller.cpp | 24 +++---- src/webui/api/torrentscontroller.cpp | 6 +- 12 files changed, 115 insertions(+), 164 deletions(-) diff --git a/src/base/bittorrent/session.cpp b/src/base/bittorrent/session.cpp index dd846fedb..970032733 100644 --- a/src/base/bittorrent/session.cpp +++ b/src/base/bittorrent/session.cpp @@ -1698,7 +1698,7 @@ void Session::handleDownloadFinished(const Net::DownloadResult &result) { case Net::DownloadStatus::Success: emit downloadFromUrlFinished(result.url); - addTorrent(TorrentInfo::load(result.data), m_downloadedTorrents.take(result.url)); + addTorrent(TorrentInfo::load(result.data).value_or(TorrentInfo()), m_downloadedTorrents.take(result.url)); break; case Net::DownloadStatus::RedirectedToMagnet: emit downloadFromUrlFinished(result.url); @@ -2028,7 +2028,7 @@ bool Session::addTorrent(const QString &source, const AddTorrentParams ¶ms) return addTorrent(magnetUri, params); TorrentFileGuard guard {source}; - if (addTorrent(TorrentInfo::loadFromFile(source), params)) + if (addTorrent(TorrentInfo::loadFromFile(source).value_or(TorrentInfo()), params)) { guard.markAsAddedToSession(); return true; @@ -3938,8 +3938,7 @@ void Session::handleTorrentFinished(TorrentImpl *const torrent) qDebug("Found possible recursive torrent download."); const QString torrentFullpath = torrent->savePath(true) + '/' + torrentRelpath; qDebug("Full subtorrent path is %s", qUtf8Printable(torrentFullpath)); - TorrentInfo torrentInfo = TorrentInfo::loadFromFile(torrentFullpath); - if (torrentInfo.isValid()) + if (TorrentInfo::loadFromFile(torrentFullpath)) { qDebug("emitting recursiveTorrentDownloadPossible()"); emit recursiveTorrentDownloadPossible(torrent); @@ -4167,7 +4166,7 @@ void Session::recursiveTorrentDownload(const TorrentID &id) AddTorrentParams params; // Passing the save path along to the sub torrent file params.savePath = torrent->savePath(); - addTorrent(TorrentInfo::loadFromFile(torrentFullpath), params); + addTorrent(TorrentInfo::loadFromFile(torrentFullpath).value_or(TorrentInfo()), params); } } } diff --git a/src/base/bittorrent/torrentinfo.cpp b/src/base/bittorrent/torrentinfo.cpp index 499c0c96e..7a514b427 100644 --- a/src/base/bittorrent/torrentinfo.cpp +++ b/src/base/bittorrent/torrentinfo.cpp @@ -106,7 +106,7 @@ TorrentInfo &TorrentInfo::operator=(const TorrentInfo &other) return *this; } -TorrentInfo TorrentInfo::load(const QByteArray &data, QString *error) noexcept +nonstd::expected TorrentInfo::load(const QByteArray &data) noexcept { // 2-step construction to overcome default limits of `depth_limit` & `token_limit` which are // used in `torrent_info()` constructor @@ -117,42 +117,23 @@ TorrentInfo TorrentInfo::load(const QByteArray &data, QString *error) noexcept const lt::bdecode_node node = lt::bdecode(data, ec , nullptr, depthLimit, tokenLimit); if (ec) - { - if (error) - *error = QString::fromStdString(ec.message()); - return TorrentInfo(); - } + return nonstd::make_unexpected(QString::fromStdString(ec.message())); TorrentInfo info {std::shared_ptr(new lt::torrent_info(node, ec))}; if (ec) - { - if (error) - *error = QString::fromStdString(ec.message()); - return TorrentInfo(); - } + return nonstd::make_unexpected(QString::fromStdString(ec.message())); return info; } -TorrentInfo TorrentInfo::loadFromFile(const QString &path, QString *error) noexcept +nonstd::expected TorrentInfo::loadFromFile(const QString &path) noexcept { - if (error) - error->clear(); - QFile file {path}; if (!file.open(QIODevice::ReadOnly)) - { - if (error) - *error = file.errorString(); - return TorrentInfo(); - } + return nonstd::make_unexpected(file.errorString()); if (file.size() > MAX_TORRENT_SIZE) - { - if (error) - *error = tr("File size exceeds max limit %1").arg(Utils::Misc::friendlyUnit(MAX_TORRENT_SIZE)); - return TorrentInfo(); - } + return nonstd::make_unexpected(tr("File size exceeds max limit %1").arg(Utils::Misc::friendlyUnit(MAX_TORRENT_SIZE))); QByteArray data; try @@ -161,20 +142,15 @@ TorrentInfo TorrentInfo::loadFromFile(const QString &path, QString *error) noexc } catch (const std::bad_alloc &e) { - if (error) - *error = tr("Torrent file read error: %1").arg(e.what()); - return TorrentInfo(); + return nonstd::make_unexpected(tr("Torrent file read error: %1").arg(e.what())); } + if (data.size() != file.size()) - { - if (error) - *error = tr("Torrent file read error: size mismatch"); - return TorrentInfo(); - } + return nonstd::make_unexpected(tr("Torrent file read error: size mismatch")); file.close(); - return load(data, error); + return load(data); } void TorrentInfo::saveToFile(const QString &path) const diff --git a/src/base/bittorrent/torrentinfo.h b/src/base/bittorrent/torrentinfo.h index 7aa1cb560..eb98d878c 100644 --- a/src/base/bittorrent/torrentinfo.h +++ b/src/base/bittorrent/torrentinfo.h @@ -33,6 +33,7 @@ #include #include +#include "base/3rdparty/expected.hpp" #include "base/indexrange.h" #include "abstractfilestorage.h" #include "torrentcontentlayout.h" @@ -55,8 +56,8 @@ namespace BitTorrent explicit TorrentInfo(std::shared_ptr nativeInfo = {}); TorrentInfo(const TorrentInfo &other); - static TorrentInfo load(const QByteArray &data, QString *error = nullptr) noexcept; - static TorrentInfo loadFromFile(const QString &path, QString *error = nullptr) noexcept; + static nonstd::expected load(const QByteArray &data) noexcept; + static nonstd::expected loadFromFile(const QString &path) noexcept; void saveToFile(const QString &path) const; TorrentInfo &operator=(const TorrentInfo &other); diff --git a/src/base/rss/rss_session.cpp b/src/base/rss/rss_session.cpp index 566e734a6..e48d8d4ed 100644 --- a/src/base/rss/rss_session.cpp +++ b/src/base/rss/rss_session.cpp @@ -142,67 +142,59 @@ Session *Session::instance() return m_instance; } -bool Session::addFolder(const QString &path, QString *error) +nonstd::expected Session::addFolder(const QString &path) { - Folder *destFolder = prepareItemDest(path, error); - if (!destFolder) - return false; + const nonstd::expected result = prepareItemDest(path); + if (!result) + return result.get_unexpected(); + const auto destFolder = result.value(); addItem(new Folder(path), destFolder); store(); - return true; + return {}; } -bool Session::addFeed(const QString &url, const QString &path, QString *error) +nonstd::expected Session::addFeed(const QString &url, const QString &path) { if (m_feedsByURL.contains(url)) - { - if (error) - *error = tr("RSS feed with given URL already exists: %1.").arg(url); - return false; - } + return nonstd::make_unexpected(tr("RSS feed with given URL already exists: %1.").arg(url)); - Folder *destFolder = prepareItemDest(path, error); - if (!destFolder) - return false; + const nonstd::expected result = prepareItemDest(path); + if (!result) + return result.get_unexpected(); + const auto destFolder = result.value(); addItem(new Feed(generateUID(), url, path, this), destFolder); store(); if (m_processingEnabled) feedByURL(url)->refresh(); - return true; + + return {}; } -bool Session::moveItem(const QString &itemPath, const QString &destPath, QString *error) +nonstd::expected Session::moveItem(const QString &itemPath, const QString &destPath) { if (itemPath.isEmpty()) - { - if (error) - *error = tr("Cannot move root folder."); - return false; - } + return nonstd::make_unexpected(tr("Cannot move root folder.")); auto item = m_itemsByPath.value(itemPath); if (!item) - { - if (error) - *error = tr("Item doesn't exist: %1.").arg(itemPath); - return false; - } + return nonstd::make_unexpected(tr("Item doesn't exist: %1.").arg(itemPath)); - return moveItem(item, destPath, error); + return moveItem(item, destPath); } -bool Session::moveItem(Item *item, const QString &destPath, QString *error) +nonstd::expected Session::moveItem(Item *item, const QString &destPath) { Q_ASSERT(item); Q_ASSERT(item != rootFolder()); - Folder *destFolder = prepareItemDest(destPath, error); - if (!destFolder) - return false; + const nonstd::expected result = prepareItemDest(destPath); + if (!result) + return result.get_unexpected(); auto srcFolder = static_cast(m_itemsByPath.value(Item::parentPath(item->path()))); + const auto destFolder = result.value(); if (srcFolder != destFolder) { srcFolder->removeItem(item); @@ -211,25 +203,17 @@ bool Session::moveItem(Item *item, const QString &destPath, QString *error) m_itemsByPath.insert(destPath, m_itemsByPath.take(item->path())); item->setPath(destPath); store(); - return true; + return {}; } -bool Session::removeItem(const QString &itemPath, QString *error) +nonstd::expected Session::removeItem(const QString &itemPath) { if (itemPath.isEmpty()) - { - if (error) - *error = tr("Cannot delete root folder."); - return false; - } + return nonstd::make_unexpected(tr("Cannot delete root folder.")); - auto item = m_itemsByPath.value(itemPath); + auto *item = m_itemsByPath.value(itemPath); if (!item) - { - if (error) - *error = tr("Item doesn't exist: %1.").arg(itemPath); - return false; - } + return nonstd::make_unexpected(tr("Item doesn't exist: %1.").arg(itemPath)); emit itemAboutToBeRemoved(item); item->cleanup(); @@ -238,7 +222,7 @@ bool Session::removeItem(const QString &itemPath, QString *error) folder->removeItem(item); delete item; store(); - return true; + return {}; } QList Session::items() const @@ -395,30 +379,18 @@ void Session::store() m_confFileStorage->store(FeedsFileName, QJsonDocument(rootFolder()->toJsonValue().toObject()).toJson()); } -Folder *Session::prepareItemDest(const QString &path, QString *error) +nonstd::expected Session::prepareItemDest(const QString &path) { if (!Item::isValidPath(path)) - { - if (error) - *error = tr("Incorrect RSS Item path: %1.").arg(path); - return nullptr; - } + return nonstd::make_unexpected(tr("Incorrect RSS Item path: %1.").arg(path)); if (m_itemsByPath.contains(path)) - { - if (error) - *error = tr("RSS item with given path already exists: %1.").arg(path); - return nullptr; - } + return nonstd::make_unexpected(tr("RSS item with given path already exists: %1.").arg(path)); const QString destFolderPath = Item::parentPath(path); - auto destFolder = qobject_cast(m_itemsByPath.value(destFolderPath)); + const auto destFolder = qobject_cast(m_itemsByPath.value(destFolderPath)); if (!destFolder) - { - if (error) - *error = tr("Parent folder doesn't exist: %1.").arg(destFolderPath); - return nullptr; - } + return nonstd::make_unexpected(tr("Parent folder doesn't exist: %1.").arg(destFolderPath)); return destFolder; } diff --git a/src/base/rss/rss_session.h b/src/base/rss/rss_session.h index 09200a466..a3d12d589 100644 --- a/src/base/rss/rss_session.h +++ b/src/base/rss/rss_session.h @@ -73,6 +73,8 @@ #include #include +#include "base/3rdparty/expected.hpp" + class QThread; class Application; @@ -110,12 +112,11 @@ namespace RSS int refreshInterval() const; void setRefreshInterval(int refreshInterval); - bool addFolder(const QString &path, QString *error = nullptr); - bool addFeed(const QString &url, const QString &path, QString *error = nullptr); - bool moveItem(const QString &itemPath, const QString &destPath - , QString *error = nullptr); - bool moveItem(Item *item, const QString &destPath, QString *error = nullptr); - bool removeItem(const QString &itemPath, QString *error = nullptr); + nonstd::expected addFolder(const QString &path); + nonstd::expected addFeed(const QString &url, const QString &path); + nonstd::expected moveItem(const QString &itemPath, const QString &destPath); + nonstd::expected moveItem(Item *item, const QString &destPath); + nonstd::expected removeItem(const QString &itemPath); QList items() const; Item *itemByPath(const QString &path) const; @@ -146,7 +147,7 @@ namespace RSS void loadFolder(const QJsonObject &jsonObj, Folder *folder); void loadLegacy(); void store(); - Folder *prepareItemDest(const QString &path, QString *error); + nonstd::expected prepareItemDest(const QString &path); Folder *addSubfolder(const QString &name, Folder *parentFolder); Feed *addFeedToFolder(const QUuid &uid, const QString &url, const QString &name, Folder *parentFolder); void addItem(Item *item, Folder *destFolder); diff --git a/src/base/torrentfileswatcher.cpp b/src/base/torrentfileswatcher.cpp index cdbf0cf23..afac49c6d 100644 --- a/src/base/torrentfileswatcher.cpp +++ b/src/base/torrentfileswatcher.cpp @@ -526,10 +526,10 @@ void TorrentFilesWatcher::Worker::processFolder(const QString &path, const QStri } else { - const auto torrentInfo = BitTorrent::TorrentInfo::loadFromFile(filePath); - if (torrentInfo.isValid()) + const nonstd::expected result = BitTorrent::TorrentInfo::loadFromFile(filePath); + if (result) { - emit torrentFound(torrentInfo, addTorrentParams); + emit torrentFound(result.value(), addTorrentParams); Utils::Fs::forceRemove(filePath); } else @@ -567,8 +567,8 @@ void TorrentFilesWatcher::Worker::processFailedTorrents() if (!QFile::exists(torrentPath)) return true; - const auto torrentInfo = BitTorrent::TorrentInfo::loadFromFile(torrentPath); - if (torrentInfo.isValid()) + const nonstd::expected result = BitTorrent::TorrentInfo::loadFromFile(torrentPath); + if (result) { BitTorrent::AddTorrentParams addTorrentParams = options.addTorrentParams; const QString exactDirPath = QFileInfo(torrentPath).canonicalPath(); @@ -578,7 +578,7 @@ void TorrentFilesWatcher::Worker::processFailedTorrents() addTorrentParams.savePath = QDir(addTorrentParams.savePath).filePath(subdirPath); } - emit torrentFound(torrentInfo, addTorrentParams); + emit torrentFound(result.value(), addTorrentParams); Utils::Fs::forceRemove(torrentPath); return true; diff --git a/src/gui/addnewtorrentdialog.cpp b/src/gui/addnewtorrentdialog.cpp index 5dfebf114..7320e2647 100644 --- a/src/gui/addnewtorrentdialog.cpp +++ b/src/gui/addnewtorrentdialog.cpp @@ -252,13 +252,13 @@ bool AddNewTorrentDialog::loadTorrentFile(const QString &torrentPath) ? QUrl::fromEncoded(torrentPath.toLocal8Bit()).toLocalFile() : torrentPath; - QString error; - m_torrentInfo = BitTorrent::TorrentInfo::loadFromFile(decodedPath, &error); - if (!m_torrentInfo.isValid()) + const nonstd::expected result = BitTorrent::TorrentInfo::loadFromFile(decodedPath); + m_torrentInfo = result.value_or(BitTorrent::TorrentInfo()); + if (!result) { RaisedMessageBox::critical(this, tr("Invalid torrent") , tr("Failed to load the torrent: %1.\nError: %2", "Don't remove the '\n' characters. They insert a newline.") - .arg(Utils::Fs::toNativePath(decodedPath), error)); + .arg(Utils::Fs::toNativePath(decodedPath), result.error())); return false; } @@ -746,36 +746,38 @@ void AddNewTorrentDialog::setupTreeview() updateDiskSpaceLabel(); } -void AddNewTorrentDialog::handleDownloadFinished(const Net::DownloadResult &result) +void AddNewTorrentDialog::handleDownloadFinished(const Net::DownloadResult &downloadResult) { - QString error; - switch (result.status) + switch (downloadResult.status) { case Net::DownloadStatus::Success: - m_torrentInfo = BitTorrent::TorrentInfo::load(result.data, &error); - if (!m_torrentInfo.isValid()) { - RaisedMessageBox::critical(this, tr("Invalid torrent"), tr("Failed to load from URL: %1.\nError: %2") - .arg(result.url, error)); - return; - } + const nonstd::expected result = BitTorrent::TorrentInfo::load(downloadResult.data); + m_torrentInfo = result.value_or(BitTorrent::TorrentInfo()); + if (!result) + { + RaisedMessageBox::critical(this, tr("Invalid torrent"), tr("Failed to load from URL: %1.\nError: %2") + .arg(downloadResult.url, result.error())); + return; + } - m_torrentGuard = std::make_unique(); + m_torrentGuard = std::make_unique(); - if (loadTorrentImpl()) - open(); - else - deleteLater(); + if (loadTorrentImpl()) + open(); + else + deleteLater(); + } break; case Net::DownloadStatus::RedirectedToMagnet: - if (loadMagnet(BitTorrent::MagnetUri(result.magnet))) + if (loadMagnet(BitTorrent::MagnetUri(downloadResult.magnet))) open(); else deleteLater(); break; default: RaisedMessageBox::critical(this, tr("Download Error"), - tr("Cannot download '%1': %2").arg(result.url, result.errorString)); + tr("Cannot download '%1': %2").arg(downloadResult.url, downloadResult.errorString)); deleteLater(); } } diff --git a/src/gui/addnewtorrentdialog.h b/src/gui/addnewtorrentdialog.h index 372be360e..961cea969 100644 --- a/src/gui/addnewtorrentdialog.h +++ b/src/gui/addnewtorrentdialog.h @@ -82,7 +82,7 @@ private slots: void updateDiskSpaceLabel(); void onSavePathChanged(const QString &newPath); void updateMetadata(const BitTorrent::TorrentInfo &metadata); - void handleDownloadFinished(const Net::DownloadResult &result); + void handleDownloadFinished(const Net::DownloadResult &downloadResult); void TMMChanged(int index); void categoryChanged(int index); void doNotDeleteTorrentClicked(bool checked); diff --git a/src/gui/rss/rsswidget.cpp b/src/gui/rss/rsswidget.cpp index 4a11a7635..6e0c6e767 100644 --- a/src/gui/rss/rsswidget.cpp +++ b/src/gui/rss/rsswidget.cpp @@ -246,10 +246,10 @@ void RSSWidget::askNewFolder() ? RSS::Session::instance()->rootFolder() : qobject_cast(m_feedListWidget->getRSSItem(destItem))); - QString error; const QString newFolderPath = RSS::Item::joinPath(rssDestFolder->path(), newName); - if (!RSS::Session::instance()->addFolder(newFolderPath, &error)) - QMessageBox::warning(this, "qBittorrent", error, QMessageBox::Ok); + const nonstd::expected result = RSS::Session::instance()->addFolder(newFolderPath); + if (!result) + QMessageBox::warning(this, "qBittorrent", result.error(), QMessageBox::Ok); // Expand destination folder to display new feed if (destItem && (destItem != m_feedListWidget->stickyUnreadItem())) @@ -287,11 +287,11 @@ void RSSWidget::on_newFeedButton_clicked() ? RSS::Session::instance()->rootFolder() : qobject_cast(m_feedListWidget->getRSSItem(destItem))); - QString error; // NOTE: We still add feed using legacy way (with URL as feed name) const QString newFeedPath = RSS::Item::joinPath(rssDestFolder->path(), newURL); - if (!RSS::Session::instance()->addFeed(newURL, newFeedPath, &error)) - QMessageBox::warning(this, "qBittorrent", error, QMessageBox::Ok); + const nonstd::expected result = RSS::Session::instance()->addFeed(newURL, newFeedPath); + if (!result) + QMessageBox::warning(this, "qBittorrent", result.error(), QMessageBox::Ok); // Expand destination folder to display new feed if (destItem && (destItem != m_feedListWidget->stickyUnreadItem())) @@ -411,10 +411,10 @@ void RSSWidget::renameSelectedRSSItem() // Check if name is already taken if (!ok) return; - QString error; - if (!RSS::Session::instance()->moveItem(rssItem, RSS::Item::joinPath(parentPath, newName), &error)) + const nonstd::expected result = RSS::Session::instance()->moveItem(rssItem, RSS::Item::joinPath(parentPath, newName)); + if (!result) { - QMessageBox::warning(nullptr, tr("Rename failed"), error); + QMessageBox::warning(nullptr, tr("Rename failed"), result.error()); ok = false; } } while (!ok); diff --git a/src/gui/torrentcreatordialog.cpp b/src/gui/torrentcreatordialog.cpp index 4db90c5b9..452b0dcd7 100644 --- a/src/gui/torrentcreatordialog.cpp +++ b/src/gui/torrentcreatordialog.cpp @@ -234,8 +234,8 @@ void TorrentCreatorDialog::handleCreationSuccess(const QString &path, const QStr if (m_ui->checkStartSeeding->isChecked()) { // Create save path temp data - const BitTorrent::TorrentInfo info = BitTorrent::TorrentInfo::loadFromFile(Utils::Fs::toNativePath(path)); - if (!info.isValid()) + const nonstd::expected result = BitTorrent::TorrentInfo::loadFromFile(Utils::Fs::toNativePath(path)); + if (!result) { QMessageBox::critical(this, tr("Torrent creation failed"), tr("Reason: Created torrent is invalid. It won't be added to download list.")); return; @@ -251,7 +251,7 @@ void TorrentCreatorDialog::handleCreationSuccess(const QString &path, const QStr } params.useAutoTMM = false; // otherwise if it is on by default, it will overwrite `savePath` to the default save path - BitTorrent::Session::instance()->addTorrent(info, params); + BitTorrent::Session::instance()->addTorrent(result.value(), params); } QMessageBox::information(this, tr("Torrent creator") , QString::fromLatin1("%1\n%2").arg(tr("Torrent created:"), Utils::Fs::toNativePath(path))); diff --git a/src/webui/api/rsscontroller.cpp b/src/webui/api/rsscontroller.cpp index f4903eb51..f0a16a8d7 100644 --- a/src/webui/api/rsscontroller.cpp +++ b/src/webui/api/rsscontroller.cpp @@ -50,9 +50,9 @@ void RSSController::addFolderAction() requireParams({"path"}); const QString path = params()["path"].trimmed(); - QString error; - if (!RSS::Session::instance()->addFolder(path, &error)) - throw APIError(APIErrorType::Conflict, error); + const nonstd::expected result = RSS::Session::instance()->addFolder(path); + if (!result) + throw APIError(APIErrorType::Conflict, result.error()); } void RSSController::addFeedAction() @@ -61,9 +61,9 @@ void RSSController::addFeedAction() const QString url = params()["url"].trimmed(); const QString path = params()["path"].trimmed(); - QString error; - if (!RSS::Session::instance()->addFeed(url, (path.isEmpty() ? url : path), &error)) - throw APIError(APIErrorType::Conflict, error); + const nonstd::expected result = RSS::Session::instance()->addFeed(url, (path.isEmpty() ? url : path)); + if (!result) + throw APIError(APIErrorType::Conflict, result.error()); } void RSSController::removeItemAction() @@ -71,9 +71,9 @@ void RSSController::removeItemAction() requireParams({"path"}); const QString path = params()["path"].trimmed(); - QString error; - if (!RSS::Session::instance()->removeItem(path, &error)) - throw APIError(APIErrorType::Conflict, error); + const nonstd::expected result = RSS::Session::instance()->removeItem(path); + if (!result) + throw APIError(APIErrorType::Conflict, result.error()); } void RSSController::moveItemAction() @@ -82,9 +82,9 @@ void RSSController::moveItemAction() const QString itemPath = params()["itemPath"].trimmed(); const QString destPath = params()["destPath"].trimmed(); - QString error; - if (!RSS::Session::instance()->moveItem(itemPath, destPath, &error)) - throw APIError(APIErrorType::Conflict, error); + const nonstd::expected result = RSS::Session::instance()->moveItem(itemPath, destPath); + if (!result) + throw APIError(APIErrorType::Conflict, result.error()); } void RSSController::itemsAction() diff --git a/src/webui/api/torrentscontroller.cpp b/src/webui/api/torrentscontroller.cpp index 14e1c14dc..771cddf0e 100644 --- a/src/webui/api/torrentscontroller.cpp +++ b/src/webui/api/torrentscontroller.cpp @@ -701,14 +701,14 @@ void TorrentsController::addAction() for (auto it = data().constBegin(); it != data().constEnd(); ++it) { - const BitTorrent::TorrentInfo torrentInfo = BitTorrent::TorrentInfo::load(it.value()); - if (!torrentInfo.isValid()) + const nonstd::expected result = BitTorrent::TorrentInfo::load(it.value()); + if (!result) { throw APIError(APIErrorType::BadData , tr("Error: '%1' is not a valid torrent file.").arg(it.key())); } - partialSuccess |= BitTorrent::Session::instance()->addTorrent(torrentInfo, addTorrentParams); + partialSuccess |= BitTorrent::Session::instance()->addTorrent(result.value(), addTorrentParams); } if (partialSuccess) From 78459fcb310af49f8055f1e252268fae353d28a8 Mon Sep 17 00:00:00 2001 From: "Vladimir Golovnev (Glassez)" Date: Wed, 6 Oct 2021 21:53:56 +0300 Subject: [PATCH 2/2] Don't throw exception in TorrentInfo::saveToFile() --- src/base/bittorrent/session.cpp | 10 +++------- src/base/bittorrent/torrentinfo.cpp | 11 ++++++----- src/base/bittorrent/torrentinfo.h | 2 +- src/gui/addnewtorrentdialog.cpp | 10 +++------- 4 files changed, 13 insertions(+), 20 deletions(-) diff --git a/src/base/bittorrent/session.cpp b/src/base/bittorrent/session.cpp index 970032733..98599d0a0 100644 --- a/src/base/bittorrent/session.cpp +++ b/src/base/bittorrent/session.cpp @@ -69,7 +69,6 @@ #include #include "base/algorithm.h" -#include "base/exceptions.h" #include "base/global.h" #include "base/logger.h" #include "base/net/downloadmanager.h" @@ -2317,14 +2316,11 @@ void Session::exportTorrentFile(const TorrentInfo &torrentInfo, const QString &f newTorrentPath = exportDir.absoluteFilePath(torrentExportFilename); } - try - { - torrentInfo.saveToFile(newTorrentPath); - } - catch (const RuntimeError &err) + const nonstd::expected result = torrentInfo.saveToFile(newTorrentPath); + if (!result) { LogMsg(tr("Couldn't export torrent metadata file '%1'. Reason: %2.") - .arg(newTorrentPath, err.message()), Log::WARNING); + .arg(newTorrentPath, result.error()), Log::WARNING); } } } diff --git a/src/base/bittorrent/torrentinfo.cpp b/src/base/bittorrent/torrentinfo.cpp index 7a514b427..317fb9726 100644 --- a/src/base/bittorrent/torrentinfo.cpp +++ b/src/base/bittorrent/torrentinfo.cpp @@ -40,7 +40,6 @@ #include #include -#include "base/exceptions.h" #include "base/global.h" #include "base/utils/fs.h" #include "base/utils/io.h" @@ -153,10 +152,10 @@ nonstd::expected TorrentInfo::loadFromFile(const QString & return load(data); } -void TorrentInfo::saveToFile(const QString &path) const +nonstd::expected TorrentInfo::saveToFile(const QString &path) const { if (!isValid()) - throw RuntimeError {tr("Invalid metadata")}; + return nonstd::make_unexpected(tr("Invalid metadata")); try { @@ -164,12 +163,14 @@ void TorrentInfo::saveToFile(const QString &path) const const lt::entry torrentEntry = torrentCreator.generate(); const nonstd::expected result = Utils::IO::saveToFile(path, torrentEntry); if (!result) - throw RuntimeError(result.error()); + return result.get_unexpected(); } catch (const lt::system_error &err) { - throw RuntimeError(QString::fromLocal8Bit(err.what())); + return nonstd::make_unexpected(QString::fromLocal8Bit(err.what())); } + + return {}; } bool TorrentInfo::isValid() const diff --git a/src/base/bittorrent/torrentinfo.h b/src/base/bittorrent/torrentinfo.h index eb98d878c..a015fa758 100644 --- a/src/base/bittorrent/torrentinfo.h +++ b/src/base/bittorrent/torrentinfo.h @@ -58,7 +58,7 @@ namespace BitTorrent static nonstd::expected load(const QByteArray &data) noexcept; static nonstd::expected loadFromFile(const QString &path) noexcept; - void saveToFile(const QString &path) const; + nonstd::expected saveToFile(const QString &path) const; TorrentInfo &operator=(const TorrentInfo &other); diff --git a/src/gui/addnewtorrentdialog.cpp b/src/gui/addnewtorrentdialog.cpp index 7320e2647..2e3dc01e5 100644 --- a/src/gui/addnewtorrentdialog.cpp +++ b/src/gui/addnewtorrentdialog.cpp @@ -43,7 +43,6 @@ #include "base/bittorrent/magneturi.h" #include "base/bittorrent/session.h" #include "base/bittorrent/torrent.h" -#include "base/exceptions.h" #include "base/global.h" #include "base/net/downloadmanager.h" #include "base/settingsstorage.h" @@ -473,14 +472,11 @@ void AddNewTorrentDialog::saveTorrentFile() if (!path.endsWith(torrentFileExtension, Qt::CaseInsensitive)) path += torrentFileExtension; - try - { - m_torrentInfo.saveToFile(path); - } - catch (const RuntimeError &err) + const nonstd::expected result = m_torrentInfo.saveToFile(path); + if (!result) { QMessageBox::critical(this, tr("I/O Error") - , tr("Couldn't export torrent metadata file '%1'. Reason: %2.").arg(path, err.message())); + , tr("Couldn't export torrent metadata file '%1'. Reason: %2.").arg(path, result.error())); } }