Closed douglasmuraoka closed 7 months ago
@TimelordUK let me know if it would be better to split these into smaller PRs so we discuss them individually.
This is great work and really very much appreciated. Replay has been tricky to get right and you have really moved things forward. I may add some more tests later. I am checking now and will merge very soon. Thanks a lot for another good PR
Found three minor issues.
One in
sequenceResetGap
, where the gap fill seq num was set withnewSeq
(should bestartGap
, because the seq num represents the start of the gap).Another issue is when the
endSeqNo
is0
. The existing code doesn't handle this scenario, and no gap fills are created. In this case, when the end seq no is 0, we should consider it as the latest seq num sent to the other peer (that's why I get it fromthis.sessionState.lastSentSeqNum()
, but let me know if this inaccurate).And the last issue is when we resend messages with zero messages retrieved from the store. In a situation where the only message sent to the other peer is
login
(seq num=1), for example, the store will be empty. But still, we have to generate a gap fill from1
to1
to respond to theResendRequest
message. The current code doesn't return anything (empty array of messages to be sent to peer).