You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Lite's App.LoadAlertSettings used to wrap eighty-seven TryGetProperty reads in one try, so one key of the wrong shape abandoned every read after it. PR #2453 replaced those with PerformanceMonitor.Common.SettingsReader, which checks ValueKind before it calls a getter, records the keys it could not read, and reports the whole set by name — so a bad value now costs exactly its own setting and the startup dialog says which.
The Darling Viewer's startup dialog has the same complaint one level up and it is not fixed by that change, because the mechanism is different. The viewer round-trips a whole object rather than reading per property, so its deserialize fails before any property exists: it can name the file and the parse position, and it cannot say which settings were lost, because at the moment it failed none of them had been read.
Why the same answer does not just drop in
SettingsReader is a per-PROPERTY reader. It gives a caller a place to record "this key was the wrong shape" precisely because the caller asks for keys one at a time. A JsonSerializer.Deserialize<T> call has no such seam — the first bad member throws out of the whole object graph, and JsonException.Path names one member at best and only for the one that threw, never the set.
So this is not "call the new type from the viewer too". The options are genuinely different, and which is right is the thing to decide:
Per-property reads in the viewer, like Lite's. Most faithful to the fix, and gives the same "which keys, all of them" message. Costs a hand-written read per setting, and the viewer's settings object is not the same shape as Lite's flat file, so it is not a copy of Lite's loader.
Deserialize with JsonSerializerOptions that tolerate the bad member — a converter or UnmappedMemberHandling-style pass that collects rather than throws. Keeps one call, but reports what a converter can see, which is not obviously the same set.
Two-pass: deserialize, and on failure re-read as JsonDocument to attribute the failure. Cheapest, and the honest limit is that the second pass is a different reader from the first, so the two can disagree about what "unreadable" means — which is the class of defect Darling monitors PostgreSQL and Amazon Aurora PostgreSQL #2213 spent a review round on.
What should decide it
Whether the viewer's settings object is stable enough to be worth eighty-odd hand-written reads, or whether its value is that it IS a serialized object. If the latter, option 2 or 3, and the message should say plainly that it names the member it stopped on rather than the whole set — an overstated capability is worse than an admitted gap.
Note also the constraint that shaped #2444's answer, in case it applies here: Lite's key literals are extracted by regex out of App.xaml.cs by SettingsSampleTests, which is why the read helper's method is called TryGetProperty. The viewer has no equivalent guard today, so this is free of that constraint — but if the viewer ever gains a documented sample file, it will want one.
Split out of #2444 so it does not close with it. #2444 named this as "the viewer has the same gap, one level up", itself split from #2434.
What #2444 fixed, and where it stops
Lite's
App.LoadAlertSettingsused to wrap eighty-sevenTryGetPropertyreads in onetry, so one key of the wrong shape abandoned every read after it. PR #2453 replaced those withPerformanceMonitor.Common.SettingsReader, which checksValueKindbefore it calls a getter, records the keys it could not read, and reports the whole set by name — so a bad value now costs exactly its own setting and the startup dialog says which.The Darling Viewer's startup dialog has the same complaint one level up and it is not fixed by that change, because the mechanism is different. The viewer round-trips a whole object rather than reading per property, so its deserialize fails before any property exists: it can name the file and the parse position, and it cannot say which settings were lost, because at the moment it failed none of them had been read.
Why the same answer does not just drop in
SettingsReaderis a per-PROPERTY reader. It gives a caller a place to record "this key was the wrong shape" precisely because the caller asks for keys one at a time. AJsonSerializer.Deserialize<T>call has no such seam — the first bad member throws out of the whole object graph, andJsonException.Pathnames one member at best and only for the one that threw, never the set.So this is not "call the new type from the viewer too". The options are genuinely different, and which is right is the thing to decide:
JsonSerializerOptionsthat tolerate the bad member — a converter orUnmappedMemberHandling-style pass that collects rather than throws. Keeps one call, but reports what a converter can see, which is not obviously the same set.JsonDocumentto attribute the failure. Cheapest, and the honest limit is that the second pass is a different reader from the first, so the two can disagree about what "unreadable" means — which is the class of defect Darling monitors PostgreSQL and Amazon Aurora PostgreSQL #2213 spent a review round on.What should decide it
Whether the viewer's settings object is stable enough to be worth eighty-odd hand-written reads, or whether its value is that it IS a serialized object. If the latter, option 2 or 3, and the message should say plainly that it names the member it stopped on rather than the whole set — an overstated capability is worse than an admitted gap.
Note also the constraint that shaped #2444's answer, in case it applies here: Lite's key literals are extracted by regex out of
App.xaml.csbySettingsSampleTests, which is why the read helper's method is calledTryGetProperty. The viewer has no equivalent guard today, so this is free of that constraint — but if the viewer ever gains a documented sample file, it will want one.