fix: prevent store message on store when it's restored

[ogis-yamazaki]

Cause

The message restored from the store was written to the store again before sending.
The writing is a time-consuming process of updating the stored message.
Meanwhile, next message overtook.

Fixes

The restored message was sent without being written to the store.

Added noStore flag to _sendStore function.

[ogis-yamazaki]

@YoDaMa

Rebased with the latest master.
Please confirm.

[YoDaMa]

hey @ogis-fujiwara, looks good. could you provide a repro of this behavior?

I want to add a test to actually catch this behavior we can demonstrate that this is fixing the bug, and that any future changes don't rollback this fix.

[ogis-yamazaki]

@YoDaMa

Added a replay test. We have also confirmed that the bug is fixed.

[YoDaMa]

I'm still not positive that this is the right move. You're removing the storage of the message which could create a new case with a bug.

What would happen if we follow your repro but after it reconnects it sends the initial packet again which is lost? It's not stored in the store before sending so it will not be resent...

[ogis-yamazaki]

@YoDaMa

I'm still not positive that this is the right move. You're removing the storage of the message which could create a new case with a bug.

PR did not seem to solve the essential problem.
I have recreated the PR, so please check it. 6d77f3e

The underlying problem was that external stores, such as mqtt-level-db, sometimes did not return callbacks in the order they were requested.

The callbacks for the external store are called in no particular order, as shown in the following link.

Level/leveldown#797

Added an internal queue so that it does not depend on the callback timing of the external store.

[redboltz]

@YoDaMa , @ogis-yamazaki and I are working on the same project that uses MQTT.js.
I reviewed the original version of the PR and I noticed that it is NOT an essential fix.
So I proposed the essential fix, then @ogis-yamazaki apply it and force pushed to the PR.
That is way 6d77f3e is my commit.

I believe that this PR is ready to merge.

[ogis-yamazaki]

Rebased on the latest main branch.

Overtaking still occurs in v4.3.7, so this PR is needed.

[codecov]

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.03 🎉

Comparison is base (a170c54) 86.22% compared to head (c05c4dd) 86.25%.

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

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1255      +/-   ##
==========================================
+ Coverage   86.22%   86.25%   +0.03%     
==========================================
  Files          13       13              
  Lines        1314     1317       +3     
==========================================
+ Hits         1133     1136       +3     
  Misses        181      181              

Impacted Files	Coverage Δ
lib/client.js	90.90% <100.00%> (+0.02%)	⬆️

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

[robertsLando]

Is this still good to merge?

[ogis-yamazaki]

@robertsLando

Rebased PR using the latest main branch.
Verified that the test passes.
Merge this in.

[ogis-yamazaki]

@robertsLando

I've added a comment.