Skip to content

fix: [sc-96140] Set explicit MQTT ConnectTimeout instead of relying on paho default#76

Merged
mlataza merged 2 commits into
mainfrom
bug/sc-96140/set-explicit-mqtt-connecttimeout-instead-of
Jun 5, 2026
Merged

fix: [sc-96140] Set explicit MQTT ConnectTimeout instead of relying on paho default#76
mlataza merged 2 commits into
mainfrom
bug/sc-96140/set-explicit-mqtt-connecttimeout-instead-of

Conversation

@mlataza

@mlataza mlataza commented Jun 5, 2026

Copy link
Copy Markdown
Collaborator

Summary

The MQTT client options assembled in runCycle set AutoReconnect=false and an OnConnectionLost callback, but never called SetConnectTimeout — so initial-connect/subscribe operations silently relied on paho.mqtt.golang's implicit 30s default. This coupled our reconnect behaviour to an undeclared library default that could drift across pinned paho versions without any signal in our code.

This PR makes the connect timeout fully owned by our code so reconnect behaviour stays predictable across paho upgrades.

Changes

  • internal/utils/time.go — Added InitialReconnectInterval (1s), now the seed for the reconnect backoff, plus DefaultMqttConnectTimeout as a documented constant alongside the backoff parameters. The default is <= the shortest possible backoff slot (1.5 × InitialReconnectInterval after worst-case jitter), so a connect attempt can never outlive the next backoff slot.
  • internal/agent/device.go — Added optional per-device override mqtt_connect_timeout_seconds (mirroring the existing mqtt_qos tunable) and a Device.MqttConnectTimeout() helper.
  • internal/mqtt/common.goNewClientOptions now calls opts.SetConnectTimeout(...) centrally for all broker paths.
  • Tests — Assert the default and per-device override in common_test.go; record the chosen timeout/rationale and prove it fits within the backoff in time_test.go.

Acceptance criteria

  • ✅ ConnectTimeout explicitly set to a documented value (constant + rationale comment).
  • ✅ Overridable per-device via the existing device config struct.
  • ✅ Value consistent with the initial backoff interval — connect attempt never outlives the next backoff slot.
  • ✅ Unit tests + comments record the chosen timeout and rationale.

Notes

The default is 1s, a behavioural reduction from paho's old 30s, chosen to strictly satisfy the "never outlives the next backoff slot" criterion. Tenants with occasionally slow TLS handshakes are covered by the new per-device mqtt_connect_timeout_seconds override rather than a looser default.

🤖 Generated with Claude Code

mlataza and others added 2 commits June 5, 2026 06:45
…n paho default

Own the MQTT connect timeout in our code rather than depending on
paho.mqtt.golang's implicit 30s default, so reconnect timing stays
predictable across paho upgrades. The default is set to the reconnect
backoff base interval so a connect attempt can never outlive the next
backoff slot, and it can be overridden per-device via the existing
device config (mqtt_connect_timeout_seconds).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@mlataza
mlataza merged commit 6fc320d into main Jun 5, 2026
12 checks passed
@mlataza
mlataza deleted the bug/sc-96140/set-explicit-mqtt-connecttimeout-instead-of branch June 5, 2026 14:01
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.

1 participant