No exceptions for error handling! ConnectedDevice::Init now returns bool, indicating success or failure.

As decreed by @hatstand.

src/CMakeLists.txt

@@ -122,7 +122,6 @@ set(SOURCES
 
   devices/connecteddevice.cpp
   devices/devicedatabasebackend.cpp
-  devices/deviceerror.cpp
   devices/devicelister.cpp
   devices/devicemanager.cpp
   devices/deviceproperties.cpp
@@ -459,7 +458,6 @@ set(HEADERS
 
   devices/connecteddevice.h
   devices/devicedatabasebackend.h
-  devices/deviceerror.h
   devices/devicelister.h
   devices/devicemanager.h
   devices/deviceproperties.h

src/devices/cddadevice.cpp

@@ -19,7 +19,6 @@
 
 #include <QUrl>
 
-#include "deviceerror.h"
 #include "library/librarybackend.h"
 #include "library/librarymodel.h"
 
@@ -42,26 +41,32 @@ CddaDevice::CddaDevice(const QUrl& url, DeviceLister* lister,
           SLOT(SongsLoaded(SongList)));
   connect(this, SIGNAL(SongsDiscovered(SongList)), model_,
           SLOT(SongsDiscovered(SongList)));
-  cdio_ = cdio_open(url_.path().toLocal8Bit().constData(), DRIVER_DEVICE);
-  if (!cdio_) {
-    throw DeviceError(url.toString(),
-                      "Cannot open device: cdio_open returned nullptr");
-  }
   connect(&disc_changed_timer_, SIGNAL(timeout()), SLOT(CheckDiscChanged()));
   WatchForDiscChanges(watch_for_disc_changes);
 }
 
 CddaDevice::~CddaDevice() {
-  Q_ASSERT(cdio_);
-  cdio_destroy(cdio_);
+  if (cdio_) {
+    cdio_destroy(cdio_);
+    cdio_ = nullptr;
+  }
 }
 
-void CddaDevice::Init() { LoadSongs(); }
+bool CddaDevice::Init() {
+  if (!cdio_) {
+    cdio_ = cdio_open(url_.path().toLocal8Bit().constData(), DRIVER_DEVICE);
+    if (!cdio_) return false;
+    LoadSongs();
+  }
+  return true;
+}
 
 CddaSongLoader* CddaDevice::loader() { return &cdda_song_loader_; }
 
 CdIo_t* CddaDevice::raw_cdio() { return cdio_; }
 
+bool CddaDevice::IsValid() const { return (cdio_ != nullptr); }
+
 void CddaDevice::WatchForDiscChanges(bool watch) {
   if (watch && !disc_changed_timer_.isActive())
     disc_changed_timer_.start(CddaDevice::kDiscChangePollingIntervalMs);
@@ -78,11 +83,12 @@ void CddaDevice::SongsLoaded(const SongList& songs) {
 }
 
 void CddaDevice::CheckDiscChanged() {
+  if (!cdio_) return;  // do nothing if not initialized
+
   // do nothing if loader is currently reading;
   // we'd just block until it's finished
   if (cdda_song_loader_.IsActive()) return;
 
-  Q_ASSERT(cdio_);
   if (cdio_get_media_changed(cdio_) == 1) {
     emit DiscChanged();
     song_count_ = 0;

src/devices/cddadevice.h

@@ -33,17 +33,25 @@ class CddaDevice : public ConnectedDevice {
   Q_OBJECT
 
  public:
+  // Initializes the CddaDevice but does not open a handle to the device.
+  // Using code MUST call Init() after construction before calling and
+  // check that it returns true any other methods, to ensure that a valid
+  // device handle was correctly opened. Using class methods without
+  // a valid device handle is undefined behavior and might result in crashes.
   Q_INVOKABLE CddaDevice(const QUrl& url, DeviceLister* lister,
                          const QString& unique_id, DeviceManager* manager,
                          Application* app, int database_id, bool first_time,
                          bool watch_for_disc_changes = true);
   ~CddaDevice();
 
-  void Init() override;
+  bool Init() override;
   bool CopyToStorage(const MusicStorage::CopyJob&) { return false; }
   bool DeleteFromStorage(const MusicStorage::DeleteJob&) { return false; }
   CddaSongLoader* loader();
+  // Access to the raw cdio device handle.
   CdIo_t* raw_cdio();  // TODO: not ideal, but Ripper needs this currently
+  // Check whether a valid device handle was opened.
+  bool IsValid() const;
   void WatchForDiscChanges(bool watch);
 
   static const int kDiscChangePollingIntervalMs;

src/devices/connecteddevice.h

@@ -44,7 +44,7 @@ class ConnectedDevice : public QObject,
                   Application* app, int database_id, bool first_time);
   ~ConnectedDevice();
 
-  virtual void Init() = 0;
+  virtual bool Init() = 0;
   virtual void ConnectAsync();
 
   virtual TranscodeMode GetTranscodeMode() const;

src/devices/deviceerror.cpp

@@ -1,37 +0,0 @@
-/* This file is part of Clementine.
-   Copyright 2021, Lukas Prediger <lumip@lumip.de>
-
-   Clementine is free software: you can redistribute it and/or modify
-   it under the terms of the GNU General Public License as published by
-   the Free Software Foundation, either version 3 of the License, or
-   (at your option) any later version.
-
-   Clementine is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-   GNU General Public License for more details.
-
-   You should have received a copy of the GNU General Public License
-   along with Clementine.  If not, see <http://www.gnu.org/licenses/>.
-*/
-
-#include "deviceerror.h"
-
-#include <QString>
-
-std::string DeviceError::CreateExceptionMessage(const QString& device_id,
-                                                const QString error_message) {
-  QString exception_message =
-      QString("Error accessing device <%1>").arg(device_id);
-  if (!error_message.isEmpty()) exception_message += ": " + error_message;
-
-  return exception_message.toLocal8Bit().toStdString();
-}
-
-DeviceError::DeviceError(const QString& device_id, const QString& error_message)
-    : std::runtime_error(CreateExceptionMessage(device_id, error_message)) {}
-
-DeviceError::DeviceError(const QString& device_id)
-    : DeviceError(device_id, "") {}
-
-DeviceError::~DeviceError() {}

src/devices/deviceerror.h

@@ -1,36 +0,0 @@
-/* This file is part of Clementine.
-   Copyright 2021, Lukas Prediger <lumip@lumip.de>
-
-   Clementine is free software: you can redistribute it and/or modify
-   it under the terms of the GNU General Public License as published by
-   the Free Software Foundation, either version 3 of the License, or
-   (at your option) any later version.
-
-   Clementine is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-   GNU General Public License for more details.
-
-   You should have received a copy of the GNU General Public License
-   along with Clementine.  If not, see <http://www.gnu.org/licenses/>.
-*/
-
-#ifndef DEVICEERROR_H
-#define DEVICEERROR_H
-
-#include <stdexcept>
-
-class QString;
-
-class DeviceError : public std::runtime_error {
- private:
-  static std::string CreateExceptionMessage(const QString& device_id,
-                                            const QString error_message);
-
- public:
-  DeviceError(const QString& device_id);
-  DeviceError(const QString& device_id, const QString& error_message);
-  virtual ~DeviceError();
-};
-
-#endif

src/devices/devicemanager.cpp

@@ -37,7 +37,6 @@
 #include "core/taskmanager.h"
 #include "core/utilities.h"
 #include "devicedatabasebackend.h"
-#include "deviceerror.h"
 #include "deviceinfo.h"
 #include "devicestatefiltermodel.h"
 #include "filesystemdevice.h"
@@ -566,24 +565,24 @@ std::shared_ptr<ConnectedDevice> DeviceManager::Connect(DeviceInfo* info) {
     return ret;
   }
 
-  try {
-    QMetaObject meta_object = device_classes_.value(device_url.scheme());
-    QObject* instance = meta_object.newInstance(
-        Q_ARG(QUrl, device_url),
-        Q_ARG(DeviceLister*, info->BestBackend()->lister_),
-        Q_ARG(QString, info->BestBackend()->unique_id_),
-        Q_ARG(DeviceManager*, this), Q_ARG(Application*, app_),
-        Q_ARG(int, info->database_id_), Q_ARG(bool, first_time));
-    ret.reset(static_cast<ConnectedDevice*>(instance));
-  } catch (const DeviceError& e) {
-    qLog(Warning) << "Could not create device: " << e.what();
-    return ret;
-  }
+  QMetaObject meta_object = device_classes_.value(device_url.scheme());
+  QObject* instance = meta_object.newInstance(
+      Q_ARG(QUrl, device_url),
+      Q_ARG(DeviceLister*, info->BestBackend()->lister_),
+      Q_ARG(QString, info->BestBackend()->unique_id_),
+      Q_ARG(DeviceManager*, this), Q_ARG(Application*, app_),
+      Q_ARG(int, info->database_id_), Q_ARG(bool, first_time));
+  ret.reset(static_cast<ConnectedDevice*>(instance));
 
   if (!ret) {
     qLog(Warning) << "Could not create device for" << device_url.toString();
   } else {
-    ret->Init();
+    if (!ret->Init()) {
+      qLog(Warning) << "Could not initialize device for "
+                    << device_url.toString();
+      ret.reset();
+      return ret;
+    }
 
     info->device_ = ret;
     QModelIndex idx = ItemToIndex(info);

src/devices/filesystemdevice.cpp

@@ -69,9 +69,10 @@ FilesystemDevice::FilesystemDevice(const QUrl& url, DeviceLister* lister,
   connect(watcher_, &LibraryWatcher::Error, app, &Application::AddError);
 }
 
-void FilesystemDevice::Init() {
+bool FilesystemDevice::Init() {
   InitBackendDirectory(url_.toLocalFile(), first_time_);
   model_->Init();
+  return true;
 }
 
 FilesystemDevice::~FilesystemDevice() {

src/devices/filesystemdevice.h

@@ -35,7 +35,7 @@ class FilesystemDevice : public ConnectedDevice,
                                bool first_time);
   ~FilesystemDevice();
 
-  void Init();
+  bool Init() override;
 
   static QStringList url_schemes() { return QStringList() << "file"; }
 

src/devices/gpoddevice.cpp

@@ -40,7 +40,7 @@ GPodDevice::GPodDevice(const QUrl& url, DeviceLister* lister,
       loader_(nullptr),
       db_(nullptr) {}
 
-void GPodDevice::Init() {
+bool GPodDevice::Init() {
   InitBackendDirectory(url_.path(), first_time_);
   model_->Init();
 
@@ -54,6 +54,7 @@ void GPodDevice::Init() {
           SLOT(LoadFinished(Itdb_iTunesDB*)));
   connect(loader_thread_, SIGNAL(started()), loader_, SLOT(LoadDatabase()));
   loader_thread_->start();
+  return true;
 }
 
 GPodDevice::~GPodDevice() {}

src/devices/gpoddevice.h

@@ -37,7 +37,7 @@ class GPodDevice : public ConnectedDevice, public virtual MusicStorage {
                          Application* app, int database_id, bool first_time);
   ~GPodDevice();
 
-  void Init();
+  bool Init() override;
 
   static QStringList url_schemes() { return QStringList() << "ipod"; }
 

src/devices/mtpdevice.cpp

@@ -47,7 +47,7 @@ MtpDevice::MtpDevice(const QUrl& url, DeviceLister* lister,
 
 MtpDevice::~MtpDevice() {}
 
-void MtpDevice::Init() {
+bool MtpDevice::Init() {
   InitBackendDirectory("/", first_time_, false);
   model_->Init();
 
@@ -59,6 +59,8 @@ void MtpDevice::Init() {
   connect(loader_, SIGNAL(TaskStarted(int)), SIGNAL(TaskStarted(int)));
   connect(loader_, SIGNAL(LoadFinished(bool)), SLOT(LoadFinished(bool)));
   connect(loader_thread_, SIGNAL(started()), loader_, SLOT(LoadDatabase()));
+
+  return true;
 }
 
 void MtpDevice::ConnectAsync() {

src/devices/mtpdevice.h

@@ -43,7 +43,7 @@ class MtpDevice : public ConnectedDevice {
                          << "gphoto2";
   }
 
-  void Init();
+  bool Init() override;
   void ConnectAsync();
 
   bool GetSupportedFiletypes(QList<Song::FileType>* ret);