From 3e8f7fbfd74fbd71a882a38fc2422e3b8dc81022 Mon Sep 17 00:00:00 2001
From: Ben Gotow <bengotow@gmail.com>
Date: Tue, 8 Mar 2016 18:32:39 -0800
Subject: [PATCH] fix(downloads): Properly handle network errors, retries of
 downloads

---
 spec/stores/file-download-store-spec.coffee |  1 -
 src/flux/stores/file-download-store.coffee  | 56 +++++++++------------
 2 files changed, 23 insertions(+), 34 deletions(-)

diff --git a/spec/stores/file-download-store-spec.coffee b/spec/stores/file-download-store-spec.coffee
index 696419a72..8dd7e69f9 100644
--- a/spec/stores/file-download-store-spec.coffee
+++ b/spec/stores/file-download-store-spec.coffee
@@ -134,7 +134,6 @@ describe "FileDownloadStore", ->
     beforeEach ->
       spyOn(FileDownloadStore.Download.prototype, 'run').andCallFake -> Promise.resolve(@)
       spyOn(FileDownloadStore, '_prepareFolder').andCallFake -> Promise.resolve(true)
-      spyOn(FileDownloadStore, '_cleanupDownload')
 
     it "should make sure that the download file path exists", ->
       FileDownloadStore._runDownload(@testfile)
diff --git a/src/flux/stores/file-download-store.coffee b/src/flux/stores/file-download-store.coffee
index 16134e82f..68bc5f0ef 100644
--- a/src/flux/stores/file-download-store.coffee
+++ b/src/flux/stores/file-download-store.coffee
@@ -56,19 +56,6 @@ class Download
 
     @promise = new Promise (resolve, reject) =>
       stream = fs.createWriteStream(@targetPath)
-
-      # We need to watch the request for `success` or `error`, but not fire
-      # a callback until the stream has ended. This helper functions ensure
-      # that resolve or reject is only fired once the stream has been closed.
-      checkIfFinished = (action) =>
-        # Wait for the stream to finish writing to disk and clear the request
-        return if @request
-
-        if @state is State.Finished
-          resolve(@) # Note: we must resolve with this
-        else if @state is State.Failed
-          reject(@)
-
       @state = State.Downloading
 
       NylasAPI.makeRequest
@@ -82,22 +69,25 @@ class Download
           .on "progress", (progress) =>
             @percent = progress.percent
             @progressCallback()
-          .on "end", =>
+
+          .on "error", (err) =>
             @request = null
-            checkIfFinished()
+            @state = State.Failed
+            stream.end()
+            if fs.existsSync(@targetPath)
+              fs.unlinkSync(@targetPath)
+            reject(@)
+
+          .on "end", =>
+            return if @state is State.Failed
+            @request = null
+            @state = State.Finished
+            @percent = 100
+            stream.end()
+            resolve(@) # Note: we must resolve with this
+
           .pipe(stream)
 
-        success: =>
-          # At this point, the file stream has not finished writing to disk.
-          # Don't resolve yet, or the browser will load only part of the image.
-          @state = State.Finished
-          @percent = 100
-          checkIfFinished()
-
-        error: =>
-          @state = State.Failed
-          checkIfFinished()
-
   ensureClosed: ->
     @request?.abort()
 
@@ -180,7 +170,10 @@ FileDownloadStore = Reflux.createStore
           @_downloads[file.id] = download
           @trigger()
           return download.run().finally =>
-            @_cleanupDownload(download)
+            download.ensureClosed()
+            if download.state is State.Failed
+              delete @_downloads[file.id]
+            @trigger()
 
   # Returns a promise that resolves with true or false. True if the file has
   # been downloaded, false if it should be downloaded.
@@ -213,7 +206,7 @@ FileDownloadStore = Reflux.createStore
     defaultPath = @_defaultSavePath(file)
     defaultExtension = path.extname(defaultPath)
 
-    NylasEnv.showSaveDialog {defaultPath: defaultPath}, (savePath) =>
+    NylasEnv.showSaveDialog {defaultPath}, (savePath) =>
       return unless savePath
       NylasEnv.savedState.lastDownloadDirectory = path.dirname(savePath)
 
@@ -234,16 +227,13 @@ FileDownloadStore = Reflux.createStore
   _abortFetchFile: (file) ->
     download = @_downloads[file.id]
     return unless download
-    @_cleanupDownload(download)
+    download.ensureClosed()
+    @trigger()
 
     downloadPath = @pathForFile(file)
     fs.exists downloadPath, (exists) ->
       fs.unlink(downloadPath) if exists
 
-  _cleanupDownload: (download) ->
-    download.ensureClosed()
-    @trigger()
-
   _defaultSavePath: (file) ->
     if process.platform is 'win32'
       home = process.env.USERPROFILE