exceptionless / Exceptionless.Net

Exceptionless clients for the .NET platform
https://exceptionless.com
Other
551 stars 142 forks source link

"server-settings.json" can become invalid because previous data is not replaced #267

Closed pv11 closed 2 years ago

pv11 commented 2 years ago

Steps to reproduce:

  1. have at least one custom setting. Example: bug-1

1.1 Let the client update the local "server-settings.json" file

  1. change the custom settings in such a way that the now value is shorter than the original value. Example: bug-2

  2. After the client updates the local "server-settings.json" file, the JSON will be invalid. Example: bug-3

Tested using "Exceptionless.dll" 4.6.2 in a C# project with .Net Framework 4.5.2

niemyjski commented 2 years ago

Can you please provide detailed steps that can reproduce this? I've tried both a sample console app as well as unit test and am unable to reproduce this issue. The only thing I could think would be some concurrency issue writing to the file at the same time. But the file should be completely overwritten and behind a lock.

        public virtual void CanRoundTripSettingsWithUpdates() {
            var settings = new SettingsDictionary {
                ["MySetting"] = "ABCDE"
            };

            var storage = GetStorage();
            storage.SaveObject("test.json", settings);

            settings["MySetting"] = "AB";
            storage.SaveObject("test.json", settings);

            var savedSettings = storage.GetObject<SettingsDictionary>("test.json");
            Assert.Single(savedSettings);
        }
{"MySetting":"AB"}
pv11 commented 2 years ago

The problem is observable like this:

        static void Main(string[] args)
        {
            const string apiKey = "TLFsO5wRvyoMDiF3SSJOgkskCNhTHw**********";
            const string useFolder = @"c:\temp";

            var serverSettingsFile = Path.Combine(useFolder, apiKey.Substring(0,8), "server-settings.json");

            ExceptionlessClient.Default.Configuration.UseFolderStorage(useFolder);
            ExceptionlessClient.Default.Configuration.UseFileLogger(Path.Combine(useFolder, $"Exceptionless.log"));

            ExceptionlessClient.Default.Startup(apiKey);

            while (true)
            {
                Console.WriteLine("Change Client Configuration Settings and watch the result here (you might need to wait up to 2 minutes...)");

                System.Threading.Thread.Sleep(5000);

                if (File.Exists(serverSettingsFile))
                {
                    Console.WriteLine($"Current settings (at {DateTime.Now:u}):");
                    Console.ForegroundColor = ConsoleColor.Cyan;
                    Console.WriteLine(File.ReadAllText(serverSettingsFile));
                    Console.ResetColor();
                }
            }
        }

console result

niemyjski commented 2 years ago

Does this happen if you don't use a temp folder? I just tried it and I couldn't reproduce

image

pv11 commented 2 years ago

Apparently the problem occurs only when using a custom folder to settings storage, i. e.: ExceptionlessClient.Default.Configuration.UseFolderStorage("c:\\myfolder");

It looks OK when using: ExceptionlessClient.Default.Configuration.UseIsolatedStorage();

(one can confirm by opening the file "C:\ProgramData\IsolatedStorage\xxx\AssemFiles\TLFsO5wR\server-settings.json" )

niemyjski commented 2 years ago

What operating system are you using? I'm on Windows 11 and I also tried it with UseFolderStorage("store") and it worked

pv11 commented 2 years ago

I'm using Windows 10 Pro.

However, I downloaded the entire code from this repository and tested by referencing directly the VIsual Studio project into my test console (instead in referencing the complied assemblies), and that way it worked OK.

So the mistery remains.

niemyjski commented 2 years ago

I've been able to reproduce using the nuget package and not source. I'll try and track this down

niemyjski commented 2 years ago

I need to get better about pushing more frequent releases. I just noticed after tracking this down that this was actually fixed in this pr quite some time ago: https://github.com/exceptionless/Exceptionless.Net/pull/247 I'm pushing a new release in a few minutes.

niemyjski commented 2 years ago

https://github.com/exceptionless/Exceptionless.Net/releases/tag/v4.7.0