[Peek]UserSettings load logging fix and refactor (#36111)

* UserSettings refactor to fix logging bug, properly initialise defaults etc.

* Apply lock only to code which changes shared data.
This commit is contained in:
Dave Rayment
2024-11-28 17:36:32 +00:00
committed by GitHub
parent 28304838af
commit 438ee39252
2 changed files with 51 additions and 47 deletions

View File

@@ -3,10 +3,8 @@
// See the LICENSE file in the project root for more information. // See the LICENSE file in the project root for more information.
using System; using System;
using System.IO;
using System.IO.Abstractions; using System.IO.Abstractions;
using System.Threading; using System.Threading;
using ManagedCommon; using ManagedCommon;
using Microsoft.PowerToys.Settings.UI.Library; using Microsoft.PowerToys.Settings.UI.Library;
using Microsoft.PowerToys.Settings.UI.Library.Utilities; using Microsoft.PowerToys.Settings.UI.Library.Utilities;
@@ -16,71 +14,77 @@ namespace Peek.UI
public class UserSettings : IUserSettings public class UserSettings : IUserSettings
{ {
private const string PeekModuleName = "Peek"; private const string PeekModuleName = "Peek";
private const int MaxNumberOfRetry = 5; private const int MaxAttempts = 4;
private readonly SettingsUtils _settingsUtils; private readonly SettingsUtils _settingsUtils;
private readonly IFileSystemWatcher _watcher;
private readonly Lock _loadingSettingsLock = new();
private readonly Lock _settingsLock = new();
[System.Diagnostics.CodeAnalysis.SuppressMessage("CodeQuality", "IDE0052:Remove unread private members", Justification = "Defined in helper called in constructor.")]
private readonly IFileSystemWatcher _watcher;
/// <summary>
/// Gets a value indicating whether Peek closes automatically when the window loses focus.
/// </summary>
public bool CloseAfterLosingFocus { get; private set; } public bool CloseAfterLosingFocus { get; private set; }
public UserSettings() public UserSettings()
{ {
_settingsUtils = new SettingsUtils(); _settingsUtils = new SettingsUtils();
CloseAfterLosingFocus = false;
LoadSettingsFromJson(); LoadSettingsFromJson();
_watcher = Helper.GetFileWatcher(PeekModuleName, "settings.json", () => LoadSettingsFromJson()); _watcher = Helper.GetFileWatcher(PeekModuleName, SettingsUtils.DefaultFileName, LoadSettingsFromJson);
}
private void ApplySettings(PeekSettings settings)
{
lock (_settingsLock)
{
CloseAfterLosingFocus = settings.Properties.CloseAfterLosingFocus.Value;
}
}
private void ApplyDefaultSettings()
{
ApplySettings(new PeekSettings());
} }
private void LoadSettingsFromJson() private void LoadSettingsFromJson()
{ {
lock (_loadingSettingsLock) for (int attempt = 1; attempt <= MaxAttempts; attempt++)
{ {
var retry = true; try
var retryCount = 0;
while (retry)
{ {
try ApplySettings(_settingsUtils.GetSettingsOrDefault<PeekSettings>(PeekModuleName));
return;
}
catch (System.IO.IOException ex)
{
Logger.LogError($"Peek settings load attempt {attempt} failed: {ex.Message}", ex);
if (attempt == MaxAttempts)
{ {
retryCount++; Logger.LogError($"Failed to load Peek settings after {MaxAttempts} attempts. Continuing with default settings.");
ApplyDefaultSettings();
if (!_settingsUtils.SettingsExists(PeekModuleName)) return;
{
Logger.LogInfo("Peek settings.json was missing, creating a new one");
var defaultSettings = new PeekSettings();
defaultSettings.Save(_settingsUtils);
}
var settings = _settingsUtils.GetSettingsOrDefault<PeekSettings>(PeekModuleName);
if (settings != null)
{
CloseAfterLosingFocus = settings.Properties.CloseAfterLosingFocus.Value;
}
retry = false;
}
catch (IOException e)
{
if (retryCount > MaxNumberOfRetry)
{
retry = false;
Logger.LogError($"Failed to Deserialize PowerToys settings, Retrying {e.Message}", e);
}
else
{
Thread.Sleep(500);
}
}
catch (Exception ex)
{
retry = false;
Logger.LogError("Failed to read changed settings", ex);
} }
// Exponential back-off then retry.
Thread.Sleep(CalculateRetryDelay(attempt));
}
catch (Exception ex)
{
// Anything other than an IO exception is an immediate failure.
Logger.LogError($"Peek settings load failed, continuing with defaults: {ex.Message}", ex);
ApplyDefaultSettings();
return;
} }
} }
} }
private static int CalculateRetryDelay(int attempt)
{
return (int)Math.Pow(2, attempt) * 100;
}
} }
} }

View File

@@ -14,7 +14,7 @@ namespace Microsoft.PowerToys.Settings.UI.Library
{ {
public class SettingsUtils : ISettingsUtils public class SettingsUtils : ISettingsUtils
{ {
private const string DefaultFileName = "settings.json"; public const string DefaultFileName = "settings.json";
private const string DefaultModuleName = ""; private const string DefaultModuleName = "";
private readonly IFile _file; private readonly IFile _file;
private readonly ISettingsPath _settingsPath; private readonly ISettingsPath _settingsPath;