From 3850550345c16ac646ab70fd8247ed086442deaf Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Thu, 5 Feb 2015 14:42:21 -0800 Subject: [PATCH 1/9] Handle save errors in Pane --- src/pane.coffee | 27 +++++++++++++++++++++++++-- src/window-bootstrap.coffee | 2 ++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/pane.coffee b/src/pane.coffee index 8524e3fae..8ba70fc76 100644 --- a/src/pane.coffee +++ b/src/pane.coffee @@ -481,7 +481,10 @@ class Pane extends Model itemURI = item.getUri() if itemURI? - item.save?() + try + item.save?() + catch + @handleSaveError(error) nextAction?() else @saveItemAs(item, nextAction) @@ -498,7 +501,10 @@ class Pane extends Model itemPath = item.getPath?() newItemPath = atom.showSaveDialogSync(itemPath) if newItemPath - item.saveAs(newItemPath) + try + item.saveAs(newItemPath) + catch error + @handleSaveError(error) nextAction?() # Public: Save all items. @@ -667,3 +673,20 @@ 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 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 diff --git a/src/window-bootstrap.coffee b/src/window-bootstrap.coffee index 886ba26dc..6b3f1a846 100644 --- a/src/window-bootstrap.coffee +++ b/src/window-bootstrap.coffee @@ -1,10 +1,12 @@ # Like sands through the hourglass, so are the days of our lives. +console.profile('loading') require './window' Atom = require './atom' window.atom = Atom.loadOrCreate('editor') atom.initialize() atom.startEditorWindow() +console.profileEnd('loading') # Workaround for focus getting cleared upon window creation windowFocused = -> From 477801ba52b784e6d898529b73fc4b138fa66cd9 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Thu, 5 Feb 2015 14:43:45 -0800 Subject: [PATCH 2/9] Remove save error reporting from Workspace --- src/workspace.coffee | 24 ++---------------------- 1 file changed, 2 insertions(+), 22 deletions(-) diff --git a/src/workspace.coffee b/src/workspace.coffee index 315fa7e94..b2fc6fbf1 100644 --- a/src/workspace.coffee +++ b/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') - - 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 + @getActivePane().saveActiveItemAs() # Destroy (close) the active pane item. # From 68e5839b1437e8d63d93b64a6f0726ba4b21fc1b Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Thu, 5 Feb 2015 14:44:16 -0800 Subject: [PATCH 3/9] Add missing error var --- src/pane.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pane.coffee b/src/pane.coffee index 8ba70fc76..f236d14ab 100644 --- a/src/pane.coffee +++ b/src/pane.coffee @@ -483,7 +483,7 @@ class Pane extends Model if itemURI? try item.save?() - catch + catch error @handleSaveError(error) nextAction?() else From 63ee46023d0e07edc7e97487cc8aee9570fc3ab7 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Thu, 5 Feb 2015 14:57:09 -0800 Subject: [PATCH 4/9] Remove profiling --- src/window-bootstrap.coffee | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/window-bootstrap.coffee b/src/window-bootstrap.coffee index 6b3f1a846..886ba26dc 100644 --- a/src/window-bootstrap.coffee +++ b/src/window-bootstrap.coffee @@ -1,12 +1,10 @@ # Like sands through the hourglass, so are the days of our lives. -console.profile('loading') require './window' Atom = require './atom' window.atom = Atom.loadOrCreate('editor') atom.initialize() atom.startEditorWindow() -console.profileEnd('loading') # Workaround for focus getting cleared upon window creation windowFocused = -> From e51c8f34070788511aed21450ba4c687fb52fb30 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Thu, 5 Feb 2015 15:07:42 -0800 Subject: [PATCH 5/9] Add specs for save/saveAs error handling --- spec/pane-spec.coffee | 38 ++++++++++++++++++++++++++++++++++++++ src/pane.coffee | 1 + 2 files changed, 39 insertions(+) diff --git a/spec/pane-spec.coffee b/spec/pane-spec.coffee index 3477acc52..ac6e1d26f 100644 --- a/spec/pane-spec.coffee +++ b/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")]) diff --git a/src/pane.coffee b/src/pane.coffee index f236d14ab..aef4b2aff 100644 --- a/src/pane.coffee +++ b/src/pane.coffee @@ -502,6 +502,7 @@ class Pane extends Model newItemPath = atom.showSaveDialogSync(itemPath) if newItemPath try + console.log 'here?' item.saveAs(newItemPath) catch error @handleSaveError(error) From c9f13afb72a3356670a6c2685c8065aee6c33acb Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Thu, 5 Feb 2015 15:12:20 -0800 Subject: [PATCH 6/9] Remove stray logging --- src/pane.coffee | 1 - 1 file changed, 1 deletion(-) diff --git a/src/pane.coffee b/src/pane.coffee index aef4b2aff..f236d14ab 100644 --- a/src/pane.coffee +++ b/src/pane.coffee @@ -502,7 +502,6 @@ class Pane extends Model newItemPath = atom.showSaveDialogSync(itemPath) if newItemPath try - console.log 'here?' item.saveAs(newItemPath) catch error @handleSaveError(error) From f7502d8508ca29f55cc97cb77c8316afcaf9f47a Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Thu, 5 Feb 2015 15:17:23 -0800 Subject: [PATCH 7/9] Handle UNKNOWN save errors --- src/pane.coffee | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/pane.coffee b/src/pane.coffee index f236d14ab..f1bc34abf 100644 --- a/src/pane.coffee +++ b/src/pane.coffee @@ -679,9 +679,7 @@ class Pane extends Model 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? + 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}'") From 5de95759cdb1bf76f9b67f0dfef65b9f0ee44136 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Thu, 5 Feb 2015 15:23:46 -0800 Subject: [PATCH 8/9] Spy on editor.save --- spec/workspace-spec.coffee | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/spec/workspace-spec.coffee b/spec/workspace-spec.coffee index b9642e17d..82526a299 100644 --- a/spec/workspace-spec.coffee +++ b/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() From 30419027a8ed815722241c6a6bd4bea3c80e21e7 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Thu, 5 Feb 2015 15:34:17 -0800 Subject: [PATCH 9/9] Show notification on config save failures --- spec/config-spec.coffee | 17 +++++++++++++++++ src/config.coffee | 7 ++++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/spec/config-spec.coffee b/spec/config-spec.coffee index 15dc3b8fa..4abd3b0db 100644 --- a/spec/config-spec.coffee +++ b/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() diff --git a/src/config.coffee b/src/config.coffee index d9695fb55..937bb1307 100644 --- a/src/config.coffee +++ b/src/config.coffee @@ -868,7 +868,12 @@ class Config save: -> allSettings = {'*': @settings} allSettings = _.extend allSettings, @scopedSettingsStore.propertiesForSource(@getUserConfigPath()) - CSON.writeFileSync(@configFilePath, allSettings) + 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