Fix crash on empty system stats aggregations#317
Open
jonathanendersby wants to merge 2 commits intotomaae:masterfrom
Open
Fix crash on empty system stats aggregations#317jonathanendersby wants to merge 2 commits intotomaae:masterfrom
jonathanendersby wants to merge 2 commits intotomaae:masterfrom
Conversation
Guard against empty or missing `mean` dictionaries in the reporting stats response, which can occur after a TrueNAS reboot or when the reporting service hasn't collected data yet. Fixes: - `ValueError: max() iterable argument is empty` when cputemp aggregations mean dict is empty - `KeyError: 'cpu_cpu'` when _systemstats_process fails to populate cpu stats due to missing keys in mean dict - `KeyError: 'memory-free_value'` when memory stats are unavailable Uses `.get()` with safe defaults and checks that the mean dict is non-empty before calling `max()`. Fixes tomaae#216, fixes tomaae#193, fixes tomaae#55
Reviewer's guide (collapsed on small PRs)Reviewer's GuideHandles empty or sparse system stats aggregation responses from TrueNAS without crashing by adding defensive checks and safe defaults when reading CPU temperature, CPU usage, and memory metrics from the reporting stats API. Sequence diagram for updated system stats handling in get_systemstatssequenceDiagram
participant HA as HomeAssistant
participant Coord as TrueNASCoordinator
participant API as TrueNASReportingAPI
HA->>Coord: get_systemstats()
Coord->>API: fetch system stats
API-->>Coord: tmp_graph (may have empty aggregations)
loop Iterate_tmp_graph_entries
Coord->>Coord: check entry name
alt cputemp_entry
Coord->>Coord: check aggregations and mean exist
alt mean_present_and_non_empty
Coord->>Coord: cpu_temperature = round(max(mean.values), 2)
else mean_missing_or_empty
Coord->>Coord: skip cpu_temperature update (no crash)
end
end
alt cpu_entry
Coord->>Coord: _systemstats_process(arr, graph, cpu)
Coord->>Coord: cpu_usage = round(system_info.get(cpu_cpu, 0.0), 2)
end
alt memory_entry
Coord->>Coord: _systemstats_process(arr, graph, memory)
Coord->>Coord: check memory-total_value > 0
alt total_value_positive
Coord->>Coord: memory_free = float(system_info.get(memory-free_value, 0))
Coord->>Coord: memory-usage_percent = round(100 * (total - memory_free) / total)
else total_value_zero_or_missing
Coord->>Coord: skip memory-usage_percent update (no crash)
end
end
end
Coord-->>HA: Updated ds with safe defaults (0.0) when data missing
Class diagram for updated coordinator system stats processingclassDiagram
class TrueNASCoordinator {
+dict ds
+get_systemstats() void
+_systemstats_process(arr, graph, t) void
}
class SystemInfoStore {
+float cpu_temperature
+float cpu_usage
+float cpu_cpu
+float memory_total_value
+float memory_free_value
+float memory_usage_percent
}
TrueNASCoordinator o-- SystemInfoStore : uses ds[system_info]
class get_systemstats {
+handles_cputemp_with_aggregation_check()
+computes_cpu_usage_with_safe_default()
+computes_memory_usage_with_safe_default()
}
class _systemstats_process {
+reads_graph_aggregations_mean_safely()
+sets_memory_free_value_for_available()
}
TrueNASCoordinator ..> get_systemstats
TrueNASCoordinator ..> _systemstats_process
class TmpGraphEntry {
+string name
+dict aggregations
}
class Aggregations {
+dict mean
}
TmpGraphEntry o-- Aggregations
get_systemstats ..> TmpGraphEntry
_systemstats_process ..> Aggregations
note for get_systemstats "CPU temperature: only uses max(mean.values) when aggregations.mean exists and is non-empty"
note for get_systemstats "CPU usage: uses system_info.get(cpu_cpu, 0.0) to avoid KeyError"
note for get_systemstats "Memory usage: uses system_info.get(memory-free_value, 0) before percentage calculation"
note for _systemstats_process "Mean lookup uses graph[aggregations][mean].get(e, 0.0) to avoid KeyError"
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
get_systemstatsyou now guard against missingaggregations.meanforcputemp, but still indextmp_graph[i]["aggregations"]["mean"]directly; consider assigningmean = tmp_graph[i]["aggregations"].get("mean")once and reusing it to avoid repeat lookups and potential inconsistencies. - In
_systemstats_process,graph["aggregations"]["mean"]is still accessed via direct indexing; ifmeancan be absent in the same scenarios where individual keys are missing, consider using.get("aggregations", {}).get("mean", {})(or an early return) to avoid additionalKeyErrors. - For consistency with the new guarded access patterns, you may want to revisit other
self.ds["system_info"][...]lookups inget_systemstats(e.g.,memory-total_value) and either enforce their presence explicitly or use.get(...)where they may be legitimately absent after a reboot.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `get_systemstats` you now guard against missing `aggregations.mean` for `cputemp`, but still index `tmp_graph[i]["aggregations"]["mean"]` directly; consider assigning `mean = tmp_graph[i]["aggregations"].get("mean")` once and reusing it to avoid repeat lookups and potential inconsistencies.
- In `_systemstats_process`, `graph["aggregations"]["mean"]` is still accessed via direct indexing; if `mean` can be absent in the same scenarios where individual keys are missing, consider using `.get("aggregations", {}).get("mean", {})` (or an early return) to avoid additional `KeyError`s.
- For consistency with the new guarded access patterns, you may want to revisit other `self.ds["system_info"][...]` lookups in `get_systemstats` (e.g., `memory-total_value`) and either enforce their presence explicitly or use `.get(...)` where they may be legitimately absent after a reboot.
## Individual Comments
### Comment 1
<location path="custom_components/truenas/coordinator.py" line_range="474-477" />
<code_context>
e = tmp_var
- tmp_val = graph["aggregations"]["mean"][e] or 0.0
+ tmp_val = graph["aggregations"]["mean"].get(e, 0.0) or 0.0
if t == "memory":
if tmp_var == "available":
</code_context>
<issue_to_address>
**suggestion:** Redundant `or 0.0` after `.get(e, 0.0)`
With `.get(e, 0.0)`, the trailing `or 0.0` only changes behavior for falsy non-`None` values like `0` or `""`, which is undesirable since `0` is a valid metric. Either use `graph["aggregations"]["mean"].get(e) or 0.0` (to only guard against `None`) or keep `.get(e, 0.0)` alone and drop `or 0.0`.
```suggestion
e = tmp_var
tmp_val = graph["aggregations"]["mean"].get(e, 0.0)
if t == "memory":
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Keeps the `or 0.0` to guard against None values from the API while avoiding the subtle issue of coercing a legitimate 0 metric value. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes a crash in the coordinator when TrueNAS returns empty aggregation data from the reporting stats API. This commonly occurs after a TrueNAS reboot or when the reporting service hasn't collected enough data yet.
Three crash points fixed:
ValueError: max() iterable argument is empty—cputempprocessing callsmax()onmean.values()without checking if the dict is emptyKeyError: 'cpu_cpu'— CPU usage readsself.ds["system_info"]["cpu_cpu"]directly, which doesn't exist when_systemstats_processfailed to populate itKeyError: 'memory-free_value'— Memory usage calculation accessesmemory-free_valuedirectly, which doesn't exist when memory stats are emptyKeyErrorin_systemstats_process— Direct dict key accessgraph["aggregations"]["mean"][e]fails when the key is missing from a sparse mean dictChanges:
meandict is non-empty before callingmax()on cputemp values.get("cpu_cpu", 0.0)for safe CPU usage access.get("memory-free_value", 0)for safe memory free value access.get(e, 0.0)in_systemstats_processfor safe mean value lookupAll fixes fall back to
0.0defaults, so sensors show zero rather than crashing the entire integration.Fixes #216, fixes #193, fixes #55
Test plan
Summary by Sourcery
Handle empty or missing TrueNAS system stats aggregations without crashing the coordinator and default metrics to zero when data is unavailable.
Bug Fixes: