Skip to content

🧹 [code health] Refactor handle_messages in notifications.py #137

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
DaTiC0 merged 1 commit into from
Jun 6, 2026
Merged

🧹 [code health] Refactor handle_messages in notifications.py #137

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 24 additions & 20 deletions notifications.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,29 @@ def handle_disconnect(_client, _userdata, rc):
_append_mqtt_log('system', f'Unexpected disconnect (rc={rc})', 'Disconnected')


def _handle_status_message(user_id, device_id, payload):
"""Update Firebase device status based on MQTT payload."""
# Guard JSON decoding to avoid noisy errors when payloads are already decoded or non-JSON
state_updates = None

if isinstance(payload, dict):
state_updates = payload
elif isinstance(payload, str):
try:
state_updates = json.loads(payload)
except (ValueError, TypeError):
logger.debug("Non-JSON status payload for %s/%s; skipping Firebase update", user_id, device_id)

if state_updates is not None:

@gemini-code-assist gemini-code-assist Bot Jun 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

If the payload is a valid JSON but not an object (e.g., a string, number, or boolean like "true" or "123"), json.loads will succeed and return a non-dictionary value. Calling ref.update() with a non-dictionary will raise an exception. To prevent this, verify that state_updates is a dictionary before attempting to update the reference.

Suggested change
if state_updates is not None:
if isinstance(state_updates, dict):

try:
ref = _get_user_device_states_ref(user_id, device_id)
if ref:
ref.update(state_updates)
logger.debug("Updated Firebase status for %s/%s", user_id, device_id)
except Exception as e:
logger.error("Failed to update Firebase from MQTT: %s", e)


@mqtt.on_message()
def handle_messages(_client, _userdata, message):
payload = _decode_payload(message.payload)
Expand All @@ -104,27 +127,8 @@ def handle_messages(_client, _userdata, message):
device_id = parts[1]
msg_type = parts[2]

# Try to update Firebase if it's a status message
if msg_type == 'status':
# Guard JSON decoding to avoid noisy errors when payloads are already decoded or non-JSON
state_updates = None

if isinstance(payload, dict):
state_updates = payload
elif isinstance(payload, str):
try:
state_updates = json.loads(payload)
except (ValueError, TypeError):
logger.debug("Non-JSON status payload for %s/%s; skipping Firebase update", user_id, device_id)

if state_updates is not None:
try:
ref = _get_user_device_states_ref(user_id, device_id)
if ref:
ref.update(state_updates)
logger.debug("Updated Firebase status for %s/%s", user_id, device_id)
except Exception as e:
logger.error("Failed to update Firebase from MQTT: %s", e)
_handle_status_message(user_id, device_id, payload)

_append_mqtt_log(topic, payload, 'Received', user_id=user_id)

Expand Down