fix(mqtt): count connect failures from a flag set before the attempt

The Starting-state gate raced the event task: client_state becomes Starting
only after connect()/reconnect() returns, so a fast failure could deliver
DISCONNECTED first and go uncounted, while a deliberate stop of an attempt in
flight was counted as a failure. A per-slot flag is now raised before the call
and cleared by onConnect, a failed start, or stopSlotClient().

Also stops zero-valued connect_failures/slots_breaker from creating an empty
stats object.
This commit is contained in:
agessaman committed 2026-09-26 11:53:54 -07:00
1 parent 7004b81c0c
commit 73098a44b6
4 files changed
+33 -4

No files matched your search

+1 -1
View File
@@ -65,7 +65,7 @@ int MQTTPayloadBuilder::buildStatusMessage(
const bool has_conn_health = conn_health.slots_up >= 0 || conn_health.slots_total >= 0 ||
conn_health.worst_outage_secs >= 0 || conn_health.heap_largest >= 0 ||
conn_health.connect_failures >= 0 || conn_health.slots_breaker >= 0;
conn_health.connect_failures > 0 || conn_health.slots_breaker > 0;
if (battery_mv >= 0 || uptime_secs >= 0 || errors >= 0 || queue_len >= 0 ||
noise_floor > -999 || tx_air_secs >= 0 || rx_air_secs >= 0 || recv_errors >= 0 ||
+10 -3
View File
@@ -772,6 +772,7 @@ MQTTBridge::MQTTBridge(NodePrefs *prefs, MQTTPrefs *obs, mesh::PacketManager *mg
_slot_reconfigure_pending[i] = false;
_slot_force_jwt_mint[i] = false;
_status_publish_pending[i] = false;
_slot_attempt_pending[i] = false;
}
// Reset CLI-requested forced NTP sync handshake (bridge object is reused across restarts)
@@ -1730,6 +1731,7 @@ bool MQTTBridge::ensureSlotClient(int index) {
return;
}
MQTT_DEBUG_PRINTLN("MQTT%d connected", index + 1);
_slot_attempt_pending[index] = false;
_slots[index].client_state = ClientState::Connected;
_slots[index].connected = true;
_slot_force_jwt_mint[index] = false;
@@ -1763,9 +1765,10 @@ bool MQTTBridge::ensureSlotClient(int index) {
});
slot.client->onDisconnect([this, index](bool sessionPresent) {
MQTT_DEBUG_PRINTLN("MQTT%d disconnected", index + 1);
// An event while an attempt is in flight ended one that never reached onConnect.
// Stopped/quarantined late events and soft-disconnects are not Starting.
if (mqttClientStateHasAttemptInFlight(_slots[index].client_state)) {
// Ended an attempt that never reached onConnect. A deliberate stop clears the flag
// first, so cancelling an attempt is not counted as a failure.
if (_slot_attempt_pending[index]) {
_slot_attempt_pending[index] = false;
_slots[index].connect_failures++;
}
// Only a live client's disconnect is news. One arriving for a client we
@@ -2411,6 +2414,7 @@ void MQTTBridge::closeLiveClientForLinkTransition(int index) {
// the rest of the boot.
void MQTTBridge::stopSlotClient(int index) {
MQTTSlot& slot = _slots[index];
_slot_attempt_pending[index] = false;
const esp_err_t r = slot.client->disconnect();
// What the SDK's results actually mean here (mqtt_client.h documents
@@ -2505,6 +2509,7 @@ esp_err_t MQTTBridge::reconnectSlotClient(int index) {
}
esp_err_t r;
_slot_attempt_pending[index] = true;
if (!slot.client->isStarted()) {
MQTT_DEBUG_PRINTLN("MQTT%d start (client was stopped)", index + 1);
r = slot.client->connect();
@@ -2517,6 +2522,8 @@ esp_err_t MQTTBridge::reconnectSlotClient(int index) {
// The attempt carries whatever credential is configured right now, so this
// is the point at which a freshly minted token becomes the one in use.
slot.applied_token_expires_at = slot.token_expires_at;
} else {
_slot_attempt_pending[index] = false;
}
return r;
}
+5
View File
@@ -286,6 +286,11 @@ private:
// _slot_reconfigure_pending; a single-byte volatile store/load is atomic.
volatile bool _status_publish_pending[RUNTIME_MQTT_SLOTS];
// Set by the bridge task before connect()/reconnect(), so a failure the event task
// delivers before that call returns is still seen; cleared by the attempt's outcome
// or a deliberate stop. A disconnect that finds it set is a connect failure.
volatile bool _slot_attempt_pending[RUNTIME_MQTT_SLOTS];
// CLI-requested forced NTP sync, marshalled onto the MQTT task (Core 0).
// All NTP I/O must run on Core 0; the CLI thread
// (Core 1) sets _ntp_force_requested and blocks in requestForcedNtpSync()
@@ -158,6 +158,23 @@ TEST(MQTTPayloadBuilder, StatusReportsConnectFailuresAndBreakerOnlyWhenNonZero)
EXPECT_FALSE(healthy_stats["mqtt_slots_breaker"].is<JsonVariant>());
}
// Zero counters with nothing else supplied must not create an empty stats object.
TEST(MQTTPayloadBuilder, StatusOmitsStatsWhenOnlyZeroCountersSupplied) {
JsonDocument scratch;
char buffer[768];
MQTTConnHealth health;
health.connect_failures = 0;
health.slots_breaker = 0;
ASSERT_GT(MQTTPayloadBuilder::buildStatusMessage(
scratch, "node", "id", "model", "firmware", "radio", "client", "online",
kTimestamp, buffer, sizeof(buffer), -1, -1, -1, -1, -999,
-1, -1, -1, -1, -1, -1, nullptr, health), 0);
JsonDocument parsed;
ASSERT_FALSE(deserializeJson(parsed, buffer));
EXPECT_FALSE(parsed["stats"].is<JsonVariant>());
}
// The outage key doubles as the "something is down" flag, so its absence has to be
// meaningful: a fully healthy board must not emit it.
TEST(MQTTPayloadBuilder, StatusOmitsOutageKeyWhenEverySlotIsUp) {