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

Missed MAM messages beyond a number of 50 are permanently lost #1548

Closed
laszlovl opened this issue May 1, 2019 · 0 comments

Comments

@laszlovl
Copy link
Contributor

commented May 1, 2019

This is similar to #1534, but I think it's different enough to warrant a separate issue.

The problem is triggered as follows:

  • User enters a MUC
  • Someone writes message "1" in the MUC
  • User closes his browser
  • Someone writes messages "2" through "100" in the MUC
  • User re-opens his browser
  • Converse will now use MAM to request 50 messages after "1", e.g. messages "2" through "51". Because MAM paging is not implemented, Converse will not request additional messages after "51".
  • Someone writes message "101" in the MUC

Messages "52" through "100" are now permanently lost. Even if the user refreshes/rejoins, Converse will only query MAM for messages created after message "101".

I propose the short-term fix of actually implementing paging in fetchNewestMessages. Thus, after requesting messages "2" through "51", Converse should query for additional MAM messages created after "51".

In the long term, this might not be very sustainable though. In case of a long absence from a MUC, this method will query for all messages left since the user was last online, which could be quite a lot in case of an active MUC. Ideally, Converse would request only the 50 most recent messages then page upwards/backwards as needed. However, implementing that in the current architecture would cause even more missed messages because of #1534 (Converse would request messages "50" through "100", but then messages "1" through "49" would be permanently lost, as it's not aware that there might be more unretrieved messages between "1" and "50")

laszlovl added a commit to laszlovl/converse.js that referenced this issue May 1, 2019
laszlovl added a commit to laszlovl/converse.js that referenced this issue May 1, 2019

@jcbrand jcbrand added the bug label May 9, 2019

@jcbrand jcbrand added this to the 5.0.0 milestone Jun 28, 2019

jcbrand added a commit that referenced this issue Aug 3, 2019
jcbrand added a commit that referenced this issue Aug 3, 2019
jcbrand added a commit that referenced this issue Aug 3, 2019

@jcbrand jcbrand closed this in 8a9a0a4 Aug 3, 2019

jcbrand added a commit that referenced this issue Aug 6, 2019
Updates #1548. MAM paging improvements.
* Explicit forwards and backwards paging
* Include upper or lower bound when calling `RSM.prototype.next` or `RSM.prototype.previous`
* Bugfix: Don't override new RSM parameters (caused infinite recursion)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.