fix: resubscribe all topics when session_present=False on connection …#88
Merged
Conversation
…resume When the AWS IoT SDK auto-reconnects internally it fires _on_connection_resumed_internal which sets _connected=True. The MqttReconnectionHandler then sees the connection is up and exits its backoff loop without calling resubscribe_all(). With clean_session=True (the library default), session_present is always False on every reconnect -- the broker clears all subscriptions. Without this fix, the client reconnects successfully but has zero active subscriptions, so no device data ever arrives. Fix: when session_present is False in _on_connection_resumed_internal, schedule resubscribe_all() immediately so topics are restored before any queued commands are sent. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses a reconnection reliability issue in the MQTT client where AWS IoT SDK internal auto-reconnect can resume a connection with session_present=False (clean session), causing all broker-side subscriptions to be dropped without the client re-subscribing—resulting in no device data being received after reconnect.
Changes:
- On connection resume, detect
session_present=Falseand proactively triggerresubscribe_all()to restore subscriptions even when the reconnection handler exits early due to_connected=True.
Comment on lines
+368
to
389
| # previous subscriptions have been dropped server-side. We must | ||
| # re-establish them before any device data can flow. This covers the | ||
| # common case where the AWS IoT SDK auto-reconnects internally before | ||
| # the MqttReconnectionHandler fires its own reconnect path — in that | ||
| # scenario the reconnect handler sees _connected==True and exits early, | ||
| # so resubscribe_all() would never be called without this block. | ||
| if ( | ||
| not session_present | ||
| and self._subscription_manager | ||
| and self._connection_manager | ||
| and self._connection_manager.connection | ||
| ): | ||
| self._subscription_manager.update_connection( | ||
| self._connection_manager.connection | ||
| ) | ||
| self._schedule_coroutine( | ||
| self._subscription_manager.resubscribe_all() | ||
| ) | ||
|
|
||
| # Send any queued commands | ||
| if self.config.enable_command_queue and self._command_queue: | ||
| self._schedule_coroutine(self._send_queued_commands_internal()) |
Comment on lines
+367
to
389
| # When the broker starts a clean session (session_present=False), all | ||
| # previous subscriptions have been dropped server-side. We must | ||
| # re-establish them before any device data can flow. This covers the | ||
| # common case where the AWS IoT SDK auto-reconnects internally before | ||
| # the MqttReconnectionHandler fires its own reconnect path — in that | ||
| # scenario the reconnect handler sees _connected==True and exits early, | ||
| # so resubscribe_all() would never be called without this block. | ||
| if ( | ||
| not session_present | ||
| and self._subscription_manager | ||
| and self._connection_manager | ||
| and self._connection_manager.connection | ||
| ): | ||
| self._subscription_manager.update_connection( | ||
| self._connection_manager.connection | ||
| ) | ||
| self._schedule_coroutine( | ||
| self._subscription_manager.resubscribe_all() | ||
| ) | ||
|
|
||
| # Send any queued commands | ||
| if self.config.enable_command_queue and self._command_queue: | ||
| self._schedule_coroutine(self._send_queued_commands_internal()) |
- Fix race condition: ensure resubscribe_all completes before sending queued commands when session_present=False - Create _handle_clean_session_resume() helper method that combines subscription restoration with queued command sending in proper order - Add comprehensive tests for clean session resume behavior in test_mqtt_clean_session_resume.py - Fix pre-existing unreachable code in periodic.py
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
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.
…resume
When the AWS IoT SDK auto-reconnects internally it fires _on_connection_resumed_internal which sets _connected=True. The MqttReconnectionHandler then sees the connection is up and exits its backoff loop without calling resubscribe_all().
With clean_session=True (the library default), session_present is always False on every reconnect -- the broker clears all subscriptions. Without this fix, the client reconnects successfully but has zero active subscriptions, so no device data ever arrives.
Fix: when session_present is False in _on_connection_resumed_internal, schedule resubscribe_all() immediately so topics are restored before any queued commands are sent.