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

Fix "close timed out" error when performing full deploy or modifying broker node. #3451

Merged
merged 1 commit into from
Feb 18, 2022

Conversation

Steve-Mcl
Copy link
Contributor

@Steve-Mcl Steve-Mcl commented Feb 17, 2022

fixes #2934

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Proposed changes

Prevent the close timed out issue (see references below)

  • Move all calls to node.client.end() to one location in code base (broker node function .disconnect() ) to reduce code paths and early calls.
  • Ensure done is always called for the "close" event in MQTT In/Out/Broker nodes (node.on('close') by removing the done callback from the node.client.end() signature - since it is (at best) unreliable due to the myriad of combination of user options and order of events (i.e. which nodes close first, flow file ordering, whether node has a close packet to sent etc)

References...

#2934
martin-doyle/node-red-contrib-aedes#46
martin-doyle/node-red-contrib-aedes#54
https://discourse.nodered.org/t/error-stopping-node-close-timed-out-mqtt-nodes/52830/5
https://discourse.nodered.org/t/mqtt-close-timeout-issue-using-aedes/57583
https://discourse.nodered.org/t/mqtt-in-node-not-working-properly-in-node-red-2-2-1/58428
https://discourse.nodered.org/t/mqtt-connects-then-disconnects/39609/6

Checklist

  • I have read the contribution guidelines
  • For non-bugfix PRs, I have discussed this change on the forum/slack team.
  • I have run grunt to verify the unit tests pass
  • I have added suitable unit tests to cover the new/changed functionality

@coveralls
Copy link

Coverage Status

Coverage increased (+0.002%) to 67.321% when pulling 669aa76 on Steve-Mcl:fix-mqtt-close-timeout into fcf2994 on node-red:master.

@Steve-Mcl Steve-Mcl changed the title Fix close timeout on MQTT nodes Fix "close timed out" error when performing full deploy or modifying broker node. Feb 17, 2022
@knolleary knolleary merged commit 6c7c120 into node-red:master Feb 18, 2022
@Steve-Mcl Steve-Mcl deleted the fix-mqtt-close-timeout branch February 18, 2022 18:09
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.

MQTT in Node: Error stopping node: Close timed out
3 participants