Better error handling for downloads

app/src/main/java/eu/kanade/mangafeed/data/download/DownloadManager.java

@@ -168,7 +168,7 @@ public class DownloadManager {
         try {
             DiskUtils.createDirectory(download.directory);
         } catch (IOException e) {
-            Timber.e(e.getMessage());
+            return Observable.error(e);
         }
 
         Observable<List<Page>> pageListObservable = download.pages == null ?
@@ -182,34 +182,35 @@ public class DownloadManager {
 
         return pageListObservable
                 .subscribeOn(Schedulers.io())
-                .doOnNext(pages -> download.downloadedImages = 0)
+                .doOnError(error -> download.setStatus(Download.ERROR))
                 .doOnNext(pages -> download.setStatus(Download.DOWNLOADING))
+                .doOnNext(pages -> download.downloadedImages = 0)
                 // Get all the URLs to the source images, fetch pages if necessary
-                .flatMap(pageList -> Observable.from(pageList)
+                .flatMap(download.source::getAllImageUrlsFromPageList)
-                        .filter(page -> page.getImageUrl() != null)
-                        .mergeWith(download.source.getRemainingImageUrlsFromPageList(pageList)))
                 // Start downloading images, consider we can have downloaded images already
-                .concatMap(page -> getDownloadedImage(page, download.source, download.directory))
+                .concatMap(page -> getOrDownloadImage(page, download))
-                .doOnNext(p -> download.downloadedImages++)
                 // Do after download completes
                 .doOnCompleted(() -> onDownloadCompleted(download))
                 .toList()
-                .flatMap(pages -> Observable.just(areAllDownloadsFinished()));
+                .flatMap(pages -> Observable.just(download))
+                // If the page list threw, it will resume here
+                .onErrorResumeNext(error -> Observable.just(download))
+                .map(d -> areAllDownloadsFinished());
     }
 
-    // Get downloaded image if exists, otherwise download it with the method below
+    // Get the image from the filesystem if it exists or download from network
-    public Observable<Page> getDownloadedImage(final Page page, Source source, File chapterDir) {
+    private Observable<Page> getOrDownloadImage(final Page page, Download download) {
-        Observable<Page> pageObservable = Observable.just(page);
+        // If the image URL is empty, do nothing
         if (page.getImageUrl() == null)
-            return pageObservable;
+            return Observable.just(page);
 
-        String imageFilename = getImageFilename(page);
+        String filename = getImageFilename(page);
-        File imagePath = new File(chapterDir, imageFilename);
+        File imagePath = new File(download.directory, filename);
 
-        if (!isImageDownloaded(imagePath)) {
+        // If the image is already downloaded, do nothing. Otherwise download from network
-            page.setStatus(Page.DOWNLOAD_IMAGE);
+        Observable<Page> pageObservable = isImageDownloaded(imagePath) ?
-            pageObservable = downloadImage(page, source, chapterDir, imageFilename);
+                Observable.just(page) :
-        }
+                downloadImage(page, download.source, download.directory, filename);
 
         return pageObservable
                 // When the image is ready, set image path, progress (just in case) and status
@@ -217,6 +218,7 @@ public class DownloadManager {
                     p.setImagePath(imagePath.getAbsolutePath());
                     p.setProgress(100);
                     p.setStatus(Page.READY);
+                    download.downloadedImages++;
                 })
                 // If the download fails, mark this page as error
                 .doOnError(e -> page.setStatus(Page.ERROR))
@@ -224,20 +226,39 @@ public class DownloadManager {
                 .onErrorResumeNext(e -> Observable.just(page));
     }
 
-    // Download the image and save it to the filesystem
+    private Observable<Page> downloadImage(Page page, Source source, File directory, String filename) {
-    private Observable<Page> downloadImage(final Page page, Source source, File chapterDir, String imageFilename) {
+        page.setStatus(Page.DOWNLOAD_IMAGE);
         return source.getImageProgressResponse(page)
                 .flatMap(resp -> {
                     try {
-                        DiskUtils.saveBufferedSourceToDirectory(resp.body().source(), chapterDir, imageFilename);
+                        DiskUtils.saveBufferedSourceToDirectory(resp.body().source(), directory, filename);
-                    } catch (IOException e) {
+                    } catch (Exception e) {
-                        Timber.e(e.fillInStackTrace(), e.getMessage());
+                        return Observable.error(e);
-                        throw new IllegalStateException("Unable to save image");
                     }
                     return Observable.just(page);
                 });
     }
 
+    // Public method to get the image from the filesystem. It does NOT provide any way to download the iamge
+    public Observable<Page> getDownloadedImage(final Page page, File chapterDir) {
+        if (page.getImageUrl() == null) {
+            page.setStatus(Page.ERROR);
+            return Observable.just(page);
+        }
+
+        File imagePath = new File(chapterDir, getImageFilename(page));
+
+        // When the image is ready, set image path, progress (just in case) and status
+        if (isImageDownloaded(imagePath)) {
+            page.setImagePath(imagePath.getAbsolutePath());
+            page.setProgress(100);
+            page.setStatus(Page.READY);
+        } else {
+            page.setStatus(Page.ERROR);
+        }
+        return Observable.just(page);
+    }
+
     // Get the filename for an image given the page
     private String getImageFilename(Page page) {
         String url;

app/src/main/java/eu/kanade/mangafeed/data/source/base/Source.java

@@ -97,6 +97,12 @@ public abstract class Source extends BaseSource {
                 });
     }
 
+    public Observable<Page> getAllImageUrlsFromPageList(final List<Page> pages) {
+        return Observable.from(pages)
+                .filter(page -> page.getImageUrl() != null)
+                .mergeWith(getRemainingImageUrlsFromPageList(pages));
+    }
+
     // Get the URLs of the images of a chapter
     public Observable<Page> getRemainingImageUrlsFromPageList(final List<Page> pages) {
         return Observable.from(pages)

app/src/main/java/eu/kanade/mangafeed/ui/reader/ReaderPresenter.java

@@ -149,14 +149,12 @@ public class ReaderPresenter extends BasePresenter<ReaderActivity> {
         Observable<Page> pageObservable;
 
         if (!isDownloaded) {
-            pageObservable = Observable.from(pageList)
+            pageObservable = source.getAllImageUrlsFromPageList(pageList)
-                    .filter(page -> page.getImageUrl() != null)
-                    .mergeWith(source.getRemainingImageUrlsFromPageList(pageList))
                     .flatMap(source::getCachedImage, 3);
         } else {
             File chapterDir = downloadManager.getAbsoluteChapterDirectory(source, manga, chapter);
             pageObservable = Observable.from(pageList)
-                    .flatMap(page -> downloadManager.getDownloadedImage(page, source, chapterDir));
+                    .flatMap(page -> downloadManager.getDownloadedImage(page, chapterDir));
         }
         return pageObservable
                 .subscribeOn(Schedulers.io())

app/src/main/java/eu/kanade/mangafeed/util/DiskUtils.java

@@ -134,8 +134,9 @@ public final class DiskUtils {
         } catch (Exception e) {
             if (bufferedSink != null) {
                 bufferedSink.close();
-                writeFile.delete();
             }
+            writeFile.delete();
+            throw new IOException("Failed saving image");
         }
 
         return writeFile;