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

connectTimeout and reconnectPeriod interaction #419

Closed
knolleary opened this issue Jun 13, 2016 · 9 comments
Closed

connectTimeout and reconnectPeriod interaction #419

knolleary opened this issue Jun 13, 2016 · 9 comments
Labels

Comments

@knolleary
Copy link
Member

If reconnectPeriod is set to 60 seconds (for example) but the connectTimeout left at the default 30 seconds, the reconnect attempt will be abandoned after 30 seconds and it will no longer keep trying to connect. Crucially, the application doesn't know this has happened and assumes the reconnect attempts will continue but they don't.

I believe the right approach is to clear the connectTimeout timer if the TCP connection fails, as that is already being handled by the reconnect logic.

Raising this for discussion/placeholder - can put together a PR later this week if agreed.

@mcollina
Copy link
Member

Yes, that would be awesome. Please send a PR!

@mcollina mcollina closed this as completed Sep 1, 2016
@mcollina
Copy link
Member

mcollina commented Sep 1, 2016

Closing with no activity. Feel free to reopen if this is still a bug for you.

@knolleary
Copy link
Member Author

Has anything been done to address the bug? Apologies for not following up with a pr by now, but this is a real issue.

@mcollina mcollina reopened this Sep 1, 2016
@mcollina
Copy link
Member

mcollina commented Sep 1, 2016

Unfortunately not, would you mind to send a PR, or at least add a quick unit test to reproduce? Those are tricky to produce :(.

@hoIIer
Copy link
Contributor

hoIIer commented Apr 24, 2020

is this still a bug? I'm trying to understand what causes the client to "close" then "end" and then "reconnect" after certain amount of time...

@YoDaMa
Copy link
Contributor

YoDaMa commented Apr 24, 2020

@burritoIand this is still a bug. we still need to address this.

@YoDaMa YoDaMa self-assigned this Apr 24, 2020
@YoDaMa YoDaMa added the bug label Apr 24, 2020
@YoDaMa
Copy link
Contributor

YoDaMa commented May 1, 2020

This would appear to take significant amount of work to disentangle the two. It would also look to require work with refactoring the state machine to be more... legitimate. So we are still waiting to get to this.

@robertsLando
Copy link
Member

@knolleary Would you mind to submit a PR to fix this?

@robertsLando
Copy link
Member

I tried to reproduce this but it seems to have been fixed by #1779, specifically the change that fixed this issue was:

https://github.com/mqttjs/MQTT.js/pull/1779/files#diff-98584baf8b7d243ed76eb88d8f603abb19f36a2957aabeb116e8a70d3c5507d9R1755

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants