Add error handling path for async song loading.

Async song loading can fail without user feedback. This change adds return codes
to these async load functions. It will now produce an error dialog in simple
scenarios (test case is user selecting a file that is not readable). Other cases,
such as directories and playlists, aren't yet covered.

src/core/songloader.cpp

@@ -130,9 +130,12 @@ SongLoader::Result SongLoader::Load(const QUrl& url) {
   return BlockingLoadRequired;
 }
 
-void SongLoader::LoadFilenamesBlocking() {
+SongLoader::Result SongLoader::LoadFilenamesBlocking() {
   if (preload_func_) {
-    preload_func_();
+    return preload_func_();
+  } else {
+    qLog(Error) << "Preload function was not set for blocking operation";
+    return Error;
   }
 }
 
@@ -207,18 +210,21 @@ SongLoader::Result SongLoader::LoadLocal(const QString& filename) {
   return BlockingLoadRequired;
 }
 
-void SongLoader::LoadLocalAsync(const QString& filename) {
+SongLoader::Result SongLoader::LoadLocalAsync(const QString& filename) {
   // First check to see if it's a directory - if so we will load all the songs
   // inside right away.
   if (QFileInfo(filename).isDir()) {
     LoadLocalDirectory(filename);
-    return;
+    return Success;
   }
 
   // It's a local file, so check if it looks like a playlist.
   // Read the first few bytes.
   QFile file(filename);
-  if (!file.open(QIODevice::ReadOnly)) return;
+  if (!file.open(QIODevice::ReadOnly)) {
+    qLog(Error) << "Could not open file " << filename;
+    return Error;
+  }
   QByteArray data(file.read(PlaylistParser::kMagicSize));
 
   ParserBase* parser = playlist_parser_->ParserForMagic(data);
@@ -234,7 +240,7 @@ void SongLoader::LoadLocalAsync(const QString& filename) {
 
     // It's a playlist!
     LoadPlaylist(parser, filename);
-    return;
+    return Success;
   }
 
   // Check if it's a cue file
@@ -249,7 +255,7 @@ void SongLoader::LoadLocalAsync(const QString& filename) {
     for (Song song : song_list) {
       if (song.is_valid()) songs_ << song;
     }
-    return;
+    return Success;
   }
 
   // Assume it's just a normal file
@@ -257,6 +263,9 @@ void SongLoader::LoadLocalAsync(const QString& filename) {
   song.InitFromFilePartial(filename);
   if (song.is_valid()) {
     songs_ << song;
+    return Success;
+  } else {
+    return Error;
   }
 }
 
@@ -374,7 +383,7 @@ void SongLoader::StopTypefind() {
   emit LoadRemoteFinished();
 }
 
-void SongLoader::LoadRemote() {
+SongLoader::Result SongLoader::LoadRemote() {
   qLog(Debug) << "Loading remote file" << url_;
 
   // It's not a local file so we have to fetch it to see what it is.  We use
@@ -399,7 +408,7 @@ void SongLoader::LoadRemote() {
   if (!source) {
     qLog(Warning) << "Couldn't create gstreamer source element for"
                   << url_.toString();
-    return;
+    return Error;
   }
 
   // Create the other elements and link them up
@@ -430,6 +439,7 @@ void SongLoader::LoadRemote() {
 
   // Wait until loading is finished
   loop.exec();
+  return Success;
 }
 
 void SongLoader::TypeFound(GstElement*, uint, GstCaps* caps, void* self) {

src/core/songloader.h

@@ -72,7 +72,7 @@ class SongLoader : public QObject {
   // Loads the files with only filenames. When finished, songs() contains a
   // complete list of all Song objects, but without metadata. This method is
   // blocking, do not call it from the UI thread.
-  void LoadFilenamesBlocking();
+  Result LoadFilenamesBlocking();
   // Completely load songs previously loaded with LoadFilenamesBlocking(). When
   // finished, the Song objects in songs() contain metadata now. This method is
   // blocking, do not call it from the UI thread.
@@ -101,7 +101,7 @@ signals:
   };
 
   Result LoadLocal(const QString& filename);
-  void LoadLocalAsync(const QString& filename);
+  Result LoadLocalAsync(const QString& filename);
   void EffectiveSongLoad(Song* song);
   Result LoadLocalPartial(const QString& filename);
   void LoadLocalDirectory(const QString& filename);
@@ -109,7 +109,7 @@ signals:
 
   void AddAsRawStream();
 
-  void LoadRemote();
+  Result LoadRemote();
   bool LoadRemotePlaylist(const QUrl& url);
 
   // GStreamer callbacks
@@ -138,7 +138,7 @@ signals:
   CueParser* cue_parser_;
 
   // For async loads
-  std::function<void()> preload_func_;
+  std::function<Result()> preload_func_;
   int timeout_;
   State state_;
   bool success_;

src/playlist/songloaderinserter.cpp

@@ -134,15 +134,23 @@ void SongLoaderInserter::AsyncLoad() {
   int async_load_id = task_manager_->StartTask(tr("Loading tracks"));
   task_manager_->SetTaskProgress(async_load_id, async_progress,
                                  pending_.count());
+  bool first_loaded = false;
   for (int i = 0; i < pending_.count(); ++i) {
     SongLoader* loader = pending_[i];
-    loader->LoadFilenamesBlocking();
+    SongLoader::Result res = loader->LoadFilenamesBlocking();
+
     task_manager_->SetTaskProgress(async_load_id, ++async_progress);
-    if (i == 0) {
+
+    if (res == SongLoader::Error) {
+      emit Error(tr("Error loading %1").arg(loader->url().toString()));
+      continue;
+    }
+    if (!first_loaded) {
       // Load everything from the first song.  It'll start playing as soon as
       // we emit PreloadFinished, so it needs to have the duration set to show
       // properly in the UI.
       loader->LoadMetadataBlocking();
+      first_loaded = true;
     }
     songs_ << loader->songs();
   }