Merge pull request #5424 from atom/ks-handle-save-errors-in-pane

Show custom notifications for more save errors

spec/config-spec.coffee

@@ -666,6 +666,23 @@ describe "Config", ->
               foo:
                 bar: 'coffee'
 
+      describe "when an error is thrown writing the file to disk", ->
+        addErrorHandler = null
+        beforeEach ->
+          atom.notifications.onDidAddNotification addErrorHandler = jasmine.createSpy()
+
+        it "creates a notification", ->
+          jasmine.unspy CSON, 'writeFileSync'
+          spyOn(CSON, 'writeFileSync').andCallFake ->
+            error = new Error()
+            error.code = 'EPERM'
+            error.path = atom.config.getUserConfigPath()
+            throw error
+
+          save = -> atom.config.save()
+          expect(save).not.toThrow()
+          expect(addErrorHandler.callCount).toBe 1
+
     describe ".loadUserConfig()", ->
       beforeEach ->
         expect(fs.existsSync(atom.config.configDirPath)).toBeFalsy()

spec/pane-spec.coffee

@@ -383,6 +383,25 @@ describe "Pane", ->
           pane.saveActiveItem()
           expect(atom.showSaveDialogSync).not.toHaveBeenCalled()
 
+    describe "when the item's saveAs method throws a well-known IO error", ->
+      notificationSpy = null
+      beforeEach ->
+        atom.notifications.onDidAddNotification notificationSpy = jasmine.createSpy()
+
+      it "creates a notification", ->
+        pane.getActiveItem().saveAs = ->
+          error = new Error("EACCES, permission denied '/foo'")
+          error.path = '/foo'
+          error.code = 'EACCES'
+          throw error
+
+        pane.saveActiveItem()
+        expect(notificationSpy).toHaveBeenCalled()
+        notification = notificationSpy.mostRecentCall.args[0]
+        expect(notification.getType()).toBe 'warning'
+        expect(notification.getMessage()).toContain 'Permission denied'
+        expect(notification.getMessage()).toContain '/foo'
+
   describe "::saveActiveItemAs()", ->
     pane = null
 
@@ -404,6 +423,25 @@ describe "Pane", ->
         pane.saveActiveItemAs()
         expect(atom.showSaveDialogSync).not.toHaveBeenCalled()
 
+    describe "when the item's saveAs method throws a well-known IO error", ->
+      notificationSpy = null
+      beforeEach ->
+        atom.notifications.onDidAddNotification notificationSpy = jasmine.createSpy()
+
+      it "creates a notification", ->
+        pane.getActiveItem().saveAs = ->
+          error = new Error("EACCES, permission denied '/foo'")
+          error.path = '/foo'
+          error.code = 'EACCES'
+          throw error
+
+        pane.saveActiveItemAs()
+        expect(notificationSpy).toHaveBeenCalled()
+        notification = notificationSpy.mostRecentCall.args[0]
+        expect(notification.getType()).toBe 'warning'
+        expect(notification.getMessage()).toContain 'Permission denied'
+        expect(notification.getMessage()).toContain '/foo'
+
   describe "::itemForURI(uri)", ->
     it "returns the item for which a call to .getURI() returns the given uri", ->
       pane = new Pane(items: [new Item("A"), new Item("B"), new Item("C"), new Item("D")])

spec/workspace-spec.coffee

@@ -955,9 +955,14 @@ describe "Workspace", ->
           expect(editor.isModified()).toBeTruthy()
 
   describe "::saveActivePaneItem()", ->
+    editor = null
+    beforeEach ->
+      waitsForPromise ->
+        atom.workspace.open('sample.js').then (o) -> editor = o
+
     describe "when there is an error", ->
       it "emits a warning notification when the file cannot be saved", ->
-        spyOn(Pane::, 'saveActiveItem').andCallFake ->
+        spyOn(editor, 'save').andCallFake ->
           throw new Error("'/some/file' is a directory")
 
         atom.notifications.onDidAddNotification addedSpy = jasmine.createSpy()
@@ -966,7 +971,7 @@ describe "Workspace", ->
         expect(addedSpy.mostRecentCall.args[0].getType()).toBe 'warning'
 
       it "emits a warning notification when the directory cannot be written to", ->
-        spyOn(Pane::, 'saveActiveItem').andCallFake ->
+        spyOn(editor, 'save').andCallFake ->
           throw new Error("ENOTDIR, not a directory '/Some/dir/and-a-file.js'")
 
         atom.notifications.onDidAddNotification addedSpy = jasmine.createSpy()
@@ -975,7 +980,7 @@ describe "Workspace", ->
         expect(addedSpy.mostRecentCall.args[0].getType()).toBe 'warning'
 
       it "emits a warning notification when the user does not have permission", ->
-        spyOn(Pane::, 'saveActiveItem').andCallFake ->
+        spyOn(editor, 'save').andCallFake ->
           error = new Error("EACCES, permission denied '/Some/dir/and-a-file.js'")
           error.code = 'EACCES'
           error.path = '/Some/dir/and-a-file.js'
@@ -987,14 +992,14 @@ describe "Workspace", ->
         expect(addedSpy.mostRecentCall.args[0].getType()).toBe 'warning'
 
       it "emits a warning notification when the operation is not permitted", ->
-        spyOn(Pane::, 'saveActiveItem').andCallFake ->
+        spyOn(editor, 'save').andCallFake ->
           error = new Error("EPERM, operation not permitted '/Some/dir/and-a-file.js'")
           error.code = 'EPERM'
           error.path = '/Some/dir/and-a-file.js'
           throw error
 
       it "emits a warning notification when the file is already open by another app", ->
-        spyOn(Pane::, 'saveActiveItem').andCallFake ->
+        spyOn(editor, 'save').andCallFake ->
           error = new Error("EBUSY, resource busy or locked '/Some/dir/and-a-file.js'")
           error.code = 'EBUSY'
           error.path = '/Some/dir/and-a-file.js'
@@ -1009,7 +1014,7 @@ describe "Workspace", ->
         expect(notificaiton.getMessage()).toContain 'Unable to save'
 
       it "emits a warning notification when the file system is read-only", ->
-        spyOn(Pane::, 'saveActiveItem').andCallFake ->
+        spyOn(editor, 'save').andCallFake ->
           error = new Error("EROFS, read-only file system '/Some/dir/and-a-file.js'")
           error.code = 'EROFS'
           error.path = '/Some/dir/and-a-file.js'
@@ -1024,7 +1029,7 @@ describe "Workspace", ->
         expect(notification.getMessage()).toContain 'Unable to save'
 
       it "emits a warning notification when the file cannot be saved", ->
-        spyOn(Pane::, 'saveActiveItem').andCallFake ->
+        spyOn(editor, 'save').andCallFake ->
           throw new Error("no one knows")
 
         save = -> atom.workspace.saveActivePaneItem()

src/config.coffee

@@ -868,7 +868,12 @@ class Config
   save: ->
     allSettings = {'*': @settings}
     allSettings = _.extend allSettings, @scopedSettingsStore.propertiesForSource(@getUserConfigPath())
+    try
       CSON.writeFileSync(@configFilePath, allSettings)
+    catch error
+      message = "Failed to save `#{path.basename(@configFilePath)}`"
+      detail = error.message
+      @notifyFailure(message, detail)
 
   ###
   Section: Private methods managing global settings

src/pane.coffee

@@ -481,7 +481,10 @@ class Pane extends Model
       itemURI = item.getUri()
 
     if itemURI?
+      try
         item.save?()
+      catch error
+        @handleSaveError(error)
       nextAction?()
     else
       @saveItemAs(item, nextAction)
@@ -498,7 +501,10 @@ class Pane extends Model
     itemPath = item.getPath?()
     newItemPath = atom.showSaveDialogSync(itemPath)
     if newItemPath
+      try
         item.saveAs(newItemPath)
+      catch error
+        @handleSaveError(error)
       nextAction?()
 
   # Public: Save all items.
@@ -667,3 +673,18 @@ class Pane extends Model
     for item in @getItems()
       return false unless @promptToSaveItem(item)
     true
+
+  handleSaveError: (error) ->
+    if error.message.endsWith('is a directory')
+      atom.notifications.addWarning("Unable to save file: #{error.message}")
+    else if error.code is 'EACCES' and error.path?
+      atom.notifications.addWarning("Unable to save file: Permission denied '#{error.path}'")
+    else if error.code in ['EPERM', 'EBUSY', 'UNKNOWN'] and error.path?
+      atom.notifications.addWarning("Unable to save file '#{error.path}'", detail: error.message)
+    else if error.code is 'EROFS' and error.path?
+      atom.notifications.addWarning("Unable to save file: Read-only file system '#{error.path}'")
+    else if errorMatch = /ENOTDIR, not a directory '([^']+)'/.exec(error.message)
+      fileName = errorMatch[1]
+      atom.notifications.addWarning("Unable to save file: A directory in the path '#{fileName}' could not be written to")
+    else
+      throw error

src/workspace.coffee

@@ -609,7 +609,7 @@ class Workspace extends Model
   # {::saveActivePaneItemAs} # will be called instead. This method does nothing
   # if the active item does not implement a `.save` method.
   saveActivePaneItem: ->
-    @saveActivePaneItemAndReportErrors('saveActiveItem')
+    @getActivePane().saveActiveItem()
 
   # Prompt the user for a path and save the active pane item to it.
   #
@@ -617,27 +617,7 @@ class Workspace extends Model
   # `.saveAs` on the item with the selected path. This method does nothing if
   # the active item does not implement a `.saveAs` method.
   saveActivePaneItemAs: ->
-    @saveActivePaneItemAndReportErrors('saveActiveItemAs')
+    @getActivePane().saveActiveItemAs()
-
-  saveActivePaneItemAndReportErrors: (method) ->
-    try
-      @getActivePane()[method]()
-    catch error
-      if error.message.endsWith('is a directory')
-        atom.notifications.addWarning("Unable to save file: #{error.message}")
-      else if error.code is 'EACCES' and error.path?
-        atom.notifications.addWarning("Unable to save file: Permission denied '#{error.path}'")
-      else if error.code is 'EPERM' and error.path?
-        atom.notifications.addWarning("Unable to save file '#{error.path}'", detail: error.message)
-      else if error.code is 'EBUSY' and error.path?
-        atom.notifications.addWarning("Unable to save file '#{error.path}'", detail: error.message)
-      else if error.code is 'EROFS' and error.path?
-        atom.notifications.addWarning("Unable to save file: Read-only file system '#{error.path}'")
-      else if errorMatch = /ENOTDIR, not a directory '([^']+)'/.exec(error.message)
-        fileName = errorMatch[1]
-        atom.notifications.addWarning("Unable to save file: A directory in the path '#{fileName}' could not be written to")
-      else
-        throw error
 
   # Destroy (close) the active pane item.
   #