Skip to content

fix: keep drive names and temperatures only from a disk read that worked - #3151

Merged
laurentiu021 merged 1 commit into
mainfrom
fix/2782-disk-reads-that-fail-are-not-cached
Nov 3, 2026
Merged

laurentiu021 merged 1 commit into
mainfrom
fix/2782-disk-reads-that-fail-are-not-cached

Conversation

@laurentiu021

Copy link
Copy Markdown
Owner

Closes #2782.

What changes for a user

  • Drive names come back after a failed read. When a read of the drives failed, a WMI service still starting at logon being enough, the Dashboard's temperature card kept the failed read as if it were the drives. Run as administrator, a drive the sensors could not name stayed "Drive 1" until SysManager was restarted. Without administrator, the drive temperatures were gone for 30 seconds. Only a read Windows answers in full is kept now, and a failed one is tried again at the next refresh, two seconds later.
  • No more refused query at every refresh. Run as administrator, the card also asked for the drive models with SELECT Model FROM Win32_DiskDrive ORDER BY Index. WQL has no ORDER BY, so WMI refused the query as invalid at every refresh, and SysManager logged each refusal. It never named a drive in any release since 1.17.0. It is removed.
  • System Health says when it could not read the drives. Run SMART check said "Collected 0 disk report(s)." when Windows did not answer. It now says the check failed, and why. A read that stopped partway says so after the drives it did read.

How

  • DiskHealthService.ReadAsync returns a DiskHealthRead: the drives, and the fault that stopped the read, or null when it finished. CollectAsync still returns the drives alone for its callers (health score, Tune-up, the report), which judge each drive and already count an empty list as not checked. A run of failed reads is logged once, the way NvidiaGpuReader logs, and after a read succeeds the next failure is logged again. An internal constructor takes the read and the log, so a test can make a read fail.
  • TemperatureService.EnrichStorageNamesAsync caches names only from a complete read and names nothing on a failed one. ReadDiskTemperaturesAsync shows a failed read's drives for that refresh but caches only a complete one. Its three catches, for exceptions DiskHealthService never lets out, are gone. Both methods are internal for the tests.
  • GetDiskNamesFromWmi and its cache are gone, and ReadHardware takes no name list. A drive LibreHardwareMonitor cannot name is "Drive N", and the enricher names it, as it already did. The prefetch paired names by position anyway, and a drive with no temperature sensor adds no reading, so if the query had worked, each drive after one like that would have shown the previous drive's model.
  • SystemHealthViewModel.DescribeDiskRead writes the status line. The catch for ManagementException or COMException could never run and is gone.
  • ArchitectureTests.NoWmiQuery_SortsWithOrderBy scans the app's code, comments left out, for ORDER BY. It also checks that at least 20 WMI queries were read, so an empty scan cannot pass.

Tests

  • Names: a failed read leaves "Drive 1" for one refresh and the next read names it. A read cut short names nothing, even when its drive count matches. A complete read is read once over three refreshes. The last one replaces the integration test TemperatureServiceMemoizeTests, which was written on the opposite premise, that an empty result from a failed read was "still a valid, cached resolution".
  • Temperatures: a failed read is read again at the next refresh, a read cut short shows its drives but is not kept, and a complete read is kept for exactly DiskTemperatureTtl and read again one second later. Before this the TTL could not be reached from a test.
  • DiskHealthService: three failures, a success and a failure log twice; a read that works logs nothing; CollectAsync still returns the drives of a read cut short. System Health: a failed check's status, and both wordings of DescribeDiskRead.
  • ReadHardware: a drive LibreHardwareMonitor cannot name is a numbered placeholder.

Red before the fix, by mutation, each restored byte for byte afterwards:

  • Caching names from any read: the failed-read and cut-short name tests fail, and the read-once test passes.
  • Caching temperatures from any read: the failed-read and cut-short temperature tests fail, and the TTL test passes.
  • Logging every failure: the log-once test fails.
  • The old "Collected N" status: the failed-check test fails, and so does the cut-short row of DescribeDiskRead.
  • An ORDER BY in a real query: the new guard fails.

Checks

  • All four projects build with 0 warnings, and dotnet format --verify-no-changes is clean.
  • The unit suite passed twice on the final tree: 10,706 tests, 0 failed. One skip was there before this change.
  • The affected integration classes pass: 63 tests, with DiskHealthServiceTests against real WMI.
  • A console harness over the built app read the real drives three times, elevated and with a standard user's token: every read complete, 2 drives. Elevated, both had temperatures; with the standard token, neither did.
  • CHANGELOG (1.136.104), the csproj bump, README (Dashboard Temperatures, System Health SMART) and ARCHITECTURE (DiskHealthService, TemperatureService) are in this pull request.

Closes #2782.

DiskHealthService returned what it had read before a fault, with nothing
to say it had failed, so TemperatureService kept a failed read as the
drives: the names for the session, the non-elevated temperatures for 30
seconds. ReadAsync now returns a DiskHealthRead with the fault, both
caches take only a complete read, the dead catches go, and a run of
failed reads is logged once.

The Win32_DiskDrive name prefetch asked WMI for ORDER BY, which WQL does
not have, so it failed on every elevated poll and never named a drive.
It is removed, and an ArchitectureTests guard rejects ORDER BY in any
WMI query. System Health's SMART check says a failed read failed instead
of "Collected 0 disk report(s).".
@laurentiu021
laurentiu021 merged commit 3a824fb into main Nov 3, 2026
7 checks passed
@laurentiu021
laurentiu021 deleted the fix/2782-disk-reads-that-fail-are-not-cached branch November 3, 2026 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Dashboard — one failed disk read keeps the drive names off the temperatures for the whole session, and hides drive temperatures for 30 seconds

1 participant