Fix KeyError when sys_tempc/sys_tempf missing from system stats - #109
Open
agu2347 wants to merge 1 commit into
Open
Fix KeyError when sys_tempc/sys_tempf missing from system stats#109agu2347 wants to merge 1 commit into
agu2347 wants to merge 1 commit into
Conversation
…nodell#108) Some QNAP models (or firmware versions) omit the sys_tempc/sys_tempf fields from the systemstats API response entirely when the device has no readable system temperature sensor. get_system_stats() accessed these fields unconditionally via root["sys_tempc"]/root["sys_tempf"], raising a KeyError and crashing the whole stats call for those users. The codebase already established the correct defensive pattern for this exact situation (see the pre-existing cpu_model handling), which this change mirrors: fall back to None when the key is absent instead of raising. Adds a new fixture directory (TS-X53-4.5.4-faulty-sys-sensor) modeling a device response with the temperature fields absent, verifying get_system_stats() returns temp_c/temp_f as null instead of raising.
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
1 participant
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.
Problem
get_system_stats()accessesroot["sys_tempc"]androot["sys_tempf"]unconditionally when parsing the QNAPsystemstatsAPI response (#108). On some QNAP models/firmware versions the device has no readable system temperature sensor, and the API omits these fields from the response entirely rather than returning a zero or null value. This causes aKeyError, crashing the entireget_system_stats()call for affected users (not just the temperature reading).Fix
Mirror the existing defensive-access pattern already used for
cpu_modela few lines above in the same function:instead of the previous unconditional
int(root["sys_tempc"])/int(root["sys_tempf"]).Testing
Added a new fixture directory
tests/responses/TS-X53-4.5.4-faulty-sys-sensor/(login.xml / login_with_get.xml copied from an existing fixture, systemstats.xml derived from an existing fixture with thesys_tempc/sys_tempflines removed to reproduce the missing-field response) plus the expectedsystemstats.jsonoutput (temp_c/temp_fbothnull, all other fields populated normally), following the existing data-driven pattern intests/test-models.py.Verified with a baseline-vs-fixed comparison across the full fixture set:
KeyError, and one existing pre-existing test category showed a corresponding error.temp_c/temp_fasnullas expected, with no other fixture's result changed (the pre-existing 7 unrelated MISMATCH results in the fixture set are unchanged before/after, confirming no regressions).Fixes #108.