-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
If the user.config file is corrupt the whole application crashes #2112
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ | |
using System.ComponentModel; | ||
using System.Configuration; | ||
using System.Diagnostics; | ||
using System.IO; | ||
using System.Runtime.InteropServices; | ||
using System.Windows; | ||
using System.Windows.Interop; | ||
|
@@ -64,9 +65,20 @@ public bool UpgradeSettings | |
{ | ||
get | ||
{ | ||
if (this["UpgradeSettings"] != null) | ||
try | ||
{ | ||
return (bool)this["UpgradeSettings"]; | ||
if (this["UpgradeSettings"] != null) | ||
{ | ||
return (bool)this["UpgradeSettings"]; | ||
} | ||
} | ||
catch (Exception ex) | ||
{ | ||
Debug.WriteLine("Failed to load settings file:\r\n{0}", ex); | ||
var filename = ((ConfigurationErrorsException)ex.InnerException).Filename; | ||
File.Delete(filename); | ||
Process.Start(Application.ResourceAssembly.Location); | ||
Application.Current.Shutdown(); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sorry for my nitpicking ;-) but why must the inner exception cast to ConfigurationErrorsException? the ex is already a ConfigurationErrorsException? should this better so? catch (ConfigurationErrorsException ex)
{
var filename = ex.Filename;
throw new MahAppsException(string.Format("The settings file {0} seems to be corrupted", filename), ex.InnerException);
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes they are the same exception type but in the outer exception Filename is null. You really have to go to the InnerException and cast it… I just debugged the code again to be sure about that. Von: Jan Karger [mailto:notifications@github.com] In MahApps.Metro/Controls/WindowSettings.cs #2112 (comment) :
sorry for my nitpicking ;-) but why must the inner exception cast to ConfigurationErrorsException? the ex is already a ConfigurationErrorsException? should this better so? catch (ConfigurationErrorsException ex) — There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Remolutionary ok, really don't know this, so i think this could be a final solution
thx There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd prefer more robust code like this:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @thoemmi 👍 |
||
return true; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think MahApps should not delete any stuff. instead we should clearly say what is going wrong. so you can use the new MahAppsException, so you can handle this in your application and tell the user, that he must delete their config file.
MahAppsException -> https://github.com/MahApps/MahApps.Metro/blob/master/MahApps.Metro/MahAppsException.cs