Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Client does not resubscribe after reconnect if clean = false #895

Closed
holopekochan opened this issue Dec 3, 2018 · 6 comments · Fixed by #1650
Closed

Client does not resubscribe after reconnect if clean = false #895

holopekochan opened this issue Dec 3, 2018 · 6 comments · Fixed by #1650

Comments

@holopekochan
Copy link

holopekochan commented Dec 3, 2018

Is there any reason we don't want resubscribe work if clean = false?
I have created a sub client with clean = false, and resubscribe = true, the client will not resubscribe the topics if MQTT aedes broker (no persistence) restart. And, sub-client will no longer receive the message of the topics
https://github.com/mcollina/aedes

https://github.com/mqttjs/MQTT.js/blob/master/lib/client.js
From MQTT.js/lib/client.js

 MqttClient.prototype._resubscribe = function () {
  if (!this._firstConnection &&
      this.options.clean &&
      Object.keys(this._resubscribeTopics).length > 0) {
    if (this.options.resubscribe) {
      this._resubscribeTopics.resubscribe = true
      this.subscribe(this._resubscribeTopics)
    } else {
      this._resubscribeTopics = {}
    }
  }

I can provide my sub client code and broker code if necessary

@zz123er
Copy link

zz123er commented Aug 20, 2019

@#749

@VirtualWolf
Copy link

I just ran into this myself with MQTT.js 4.2.8 and using Mosquitto 2.0.12 as the broker, and was extremely surprised by the behaviour. Was there a specific reason for it behaving like this, or is it some unintended bug?

@github-actions
Copy link

This is an automated message to let you know that this issue has
gone 365 days without any activity. In order to ensure that we work
on issues that still matter, this issue will be closed in 14 days.

If this issue is still important, you can simply comment with a
"bump" to keep it open.

Thank you for your contribution.

@github-actions github-actions bot added the stale label Oct 11, 2022
@Hypfer
Copy link

Hypfer commented Oct 11, 2022

bump

@github-actions github-actions bot removed the stale label Oct 12, 2022
@simonnilsson
Copy link
Contributor

The sessionPresentFlag should be available since version 3.1.1. And it is possible for broker to lose the persistent session, in that case you would want the client to automatically resubscribe.

Thus this line:

MQTT.js/src/lib/client.ts

Lines 2108 to 2110 in 18bdd49

(this.options.clean ||
(this.options.protocolVersion === 5 &&
!this.connackPacket.sessionPresent)) &&

Should be:

(this.options.clean || (this.options.protocolVersion >= 4 && !this.connackPacket.sessionPresent)) &&

Right?
@robertsLando

This problem seem to be observed in multiple issues
#749, #895, #1288

I can make a pull request for this. Not sure if this would be considered a breaking change, But I think most people woud expect resubscribe to work like this.

@robertsLando
Copy link
Member

Yeah that's a bug. Open a PR if possible, remember to add a test. Thanks

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 a pull request may close this issue.

6 participants