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: _storeProcessing staying true after outStore got emptied #1492

Merged
merged 2 commits into from
Jun 28, 2023

Conversation

Genoil
Copy link
Contributor

@Genoil Genoil commented Jun 8, 2022

This PR makes sure that new messages get published after a local store that accumulated some messages while the broker was offline, reconnects. In storeDeliver, this._storeProcessing got set to true before the check if there are actually stored packets left to send. So if there aren't any, this._storeProcessing will still be true on the next publish(), resulting in packets accumulating in this._storeProcessingQueue, but their invokes are never called, because Readable Event 'end' has already happened for the emptied Store and no new messages will be put into the Store as well.

Fixes #1116

@robertsLando
Copy link
Member

@Genoil Could you add a unit test for this please? I'm ok to merge it as soon as a test covers the fix

@codecov
Copy link

codecov bot commented Jun 23, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (3a80bde) 86.22% compared to head (d52f331) 86.22%.

❗ Current head d52f331 differs from pull request most recent head 40abfbf. Consider uploading reports for the commit 40abfbf to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1492   +/-   ##
=======================================
  Coverage   86.22%   86.22%           
=======================================
  Files          13       13           
  Lines        1314     1314           
=======================================
  Hits         1133     1133           
  Misses        181      181           
Impacted Files Coverage Δ
lib/client.js 90.87% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@robertsLando robertsLando changed the title Fix _storeProcessing staying true after outStore got emptied fix: _storeProcessing staying true after outStore got emptied Jun 27, 2023
@robertsLando
Copy link
Member

@gabrielfnogueira could you confirm this fixes your issue?

@gabrielfnogueira
Copy link

@gabrielfnogueira could you confirm this fixes your issue?

yes, this does fixes my issue!

@robertsLando robertsLando merged commit f3f7be7 into mqttjs:main Jun 28, 2023
6 checks passed
robertsLando added a commit that referenced this pull request Jun 30, 2023
Co-authored-by: Jan Willem Penterman <janwillem@gingeros.ai>
Co-authored-by: Daniel Lando <daniel.sorridi@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New messages not being published after store messages are processed - possible solution
4 participants