Skip to content

Conversation

@dimitrieh
Copy link
Contributor

Description

The housekeeper leadership voting feature introduced in PR #6239 assumes the MQTT broker is always configured. This causes a crash when running FlowFuse without broker configuration (which is valid for local dev).

Add optional chaining to safely handle cases where app.comms.platform is undefined, consistent with how other parts of the codebase handle missing broker configuration (e.g., forge/settings/index.js).

Related Issue(s)

No issue as of yet. But the change was necessary due to #6239

Checklist

  • I have read the contribution guidelines
  • Suitable unit/system level tests have been added and they pass
  • Documentation has been updated
    • Upgrade instructions
    • Configuration details
    • Concepts
  • Changes flowforge.yml?
    • Issue/PR raised on FlowFuse/helm to update ConfigMap Template
    • Issue/PR raised on FlowFuse/CloudProject to update values for Staging/Production
  • Link to Changelog Entry PR, or note why one is not needed.

Labels

  • Includes a DB migration? -> add the area:migration label

The housekeeper leadership voting feature introduced in PR #6239 assumes
the MQTT broker is always configured. This causes a crash when running
FlowFuse without broker configuration (which is valid for local dev).

Add optional chaining to safely handle cases where app.comms.platform
is undefined, consistent with how other parts of the codebase handle
missing broker configuration (e.g., forge/settings/index.js).
@dimitrieh dimitrieh requested a review from hardillb November 19, 2025 20:56
@dimitrieh
Copy link
Contributor Author

@hardillb can you see if this makes sense according to you for dev usecases?

@codecov
Copy link

codecov bot commented Nov 19, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.61%. Comparing base (68b3f49) to head (fc110ef).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6311   +/-   ##
=======================================
  Coverage   76.61%   76.61%           
=======================================
  Files         387      387           
  Lines       19582    19582           
  Branches     4707     4707           
=======================================
  Hits        15003    15003           
  Misses       4579     4579           
Flag Coverage Δ
backend 76.61% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@hardillb
Copy link
Contributor

@dimitrieh running with a broker locally is probably worth it as a lot of the EE features now depend on it, but this should allow housekeeper tasks to run without.

@dimitrieh
Copy link
Contributor Author

@hardillb 👍 . merging in that case

@dimitrieh dimitrieh merged commit bf53442 into main Nov 20, 2025
25 checks passed
@dimitrieh dimitrieh deleted the fix/housekeeper-comms-optional-chaining branch November 20, 2025 08:52
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.

3 participants