fix(attachments): Properly handle HTTP errors in addition to socket errors #2136

src/flux/stores/file-download-store.coffee

@@ -54,10 +54,26 @@ class Download
     # initiate multiple downloads for the same file
     return @promise if @promise
 
+    # Note: we must resolve or reject with `this`
     @promise = new Promise (resolve, reject) =>
       stream = fs.createWriteStream(@targetPath)
       @state = State.Downloading
 
+      onFailed = (err) =>
+        @request = null
+        stream.end()
+        @state = State.Failed
+        if fs.existsSync(@targetPath)
+          fs.unlinkSync(@targetPath)
+        reject(err)
+
+      onSuccess = =>
+        @request = null
+        stream.end()
+        @state = State.Finished
+        @percent = 100
+        resolve(@)
+
       NylasAPI.makeRequest
         json: false
         path: "/files/#{@fileId}/download"
@@ -70,21 +86,19 @@ class Download
             @percent = progress.percent
             @progressCallback()
 
-          .on "error", (err) =>
+          # This is a /socket/ error event, not an HTTP error event. It fires
-            @request = null
+          # when the conn is dropped, user if offline, but not on HTTP status codes.
-            @state = State.Failed
+          # It is sometimes called in place of "end", not before or after.
-            stream.end()
+          .on("error", onFailed)
-            if fs.existsSync(@targetPath)
-              fs.unlinkSync(@targetPath)
-            reject(@)
 
           .on "end", =>
             return if @state is State.Failed
-            @request = null
+
-            @state = State.Finished
+            statusCode = @request.response?.statusCode
-            @percent = 100
+            if [200, 202, 204].includes(statusCode)
-            stream.end()
+              onSuccess()
-            resolve(@) # Note: we must resolve with this
+            else
+              onFailed(new Error("Server returned a #{statusCode}"))
 
           .pipe(stream)