Hold back the Expert disconnect notice until a drop actually lasts - #8632
Merged
Merged
Conversation
reconnectAttempt was cleared on every successful connect, so a link that connected and then dropped seconds later never backed off and kept retrying at the initial delay. Record when the connection came up instead, and only clear the counter once it has held for STABLE_CONNECTION_MS. _dispatch also invoked observer handlers without catching, so any async handler that rejected escaped as an unhandled rejection. That is how in-flight packets flushed by a client teardown surfaced as "Connection closed". It now catches sync throws and attaches a handler to any promise a handler returns, staying quiet for teardown errors and warning on everything else. Also attaches a catch to the fire-and-forget abort publish and gives the expert:status-message ack the try/catch its sibling branches already have.
_onMqttClose posted "Looks like we got disconnected. Send another message to pick up where we left off, or start a fresh session." on every close, including drops the client recovered from a second later. That advice is wrong for a blip, and the message stays in the transcript for good. Start a 15s timer instead and let a reconnect cancel it. The "I'm back" greeting only fires when the previous message was an error, so suppressing the notice suppresses the greeting with it and a short drop is silent at both ends. A real outage still posts once at 15s and still gets its greeting on recovery. handleMqttError and startOver clear a pending notice too: the first posts a message of its own, the second clears the transcript it belonged to.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 8630-expert-mqtt-reconnect-churn #8632 +/- ##
=================================================================
Coverage 77.25% 77.25%
=================================================================
Files 466 466
Lines 25131 25131
Branches 6690 6690
=================================================================
Hits 19414 19414
Misses 5717 5717
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
andypalmi
approved these changes
Sep 22, 2026
This branch was successfully deployed
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
2 participants
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.
Closes #8630
Stacked on #8631, so review that one first.
_onMqttCloseposted its message on every close, including drops the client recovered from a second later. "Send another message to pick up where we left off, or start a fresh session" is wrong advice when the connection is about to come back on its own, and the message then sits in the transcript for good.The close handler now starts a 15s timer and a reconnect cancels it. The
_onMqttConnectgreeting is gated on the previous message being an error, so suppressing the notice suppresses the greeting along with it and a short drop is silent at both ends. An outage that genuinely lasts still posts once at 15s and still gets its greeting when it recovers.handleMqttErrorandstartOverclear a pending notice too, the first because it posts a message of its own, the second because the transcript the notice belonged to is about to be cleared.Worth knowing for review:
addPredefinedAiMessagealready dedups onerrorCodeacross the last three messages, so a pure close and connect loop was only ever posting one pair rather than one per cycle. This is a correctness fix on a misleading message, not a volume fix. The issue body has the detail.Five tests added, four of which fail without the change.