mirror of
https://github.com/microsoft/PowerToys.git
synced 2026-04-08 20:27:36 +02:00
CmdPal: Handle corrupted app state file when saving new state (#41422)
## Summary of the Pull Request Ensures the new state is saved even if the previous one is corrupted, which is then ignored. Improves error handling and code clarity in `AppStateModel`. Replaces Debug sink logging with structured Logger. <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist - [x] Closes: #41421 - [ ] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [ ] **Tests:** Added/updated and all pass - [ ] **Localization:** All end-user-facing strings can be localized - [ ] **Dev docs:** Added/updated - [ ] **New binaries:** Added on the required places - [ ] [JSON for signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json) for new binaries - [ ] [WXS for installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs) for new binaries and localization folder - [ ] [YML for CI pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml) for new test projects - [ ] [YML for signed pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml) - [ ] **Documentation updated:** If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys) and link it here: #xxx <!-- Provide a more detailed description of the PR, other things fixed, or any additional comments/features here --> ## Detailed Description of the Pull Request / Additional comments <!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well --> ## Validation Steps Performed
This commit is contained in:
@@ -3,10 +3,12 @@
|
|||||||
// See the LICENSE file in the project root for more information.
|
// See the LICENSE file in the project root for more information.
|
||||||
|
|
||||||
using System.Diagnostics;
|
using System.Diagnostics;
|
||||||
|
using System.Diagnostics.CodeAnalysis;
|
||||||
using System.Text.Json;
|
using System.Text.Json;
|
||||||
using System.Text.Json.Nodes;
|
using System.Text.Json.Nodes;
|
||||||
using System.Text.Json.Serialization;
|
using System.Text.Json.Serialization;
|
||||||
using CommunityToolkit.Mvvm.ComponentModel;
|
using CommunityToolkit.Mvvm.ComponentModel;
|
||||||
|
using ManagedCommon;
|
||||||
using Microsoft.CommandPalette.Extensions.Toolkit;
|
using Microsoft.CommandPalette.Extensions.Toolkit;
|
||||||
using Windows.Foundation;
|
using Windows.Foundation;
|
||||||
|
|
||||||
@@ -39,7 +41,7 @@ public partial class AppStateModel : ObservableObject
|
|||||||
{
|
{
|
||||||
if (string.IsNullOrEmpty(FilePath))
|
if (string.IsNullOrEmpty(FilePath))
|
||||||
{
|
{
|
||||||
throw new InvalidOperationException($"You must set a valid {nameof(SettingsModel.FilePath)} before calling {nameof(LoadState)}");
|
throw new InvalidOperationException($"You must set a valid {nameof(FilePath)} before calling {nameof(LoadState)}");
|
||||||
}
|
}
|
||||||
|
|
||||||
if (!File.Exists(FilePath))
|
if (!File.Exists(FilePath))
|
||||||
@@ -77,43 +79,84 @@ public partial class AppStateModel : ObservableObject
|
|||||||
try
|
try
|
||||||
{
|
{
|
||||||
// Serialize the main dictionary to JSON and save it to the file
|
// Serialize the main dictionary to JSON and save it to the file
|
||||||
var settingsJson = JsonSerializer.Serialize(model, JsonSerializationContext.Default.AppStateModel);
|
var settingsJson = JsonSerializer.Serialize(model, JsonSerializationContext.Default.AppStateModel!);
|
||||||
|
|
||||||
// Is it valid JSON?
|
// validate JSON
|
||||||
if (JsonNode.Parse(settingsJson) is JsonObject newSettings)
|
if (JsonNode.Parse(settingsJson) is not JsonObject newSettings)
|
||||||
{
|
{
|
||||||
// Now, read the existing content from the file
|
Logger.LogError("Failed to parse app state as a JsonObject.");
|
||||||
var oldContent = File.Exists(FilePath) ? File.ReadAllText(FilePath) : "{}";
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
// Is it valid JSON?
|
// read previous settings
|
||||||
if (JsonNode.Parse(oldContent) is JsonObject savedSettings)
|
if (!TryReadSavedState(out var savedSettings))
|
||||||
{
|
{
|
||||||
foreach (var item in newSettings)
|
savedSettings = new JsonObject();
|
||||||
{
|
}
|
||||||
savedSettings[item.Key] = item.Value?.DeepClone();
|
|
||||||
}
|
|
||||||
|
|
||||||
var serialized = savedSettings.ToJsonString(JsonSerializationContext.Default.AppStateModel.Options);
|
// merge new settings into old ones
|
||||||
File.WriteAllText(FilePath, serialized);
|
foreach (var item in newSettings)
|
||||||
|
{
|
||||||
|
savedSettings[item.Key] = item.Value?.DeepClone();
|
||||||
|
}
|
||||||
|
|
||||||
// TODO: Instead of just raising the event here, we should
|
var serialized = savedSettings.ToJsonString(JsonSerializationContext.Default.AppStateModel!.Options);
|
||||||
// have a file change watcher on the settings file, and
|
File.WriteAllText(FilePath, serialized);
|
||||||
// reload the settings then
|
|
||||||
model.StateChanged?.Invoke(model, null);
|
// TODO: Instead of just raising the event here, we should
|
||||||
}
|
// have a file change watcher on the settings file, and
|
||||||
else
|
// reload the settings then
|
||||||
{
|
model.StateChanged?.Invoke(model, null);
|
||||||
Debug.WriteLine("Failed to parse settings file as JsonObject.");
|
}
|
||||||
}
|
catch (Exception ex)
|
||||||
|
{
|
||||||
|
Logger.LogError($"Failed to save application state to {FilePath}:", ex);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
private static bool TryReadSavedState([NotNullWhen(true)] out JsonObject? savedSettings)
|
||||||
|
{
|
||||||
|
savedSettings = null;
|
||||||
|
|
||||||
|
// read existing content from the file
|
||||||
|
string oldContent;
|
||||||
|
try
|
||||||
|
{
|
||||||
|
if (File.Exists(FilePath))
|
||||||
|
{
|
||||||
|
oldContent = File.ReadAllText(FilePath);
|
||||||
}
|
}
|
||||||
else
|
else
|
||||||
{
|
{
|
||||||
Debug.WriteLine("Failed to parse settings file as JsonObject.");
|
// file doesn't exist (might not have been created yet), so consider this a success
|
||||||
|
// and return empty settings
|
||||||
|
savedSettings = new JsonObject();
|
||||||
|
return true;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
catch (Exception ex)
|
catch (Exception ex)
|
||||||
{
|
{
|
||||||
Debug.WriteLine(ex.ToString());
|
Logger.LogWarning($"Failed to read app state file {FilePath}:\n{ex}");
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
// detect empty file, just for sake of logging
|
||||||
|
if (string.IsNullOrWhiteSpace(oldContent))
|
||||||
|
{
|
||||||
|
Logger.LogInfo($"App state file is empty: {FilePath}");
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
// is it valid JSON?
|
||||||
|
try
|
||||||
|
{
|
||||||
|
savedSettings = JsonNode.Parse(oldContent) as JsonObject;
|
||||||
|
return savedSettings != null;
|
||||||
|
}
|
||||||
|
catch (Exception ex)
|
||||||
|
{
|
||||||
|
Logger.LogWarning($"Failed to parse app state from {FilePath}:\n{ex}");
|
||||||
|
return false;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user