unify error handling on app/project save — avoid ugly modal message dialog

platform/platform-impl/src/com/intellij/openapi/application/impl/ApplicationImpl.java

@@ -18,7 +18,6 @@ package com.intellij.openapi.application.impl;
 import com.intellij.BundleBase;
 import com.intellij.CommonBundle;
 import com.intellij.diagnostic.PerformanceWatcher;
-import com.intellij.diagnostic.PluginException;
 import com.intellij.diagnostic.ThreadDumper;
 import com.intellij.ide.*;
 import com.intellij.ide.plugins.IdeaPluginDescriptor;
@@ -1351,38 +1350,11 @@ public class ApplicationImpl extends PlatformComponentManagerImpl implements App
     myDispatcher.getMulticaster().writeActionFinished(action);
   }
 
-  public void _saveSettings() { // public for testing purposes
+  // public for testing purposes
+  public void _saveSettings() {
     if (mySaveSettingsIsInProgress.compareAndSet(false, true)) {
       try {
-        StoreUtil.doSave(getStateStore());
-      }
-      catch (final Throwable e) {
-        if (isUnitTestMode()) {
-          System.out.println("Saving application settings failed");
-          e.printStackTrace();
-        }
-        else {
-          LOG.info("Saving application settings failed", e);
-          invokeLater(new Runnable() {
-            @Override
-            public void run() {
-              if (e instanceof PluginException) {
-                final PluginException pluginException = (PluginException)e;
-                PluginManagerCore.disablePlugin(pluginException.getPluginId().getIdString());
-                Messages.showMessageDialog("The plugin " +
-                                           pluginException.getPluginId() +
-                                           " failed to save settings and has been disabled. Please restart " +
-                                           ApplicationNamesInfo.getInstance().getFullProductName(), CommonBundle.getErrorTitle(),
-                                           Messages.getErrorIcon());
-              }
-              else {
-                Messages.showMessageDialog(ApplicationBundle.message("application.save.settings.error", e.getLocalizedMessage()),
-                                           CommonBundle.getErrorTitle(), Messages.getErrorIcon());
-
-              }
-            }
-          });
-        }
+        StoreUtil.save(getStateStore(), null);
       }
       finally {
         mySaveSettingsIsInProgress.set(false);

platform/platform-impl/src/com/intellij/openapi/components/impl/stores/StoreUtil.java

@@ -18,40 +18,62 @@ package com.intellij.openapi.components.impl.stores;
 import com.intellij.diagnostic.IdeErrorsDialog;
 import com.intellij.diagnostic.PluginException;
 import com.intellij.ide.plugins.PluginManagerCore;
+import com.intellij.notification.Notification;
+import com.intellij.notification.NotificationType;
+import com.intellij.openapi.application.ApplicationManager;
+import com.intellij.openapi.application.ApplicationNamesInfo;
+import com.intellij.openapi.application.ex.ApplicationManagerEx;
 import com.intellij.openapi.components.PersistentStateComponent;
 import com.intellij.openapi.components.State;
 import com.intellij.openapi.components.StateStorage.SaveSession;
-import com.intellij.openapi.components.StateStorageException;
+import com.intellij.openapi.diagnostic.Logger;
 import com.intellij.openapi.extensions.PluginId;
+import com.intellij.openapi.project.Project;
 import com.intellij.openapi.util.Pair;
 import com.intellij.openapi.util.ShutDownTracker;
+import com.intellij.openapi.util.text.StringUtil;
 import com.intellij.openapi.vfs.VirtualFile;
 import com.intellij.util.SmartList;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
 public final class StoreUtil {
+  private static final Logger LOG = Logger.getInstance(StoreUtil.class);
+
   private StoreUtil() {
   }
 
-  public static void doSave(@NotNull IComponentStore stateStore) {
+  public static void save(@NotNull IComponentStore stateStore, @Nullable Project project) {
     ShutDownTracker.getInstance().registerStopperThread(Thread.currentThread());
     try {
       stateStore.save(new SmartList<Pair<SaveSession, VirtualFile>>());
     }
+    catch (IComponentStore.SaveCancelledException e) {
+      LOG.info(e);
+    }
     catch (Throwable e) {
+      if (ApplicationManager.getApplication().isUnitTestMode()) {
+        LOG.error("Save settings failed", e);
+      }
+      else {
+        LOG.warn("Save settings failed", e);
+      }
+
+      String messagePostfix = " Please restart " + ApplicationNamesInfo.getInstance().getFullProductName() + "</p>" +
+                              (ApplicationManagerEx.getApplicationEx().isInternal() ? "<p>" + StringUtil.getThrowableText(e) + "</p>" : "");
+
       PluginId pluginId = IdeErrorsDialog.findPluginId(e);
       if (pluginId == null) {
-        //noinspection InstanceofCatchParameter
-        if (e instanceof RuntimeException) {
-          throw ((RuntimeException)e);
-        }
-        else {
-          throw new StateStorageException(e);
-        }
+        new Notification("Settings Error", "Unable to save settings",
+                         "<p>Failed to save settings." + messagePostfix,
+                         NotificationType.ERROR).notify(project);
       }
       else {
-        throw new PluginException(e, pluginId);
+        PluginManagerCore.disablePlugin(pluginId.getIdString());
+
+        new Notification("Settings Error", "Unable to save plugin settings",
+                         "<p>The plugin <i>" + pluginId + "</i> failed to save settings and has been disabled." + messagePostfix,
+                         NotificationType.ERROR).notify(project);
       }
     }
     finally {

platform/platform-impl/src/com/intellij/openapi/project/impl/ProjectImpl.java

@@ -15,14 +15,12 @@
  */
 package com.intellij.openapi.project.impl;
 
-import com.intellij.diagnostic.PluginException;
 import com.intellij.ide.RecentProjectsManager;
 import com.intellij.ide.plugins.IdeaPluginDescriptor;
 import com.intellij.ide.plugins.PluginManagerCore;
 import com.intellij.ide.startup.StartupManagerEx;
 import com.intellij.notification.*;
 import com.intellij.openapi.application.ApplicationManager;
-import com.intellij.openapi.application.ApplicationNamesInfo;
 import com.intellij.openapi.application.PathMacros;
 import com.intellij.openapi.application.ex.ApplicationEx;
 import com.intellij.openapi.application.ex.ApplicationManagerEx;
@@ -42,12 +40,14 @@ import com.intellij.openapi.extensions.ExtensionPointName;
 import com.intellij.openapi.extensions.Extensions;
 import com.intellij.openapi.progress.ProgressIndicator;
 import com.intellij.openapi.progress.ProgressIndicatorProvider;
-import com.intellij.openapi.project.*;
+import com.intellij.openapi.project.DumbAwareRunnable;
+import com.intellij.openapi.project.Project;
+import com.intellij.openapi.project.ProjectManager;
+import com.intellij.openapi.project.ProjectManagerAdapter;
 import com.intellij.openapi.project.ex.ProjectEx;
 import com.intellij.openapi.project.ex.ProjectManagerEx;
 import com.intellij.openapi.startup.StartupManager;
 import com.intellij.openapi.ui.Messages;
-import com.intellij.openapi.ui.ex.MessagesEx;
 import com.intellij.openapi.util.Key;
 import com.intellij.openapi.util.io.FileUtil;
 import com.intellij.openapi.util.text.StringUtil;
@@ -68,13 +68,11 @@ import org.picocontainer.defaults.CachingComponentAdapter;
 import javax.swing.*;
 import javax.swing.event.HyperlinkEvent;
 import java.io.File;
-import java.io.IOException;
 import java.util.*;
 import java.util.concurrent.atomic.AtomicBoolean;
 
 public class ProjectImpl extends PlatformComponentManagerImpl implements ProjectEx {
   private static final Logger LOG = Logger.getInstance("#com.intellij.project.impl.ProjectImpl");
-  private static final String PLUGIN_SETTINGS_ERROR = "Plugin Settings Error";
 
   public static final String NAME_FILE = ".name";
   public static Key<Long> CREATION_TIME = Key.create("ProjectImpl.CREATION_TIME");
@@ -328,7 +326,8 @@ public class ProjectImpl extends PlatformComponentManagerImpl implements Project
   @Override
   public void save() {
     if (ApplicationManagerEx.getApplicationEx().isDoNotSave()) {
-      return; //no need to save
+      // no need to save
+      return;
     }
 
     if (!mySavingInProgress.compareAndSet(false, true)) {
@@ -337,48 +336,26 @@ public class ProjectImpl extends PlatformComponentManagerImpl implements Project
 
     try {
       if (isToSaveProjectName()) {
-        VirtualFile baseDir = getStateStore().getProjectBaseDir();
-        if (baseDir != null && baseDir.isValid()) {
-          VirtualFile ideaDir = baseDir.findChild(DIRECTORY_STORE_FOLDER);
-          if (ideaDir != null && ideaDir.isValid() && ideaDir.isDirectory()) {
-            File nameFile = new File(ideaDir.getPath(), NAME_FILE);
-            try {
+        try {
+          VirtualFile baseDir = getStateStore().getProjectBaseDir();
+          if (baseDir != null && baseDir.isValid()) {
+            VirtualFile ideaDir = baseDir.findChild(DIRECTORY_STORE_FOLDER);
+            if (ideaDir != null && ideaDir.isValid() && ideaDir.isDirectory()) {
+              File nameFile = new File(ideaDir.getPath(), NAME_FILE);
+
               FileUtil.writeToFile(nameFile, getName());
               myOldName = null;
+
+              RecentProjectsManager.getInstance().clearNameCache();
             }
-            catch (IOException e) {
-              LOG.info("Unable to store project name to: " + nameFile.getPath());
-            }
-            RecentProjectsManager.getInstance().clearNameCache();
           }
         }
+        catch (Throwable e) {
+          LOG.error("Unable to store project name");
+        }
       }
 
-      StoreUtil.doSave(getStateStore());
-    }
-    catch (IComponentStore.SaveCancelledException e) {
-      LOG.info(e);
-    }
-    catch (PluginException e) {
-      PluginManagerCore.disablePlugin(e.getPluginId().getIdString());
-      Notification notification = new Notification(
-        PLUGIN_SETTINGS_ERROR,
-        "Unable to save plugin settings!",
-        "<p>The plugin <i>" + e.getPluginId() + "</i> failed to save settings and has been disabled. Please restart " +
-        ApplicationNamesInfo.getInstance().getFullProductName() + "</p>" +
-        (ApplicationManagerEx.getApplicationEx().isInternal() ? "<p>" + StringUtil.getThrowableText(e) + "</p>" : ""),
-        NotificationType.ERROR);
-      Notifications.Bus.notify(notification, this);
-      LOG.info("Unable to save plugin settings", e);
-    }
-    catch (Throwable e) {
-      if (ApplicationManager.getApplication().isUnitTestMode()) {
-        LOG.error(e);
-      }
-      else {
-        LOG.info("Error saving project", e);
-        MessagesEx.error(this, ProjectBundle.message("project.save.error", e.getMessage())).showLater();
-      }
+      StoreUtil.save(getStateStore(), this);
     }
     finally {
       mySavingInProgress.set(false);

platform/platform-resources-en/src/messages/ApplicationBundle.properties

@@ -546,7 +546,6 @@ error.path.variable.is.undefined=Path variable "{0}" is undefined
 text.no.path.variables=No path variables
 config.import.invalid.directory.error={0} is not a directory!
 command.exit=Exit
-application.save.settings.error=Could not save application settings: {0}
 exit.confirm.title=Confirm Exit
 exit.confirm.prompt=Are you sure you want to exit {0}?
 exit.confirm.prompt.tasks=You have background tasks running. Are you sure you want to stop them and exit {0}?

platform/platform-tests/testSrc/com/intellij/openapi/components/impl/ApplicationStoreTest.java

@@ -60,7 +60,7 @@ public class ApplicationStoreTest extends LightPlatformLangTestCase {
     }
   }
 
-  public void testStreamProviderSaveIfSeveralStoragesConfigured() throws Exception {
+  public void testStreamProviderSaveIfSeveralStoragesConfigured() throws Throwable {
     SeveralStoragesConfigured component = new SeveralStoragesConfigured();
 
     MyStreamProvider streamProvider = new MyStreamProvider();
@@ -68,7 +68,7 @@ public class ApplicationStoreTest extends LightPlatformLangTestCase {
 
     componentStore.initComponent(component, false);
     component.foo = "newValue";
-    StoreUtil.doSave(componentStore);
+    StoreUtil.save(componentStore, null);
 
     assertThat(streamProvider.data.get(RoamingType.PER_USER).get(StoragePathMacros.APP_CONFIG + "/proxy.settings.xml"), equalTo("<application>\n" +
                                                                                                                                 "  <component name=\"HttpConfigurable\">\n" +