Skip to content

Add comms heartbeat to detect lack of message flow - #8450

Open
knolleary wants to merge 1 commit into
mainfrom
8362-comms-heartbeat
Open

Add comms heartbeat to detect lack of message flow#8450
knolleary wants to merge 1 commit into
mainfrom
8362-comms-heartbeat

Conversation

@knolleary

@knolleary knolleary commented Sep 7, 2026

Copy link
Copy Markdown
Member

Closes #8362

This adds a heartbeat to the CommsClient MQTT connection.

  • It publishes to ff/v1/platform/${this.platformId}/heartbeat every 15 seconds with a payload of the current time in ms.
  • It subscribes to the same topic.
  • A watchdog thread checks how long ago the last heartbeat message was received.
    • <30s - that's fine
    • 30-60s - log a warning - No comms heartbeat received for ${delta} ms
    • >60s - disconnect and reconnect the mqtt client - No comms heartbeat received for ${delta} ms - reconnecting

For the issue we're trying to handle, messages appear to stop flowing after a client reconnect. The underlying mqtt client handles resubscribing - the theory here is something goes wrong with those subscriptions.

We cannot simply call client.reconnect in case it is the internal state of the local client that has gone wrong. Instead we disconnect the existing client and reinitialise a new one. Some care has gone into this PR to ensure the old client is properly cleaned up and doesn't interfere with the new one.

One key assumptions about the issue we're trying to fix here: That all subscriptions are similarly impacted - so adding one more here works as a detection of a problem elsewhere.

Given we cannot reproduce the issue at will, there is definitely some speculation about whether this fix will help detect it.

The most notable symptom is the Device Status checkin messages not getting through. They are on a shared subscription. If the underlying issue is broker-side related to the shared sub, then this new non-shared subscription isn't going to necessarily spot it.

An alternative would be to send a heartbeat on the device checkin topic - with placeholder team/device ids that can be passed through the ACL check. Going to think about that separately - but we can get this watchdog in place first.

@codecov

codecov Bot commented Sep 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 26.86567% with 49 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.64%. Comparing base (4d85e86) to head (7ec9738).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
forge/comms/commsClient.js 26.86% 49 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8450      +/-   ##
==========================================
- Coverage   76.77%   76.64%   -0.14%     
==========================================
  Files         460      460              
  Lines       24703    24766      +63     
  Branches     6573     6587      +14     
==========================================
+ Hits        18966    18982      +16     
- Misses       5737     5784      +47     
Flag Coverage Δ
backend 76.64% <26.86%> (-0.14%) ⬇️

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

☔ View full report in Codecov by Harness.
📢 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

hardillb commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Just double checking, because each instance (in the case of FFC where we have 2) has it's own platformId so they should each work and reset independently of each other, but to get the symptoms we are seeing they both must be failing at the same time? (assuming shared subscriptions group should be sending half of device check in to each instance)

@hardillb hardillb left a comment

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.

Approved with the comments about if this is a shared subscription problem this won't catch it from our chat this morning.

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.

CommsClient heartbeat health check

3 participants